Skip to content
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

CurvedSegment Ribs are too small and don't fit the hullcurve #34

Closed
AIRCAP opened this issue Sep 16, 2024 · 5 comments · May be fixed by #53
Closed

CurvedSegment Ribs are too small and don't fit the hullcurve #34

AIRCAP opened this issue Sep 16, 2024 · 5 comments · May be fixed by #53

Comments

@AIRCAP
Copy link
Contributor

AIRCAP commented Sep 16, 2024

The CurvedSegment tool creates ribs that are too small, they do not fill the hullcurve.

In the attached document, the beginning and end shape is identical (a circle) - the ribs in between are interpolated once with CurvedArray using only one reference shape, and once with CurvedSegment using both, in both cases using the same single hullcurve (which is just a straight line/box)
while the curved array ribs completely fill the hullcurve (green), the curved segment ribs are about 5% too short (red)

wrongshape.zip
test2
test

@AIRCAP
Copy link
Contributor Author

AIRCAP commented Sep 16, 2024

5% was a guesstimate. here are the boundingboxes:

App.ActiveDocument.CurvedArray.Shape.Edges[1].BoundBox
BoundBox (-50, 20, -50, 50, 20, 50)
App.ActiveDocument.CurvedSegment.Shape.Edges[0].BoundBox
BoundBox (-49.9999, 22, -49.2467, 50, 22, 49.2459)

in this case the ribs are too short by 1.5mm for a diameter of 100mm - or 1.5%
the issue also happens when using 2 hullcurves

@AIRCAP
Copy link
Contributor Author

AIRCAP commented Sep 16, 2024

when not using a hullcurve, the size is correct - so the issue doesn't seem to be the shape interpolation of the ribs (which is trivial in this case) but the scaling to the hullcurve(s)

@AIRCAP
Copy link
Contributor Author

AIRCAP commented Sep 16, 2024

found the issue. scaleByBoundbox() computes the scale factor based on relative sizes of the bounding boxes. The bounding box of the hullcurve is calculated correctly. However the bounding box of the to be rescaled shape - in case of the CurvedArray is taken from the base shape - which is almost always correct (unless the base shape has been created by script)
while for a CurvedSegment, the shape is newly generated (half way between the end shapes) but has not yet been displayed.
FreeCAD only tesselates shapes when they are shown in the GUI. prior to tesselation the BoundBox is a rough estimate and can be too large by a few percent.

The following workaround fixes the issue:

--- a/CurvedSegment.py
+++ b/CurvedSegment.py
@@ -153,7 +153,8 @@ class CurvedSegmentWorker:
             normal = CurvedShapes.vectorMiddle(fp.NormalShape1, fp.NormalShape2, d)
             #Draft.makeLine(ribs[i].BoundBox.Center, ribs[i].BoundBox.Center + normal)
             bbox = CurvedShapes.boundbox_from_intersect(fp.Hullcurves, ribs[i].BoundBox.Center, normal, self.doScaleXYZ)
-            if bbox:              
+            if bbox:
+                ribs[i].tessellate(0.01) # needed for correct bounding box of newly created shapes
                 ribs[i] = CurvedShapes.scaleByBoundbox(ribs[i], bbox, self.doScaleXYZsum, copy=False)

@AIRCAP
Copy link
Contributor Author

AIRCAP commented Sep 16, 2024

@chbergmann
Copy link
Owner

Thank you for your contribution to fix this

AIRCAP added a commit to AIRCAP/CurvedShapesWorkbench that referenced this issue Nov 19, 2024
@AIRCAP AIRCAP mentioned this issue Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants