-
Notifications
You must be signed in to change notification settings - Fork 511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Circle rendering is very conservative #295
Comments
Imagine you're extruding this circle. Now, the entity (the flat circle) has 128 edges, but the solid mesh (the spatial cylinder) has 8 edges. No way to rectify this inconsistency. cc @Evil-Spirit @jwesthues this seems to be a really common complaint, any ideas to at least ameliorate it? |
Hmm so I had a look in the source code and discovered the chord tolerance setting, which obviously helps. Reducing it from 0.5% to 0.01% basically fixes this for circles down to 1mm diameter, which is probably about the limit that I imagine most people would care about. To be honest I would have expected the code to work by representing all the geometry as true curves, and then turning them into polygons for display. In which case it should be easy (well "easy") to set the chord accuracy dynamically based on how big the object is on screen. Or in other words, the chord accuracy should be measured in screen-space pixels, not world-space mm. You know more than me though so maybe it isn't that simple. :-) I just checked how Blender handles this. It seems like it uses a fixed number of divisions of each bezier irrespective of their size. Also not ideal. |
I guess this is the relevant code?
Maybe as a not-ideal fix it could just be changed so that the minimum number of segments is more than 2. |
That's exactly what we do.
Sure, but regenerating the entire mesh for a large project can take seconds, if not minutes. Do you want this on every zoom? |
Ideas:
You're obviously doing lots of little things that help (2). I don't see any one easy big thing, or else we'd have done it already... @Timmmm This is the same tolerance used for the NURBS Boolean operations, so the computation isn't trivial. We could make the tolerances different for entities vs. the mesh, but then e.g. entities wouldn't line up with the edges of the mesh they were extruded to form. |
I don't think if we should do something with this. |
This is worth exploring, I think. |
How does other CAD software handle this? I don't recall Solidworks or NX ever getting close to an octagon, even for 1mm circles. (Sorry if it sounds like I'm complaining btw - this is by far the best open source CAD software in existence, and better in a few ways than the big commercial ones.) |
I think they indeed calculate tolerance relative to primitive bound. The more I think about it, the more I become convinced that this is the only reasonable option. It might even replace the absolute/export tolerance (or at least both tolerances will be relative values). @jwesthues, any opinion? |
Commercial CAD systems generally do a fixed chord tolerance, either absolute (like for export) or as a function of model size and/or zoom level and stuff. They presumably have lots of special case heuristics on top of that. If you've never seen the PWLs there, then I'd guess it's because they're just faster, making better use of the GPU, selectively re-meshing, etc. So they can set a finer tolerance without getting too slow. All of that could be done in SolveSpace, but it's a lot of work. What's the rationale behind tolerance as a fraction of primitive bound? Like, if I represent the exact same geometric curve as one Bezier piece vs. two, why do I want finer PWLs for the two case? A single chord tolerance for the entire model is at least internally consistent. Note that the Booleans also use a chord tolerance, and rely on that geometric consistency. |
Ack. This seems a somewhat frequent UI papercut. What do we do here? Teach it better? Add some sort of heuristic that acts when you place the first sketch? |
Add a "chord angle" tolerance, on the maximum angle between the curve and the chord approximating it? So if you want big things meshed finer, then you make the chord distance tolerance smaller, but if you want little things meshed finer then you make the angle finer. That's how SolidWorks export does it, for example. |
That seems very elegant. |
And easy to implement--just modify |
For what it's worth, this also occurs during image export which is highly undesirable. My specific instance is to make a lineart isometric diagram of a small part; all the circles render as octagons even when >10% the size of the whole page. |
Hi, this is always a first thing I change in the freshly compiled, first time run solvespace. Then I always need to play with the max segments and tolerances. Relative tolerances would be way better I think. I usually change my segments settings to 32 or 100, and only back it down to lower number if performance suffers, which is almost never. 8 is really super low. |
This is by far my biggest issue with Solvespace at the moment, and Solvespace is by far the best open source CAD software, so I had a look at the code to understand this more. This is on How it currently works
(By the way is there a GUI for these settings? I can't see a way to edit them...)
So Same small circle but with the large circle deleted:
Here it also checks the max segments. Also, when exporting it just returns
Problems with this approach
Possible solutionsWe have the existing constraints:
There are some other constraints that are possible.
My thoughts on the constraints are:
All these constraints can be combined in a boolean expression. Maybe one day the user could pick it. Anyway I am going to experiment with some different ones - I'll start with 4. |
Oh, also if you don't use a constraint that is proportional to the scene size, you don't need to call |
I think your suggestion (4) makes a lot of sense. I'd also ask @jwesthues about it. |
You are correct that the max line segment count per Bezier segment (1) doesn't make any geometric sense. I intended it just as a sanity check, to stop the regeneration from becoming unusably slow if the user chooses a very fine distance tolerance, especially since there's no way to interrupt that. If you haven't hit such problems, then I suspect you don't make big solid models and would be happy greatly increasing (1) from the default. Per above, distance as a fraction of Bezier length/bounding box/whatever (4) also doesn't make geometric sense--the Beziers will get chopped up by Boolean operations, and it doesn't make sense to represent the same physical curve finer just because it's now in two pieces. The Booleans assume a consistent distance tolerance, which is achieved only by (2) and (5). Perhaps (4) could replace line segment count as the sanity check, though it's perhaps more obvious to a user that "10000 segments" might get slow than that "0.0001 relative chord tolerance" will. I'm not sure what you mean by a tolerance on the cross product? The magnitude of that has dimensions of length squared times angle, which does not seem useful. The combination of angle tolerance (for little things) and chord tolerance (for big things) is pretty common in professional CAD. |
@Evil-Spirit Your work has served us very well so far, and adding an angle chord tolerance would be building on top of it, not replacing it. Or am I missing something? |
Personally I do not mnid tiny (compared to the model size) octagonal circles/holes at all. It is obvious to (this) user that it is just an optimization for the visual rendering. Real tiny octagons are "rare" (nuts are usually hexagonal :-) so there is little risk for confusion. As for visually checking alignment - constraints are for that. So i would not trade "smooth tiny circles" for any performance loss. If it is possible to do it without performance loss AND - more importantly - very complicated (potentially buggy) code - then fine. I love Solvespace because it is small and the code is beautiful and (relatively) simple. This issue seems to go against this. |
@phkahler This isn't really a mesh issue, is it? The Bezier curves from entities are turned into piecewise linear curves independently from the solid model. |
@Timmmm and @Evil-Spirit Thank for putting a lot effort into your investigations. I am not super familiar with solvespace internals, especially rendering, only some pieces of the solver and algebraic constraints, but I am pretty sure there is room for improvments, so the rendered can be more smart. @ruevs octagonal ones bugs me a little bit. If it was 16-sided polygon I would be okish with it. No commercial CAD program in the world really has pixel accurate rendering (which is actually very unfortunate), but I think there might be some interesting smarter options to look at. How about when doing instantiation of the geometry for visualization, looking at the angles between consecutive line segments. And if it is too high (weighted by the zoom factor maybe?) backtrack and refine the approximation? Another observation. People often edit the model, and then look at it, rotate, without changing any constraints. How about having a thread in a background asynchronously that do recalculations at higher level of detail and when it is ready update it. If new update comes in, cancel it. Just some ideas. I work with solvespace on desktop, with good CPU and good GPU, so I rearly see performance issues, but more interactive experience is worthwhile for everybody. |
That was suggested in #295 (comment). It seems like a good route.
At the moment the internals of SolveSpace aren't well suited for parallel, asynchronous processing, so this is probably not viable. It would also not work for the single-threaded Emscripten port, and given that it significantly affects UI, I'd much rather evaluate other approaches first. |
Hi all, Supporting XP has become eccentric - I understand. But I enjoy my machine booting to 165MB of RAM used. And as long as I have XP machines that are used and useful (right now I have two) I will keep a "fork" of SolveSpace that works on them. I understand that Now back to the OpeGL 3 renderer and it's shaders. I know almost nothing about OpenGL programming but Google knows and from what I have read if you want to do
This requires a tessellation shader and not a geometric shader. Tesselation shaders were introduced in OenGL 4 and thus using them will require graphics drivers that support OpenGL 4. It seems Intel graphics started supprting OpenGL 4 in Gen7, which is Ivy Bridge. So for example people with a Sandy Bridge e.g Core i7-2600K 4 cores/8 threads, 3.40 GHz/3.80 GHz Turbo, 8 MB Cache will not be able to run SolveSpace. Really?!? And if a fallback is considered necessary anyway is it worth having the shader for performance, the fallback and the added complexity of "detecting and switching"? |
There's nothing complex about that—the story of maintaining an open-source project is the story of dealing with boundless entitlement, passive aggression, and guilt trips from people benefiting from labor of volunteers. I maintain a lot more of them than you do (which is a wise choice on your part), am correspondingly exposed to more, and therefore have far lower tolerance for it.
Yup, that's why I mentioned OpenGL 4 support.
Of course not. The reason I said that we "can" use geometric shaders is because the hardware and software support for them became widely available, in the sense that all of our target platforms, in principle, should be able to use them on recent hardware. The context you are probably missing here is that a few years ago, @Evil-Spirit suggested using OpenGL 4 features, and back then they were available only on select high-end hardware and on few platforms, so we couldn't use them at all, in contrast to the current situation. So at the time, I decided that we wouldn't spend any time trying to add OpenGL 4 dependent features at all, which has changed now.
I don't understand how can you argue, in two statements that follow each other, simultaneously in favor of having a fallback OpenGL 1 renderer for machines without OpenGL 3, and against having a fallback OpenGL 3 renderer for machines without OpenGL 4. So another reason why your attitude annoys me is that your position seems to be that the complexity that benefits you is necessary or at least tolerable, whereas the complexity that doesn't benefit you is a drawback. You're, of course, free to consider solely your personal convenience, but as a maintainer I don't have this option. |
As for Angle - as we have discussed - I know it can work under XP - Firefox 52.9.0esr is a living proof. Maybe I will spend the time to figure out how one day. |
@whitequark "ACK" for all the technical points.
Just because the GL1 renderer exists, happens to still work OK and (as far as I am aware) causes no work or grief for anyone. So it is a zero overhead left-over that no one but me cares about and will bit-rot until removed.
See above about the complexity. As for "my convenience" - I am not expecting or demanding anything from anyone - thus the "sad" statement above was useless and I should not have written it. I work on SolveSpace (less than I would like to) - when I have some time - because I like it, the architecture (mostly), the code, the UI - that's all. Back to semi-technical arguments - Ivy Bridge was introduced in 2012 so if using OpenGL 4 features and dropping OpenGL 3 (and 1) will make SolveSpace faster and simpler I think it is better to do it instead of adding "fallbacks". It's not my intention to reduce the S/N ratio, thus this is my last non-technical post. But I reserve the "right" to annoy anyone and everyone with technical arguments if I think it's important :-D |
@ruevs Thank you for being considerate, and I should note that I appreciate both your technical arguments and your other contributions. I do note that what counts as "too complex" is inherently social, and should be, perhaps, handled with more care than usually happens in technical discussions—but by no means I want to shut down that discussion either.
I think we are in agreement here: I also think that if dropping support for obsolete hardware simplifies the codebase, and if we can actually do it without impacting too many users (we'll never impact zero and that's just one of the unfortunate facts of life), we should do it. So the question is, what's the cost, and what's the impact? We can never be sure of impact because we don't collect telemetry. We can only rely on hardware surveys by other projects, like Firefox or Steam, and those aren't necessarily representative. Cost is a much easier consideration. I would not expect that a GPU-based Bezier renderer would result in major increase in complexity (wouldn't it be based on a pwl decomposition anyway?), but I could be wrong. In any case, adding a fallback wouldn't require forking the renderer, and so it's much easier to test and maintain. While looking into this I discovered that even WebGL 2 doesn't actually support geometry shaders, it only has transform feedback. So the question of whether we'll need a fallback if we use GPU-based Bezier rendering is moot: the answer is yes. |
I'm afraid we may be discussing this into exhaustion. But just to compound that, I think I see people doing Beziers just with vertex shaders, with a fixed number of line segments per Bezier? The fixed segment count is wasteful, but maybe the whole thing then gets fast enough we don't care, or we could use simple heuristics to choose from a small number of fixed segment counts? Or just more line segments on the CPU of course (for loose curves only, not for the solid model). I suspect all of that is near-infinitely fast compared to the solid model. |
Yeah, the discussion veered into geometry shaders without much consideration to whether we can already do it. I think an approach using a fixed segment count would be entirely acceptable while not requiring anything beyond existing OpenGL 3. I remember seeing quite a few options; besides the ones I've mentioned earlier I think you can also do this with signed distance functions. |
I'll just chime in again, and then I'm moving on to better things. IMHO:
|
You're completely right, and I missed the Z-buffer implications. I suspect this rules out any solution that doesn't ultimately amount to a more sophisticated pwl decomposition. |
Not to disagree with you, but to refine this statement: once we tie the Beziers to the mesh, the performance implications become correctness implications as well. I've had a design with a combination of small holes and large holes, and it was not trivial to find a display chord tolerance that wouldn't cause booleans to fail. (Then again for export, but what we currently do for export is good.) This was after we tied display chord tolerance to a percentage of entity bounding boxes in 34a5d87. I'd have to investigate but I suspect that is not sufficient in some cases, though it certainly makes things a lot better. |
I wrote about such approach - we can use it without geometrical shaders. |
@jwesthues Can you explain why changing circles to a minimum of 16 segments (4 per curve) results in the issues shown above by both @ruevs and myself? Is there a mismatch between this and some other part of the code? and why does it not make squares from octagons? This seems important to understand for people working on the innards. |
@phkahler No idea without actually debugging, I'm afraid. What happens if you just modify the worker to keep recursing until |
I found the problem with 4-segment arcs. At the top of function: void SCurve::RemoveShortSegments(SSurface *srfA, SSurface *srfB) {
// Five, not two; curves are pwl'd to at least four edges (three points)
// even if not necessary, to avoid square holes.
if(pts.n <= 5) return; This is my fix. It used to say "three, not two" because curves had minimum 2 segments "to avoid square holes". Together with this change over in raypoly: // unhappy with sixteen sides (formerly octagons).
MakePwlInitialWorker(l, 0.0, 0.25, chordTol);
MakePwlInitialWorker(l, 0.25, 0.5, chordTol);
MakePwlInitialWorker(l, 0.5, 0.75, chordTol);
MakePwlInitialWorker(l, 0.75, 1.0, chordTol); We can have all circles with minimum 16 segments (4 per curve). The function in curve.cpp was deleting short segments on the pwl, but only for one surface? I thought pwl were shared by the two surfaces, so deleting segments from one affects both. Anyway, by forcing more edges that go below chord tolerance, that other function needs to be changed or it will delete edges. I have no idea if there are any negative consequences for doing this. The comment above RemoveShortSegments suggests that operation is necessary because short segments can break booleans. That suggests to me the possibility that some boolean errors may be caused by the early exit leaving small edges that break booleans. That would also mean that increasing the minimum edges to 4 there might make things worse (more early exits). It seems like there should be a more elegant way to remove the short edges while not breaking booleans and not reducing circle edges. @jwesthues @ruevs @Evil-Spirit Any ideas? |
Thanks, had forgotten about that. Edges smaller than the chord tolerance are no problem; it's when edges get small compared to our numerical epsilons that stuff can break, and that's much smaller. So I wouldn't expect any new problems from more segments in our circles. But I think it would be better if And I'm not sure what you mean by "for only one surface"? In the pictures, I thought the smoother circles were the bare entities (independent of the solid model), and the solid model was consistently square. If the two surfaces' trim curves are being treated differently (and thus creating a non-watertight mesh?) then that would definitely be a bug, and one that potentially affects the Booleans. |
Yes, that was a mistake in my memory. When I first saw it I thought the cylinder was keeping all the edges but it's just the curve and none of the surfaces (still seems a little weird). I also had the idea of using a fraction of the curve tolerance there (I don't want to speak too much, especially when soliciting ideas). The original pwl points are presumably correct WRT curve tolerance, and the split point will be at the end. The only points that might be removable should be the ones adjacent to the ends then? Maybe only if they're geometrically very close to the end point? |
A split can occur anywhere along the curve, since a curve in shell A can get split anywhere that it intersects a surface from shell B (and surfaces in shell B can be anywhere). |
I mean the problematic points should be near the end of the portion of the PWL after it's split. There may be 20 points and it splits right next to the 2nd one so we get a pwl with 3 points - 2 original ones and one at the splitting surface. Or the remaining part could have 10 points but the only ones in question should be near the split plane. |
First, note that the beginning of an You can also construct geometry where a curve really does need to get split in the middle, e.g. a cylinder minus a triangle inscribed in that circle and extruded (though many such cases will also break for other reasons). I think curves get split unnecessarily too, though it's been a long time. In any case, the short segments definitely aren't just at the beginning or end of the |
I have a solution in PR #626. It keeps non-vertex point if the curve is exact and the distance in t-parameter between points is more than 0.1 regardless of chord tolerance. Such points should only have been added deliberately as in the circle/octagon code. This allows removal of the early exit based on number of points. |
Also including an update so 2nd or 3rd degree curves have a minimum of 4 segments. Circles will have at least 16 segments so we shall close this issue with that PR. There will also be another PR to reduce default curve tolerance to a more useful value. |
Okay. I'd like to take a close look at those before merging, please let me know when you're done. |
@whitequark woops, I merged this one before I saw your post. I'll hold off the other one, which is having conflicts due to this anyway. |
Before this commit, with the highest chord tolerance settings, circles would render as octagons, which confused a lot of people. See #295.
Before this commit, with the highest chord tolerance settings, circles would render as octagons, which confused a lot of people. See solvespace#295.
SolveSpace version: 2.3
Operating system: MacOS 10.13
When sketching a circle it seems like the rendering accuracy (i.e. how many line segments it is split into) depends on the physical size of the circle, rather than its on-screen size. The result is if you draw a small circle and zoom in it looks like this:
I.e. not very circular. Nobody is running this on a 386 so I would suggest as a simple hacky fix just increase the minimum number of segments from 8 to something a bit more reasonable like 128.
The text was updated successfully, but these errors were encountered: