Engraving #394
Engraving #394
Conversation
src/export.cpp
Outdated
| @@ -92,8 +91,27 @@ void SolveSpaceUI::ExportSectionTo(const Platform::Path &filename) { | |||
| sb->auxA = Style::SOLID_EDGE; | |||
| } | |||
|
|
|||
| el.CullExtraneousEdges(); | |||
| bl.CullIdenticalBeziers(); | |||
| el.CullExtraneousEdges(true); | |||
There was a problem hiding this comment.
Would be good to have /*both=*/ here and elsewhere because it looks quite puzzling otherwise.
| @@ -156,7 +156,7 @@ class SBezierLoopSet { | |||
| static SBezierLoopSet From(SBezierList *spcl, SPolygon *poly, | |||
| double chordTol, | |||
| bool *allClosed, SEdge *errorAt, | |||
| SBezierList *openContours); | |||
| SBezierLoopSet *openContours); | |||
There was a problem hiding this comment.
I don't quite understand why this is needed. Could you elaborate on it? That would also make a good commit message.
There was a problem hiding this comment.
Due to their different style, the engravings end up in the list of open contours during FindOuterFacesFrom, regardless of whether they form a closed contour or not. Considering that openContours is a list of segments, open loops have to be added per segment, thereby losing their connectivity. For example, an engraved circle will be exported as 4 separate arcs (at least for SVG). To fix this, I had to change the type of open contours to a loopset, which does retain connectivity.
There was a problem hiding this comment.
I'm still confused. Aren't open contours, by definition, not bezier loops?
There was a problem hiding this comment.
I guess. Though SBezierLoop::FromCurves returns open loops and there is SBezierLoop::IsClosed() to tell if a loop is closed. So I assumed that it's okay for loops to be open. And in every other aspect, that class does exactly what I need from it. If you want, I can rename SBezierLoop to SBezierContour, to make it more consistent.
There was a problem hiding this comment.
Oh, you're right. SBezierLoop doesn't have to be closed. That's OK then.
| } | ||
| openContours->l.Add(loop); | ||
| } else { | ||
| loop->Clear(); |
There was a problem hiding this comment.
Here is where the open loops used to be exploded into separate segments.
|
@bcmpinc Thank you for the PR, and sorry for the delay in merging. |
I mostly use solvespace for designing stuff that I then cut out using a laser cutter. For that purpose I always use the section export function. In the past I used Inkscape to add text or line segments, but I would prefer to be able to model these in solvespace, especially when positioning is important.
For custom line styles, there is an option "export these objects", which I hoped meant that these lines are included in the section export, but they aren't. This patch changes that, such that they are included. To get it to work properly I had to change some parts of the code for which I'm not sure whether I got them correct.
I don't know what "export these objects" is supposed to do, so I guess this is not entirely the right place for configuring a style for section export and that a new flag might be more appropriate.
Please let me know what you think of it.
P.S. I thought that it would be useful to add a way to disable the closed contour check for these custom line styles, but this is already possible by turning them into construction line segments (although those are visually indistinguishable).