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

Fillet extrusion of an SVG path #805

Closed
gautaz opened this issue Nov 30, 2024 · 13 comments
Closed

Fillet extrusion of an SVG path #805

gautaz opened this issue Nov 30, 2024 · 13 comments

Comments

@gautaz
Copy link

gautaz commented Nov 30, 2024

Hello,

For testing, I am using the following test.svg file:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 20010904//EN"
              "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">

<svg xmlns="http://www.w3.org/2000/svg"
     width="6.4in" height="3.6in"
     viewBox="0 0 1920 1080">
  <path id="Selection"
        fill="none" stroke="black" stroke-width="1"
        d="M 979.00,259.14
           C 979.00,259.14 1016.00,259.14 1016.00,259.14
             1192.00,436.00 1192.00,483.00 1192.00,483.00
             1062.56,678.37 1048.26,689.66 1032.00,697.51
             818.00,705.91 805.00,700.54 805.00,700.54
             618.26,569.37 615.07,566.81 611.31,560.00
             714.72,297.67 723.99,296.88 733.00,295.57
             928.00,264.00 968.00,260.17 968.00,260.17
             968.00,260.17 979.00,259.14 979.00,259.14 Z" />
</svg>

When trying to fillet the extrusion of this SVG with:

import logging
from build123d import *

logging.basicConfig(level=logging.INFO)

svg_scale, thickness = 0.3, 4


with BuildPart() as blob:

    with BuildSketch(Plane.XY.rotated((0, 0, -4.6))):
        with BuildLine():
            with Locations((-135, -110)):
                add(import_svg("./blob.svg"))
        make_face()
    extrude(amount=thickness/svg_scale)
    scale(by=svg_scale)

    fillet(blob.edges().group_by(Axis.Z)[-1], radius=1)


logging.info("Exporting...")
exporter3d = Mesher()
exporter3d.add_shape(blob.part)
exporter3d.write("blob.3mf")
logging.info("Export done")

It fails with these logs:

build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:None context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Entering BuildPart with mode=Mode.ADD which is in different scope as parent
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is pushing 1 workplanes: [Plane(o=(0.00, 0.00, 0.00), x=(1.00, 0.00, 0.00), z=(0.00, 0.00, 1.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is pushing 1 points: [(p=(0.00, 0.00, 0.00), o=(-0.00, 0.00, -0.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Entering BuildSketch with mode=Mode.ADD which is in same scope as parent
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is pushing 1 workplanes: [Plane(o=(0.00, 0.00, 0.00), x=(1.00, -0.08, 0.00), z=(0.00, 0.00, 1.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is pushing 1 points: [(p=(0.00, 0.00, 0.00), o=(-0.00, 0.00, -0.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildSketch context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildSketch context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildSketch context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Entering BuildLine with mode=Mode.ADD which is in same scope as parent
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is pushing 1 workplanes: [Plane(o=(0.00, 0.00, 0.00), x=(1.00, 0.00, 0.00), z=(0.00, 0.00, 1.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is pushing 1 points: [(p=(0.00, 0.00, 0.00), o=(-0.00, 0.00, -0.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Locations is pushing 1 points: [(p=(-135.00, -110.00, 0.00), o=(-0.00, 0.00, -0.00))]
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildLine context requested by None
build123d-1  | 2024-11-30T18:25:48+01:00 /opt/build123d/lib/python3.12/site-packages/build123d/topology.py:1784: UserWarning: Unable to clean Compound at 0x7f991e4ee930, label(), #children(0)
build123d-1  | 2024-11-30T18:25:48+01:00   warnings.warn(f"Unable to clean {self}")
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Completed integrating 7 object(s) into part with Mode=Mode.ADD
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Locations is popping 1 points
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is popping 1 points
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is popping 1 workplanes
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Exiting BuildLine
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildSketch context requested by make_face
build123d-1  | 2024-11-30T18:25:48+01:00 /opt/build123d/lib/python3.12/site-packages/build123d/topology.py:1784: UserWarning: Unable to clean <build123d.topology.Face object at 0x7f991e9d5e50>
build123d-1  | 2024-11-30T18:25:48+01:00   warnings.warn(f"Unable to clean {self}")
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Completed integrating 1 object(s) into part with Mode=Mode.ADD
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is popping 1 points
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is popping 1 workplanes
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Exiting BuildSketch
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by extrude
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:1 face(s) to extrude on 1 face plane(s)
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Completed integrating 1 object(s) into part with Mode=Mode.ADD
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by scale
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Completed integrating 1 object(s) into part with Mode=Mode.REPLACE
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:BuildPart context requested by fillet
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:LocationList is popping 1 points
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:WorkplaneList is popping 1 workplanes
build123d-1  | 2024-11-30T18:25:48+01:00 INFO:build123d:Exiting BuildPart
build123d-1  | 2024-11-30T18:25:48+01:00 Traceback (most recent call last):
build123d-1  | 2024-11-30T18:25:48+01:00   File "/models/./blob.py", line 20, in <module>
build123d-1  | 2024-11-30T18:25:48+01:00     fillet(blob.edges().group_by(Axis.Z)[-1], radius=1)
build123d-1  | 2024-11-30T18:25:48+01:00   File "/opt/build123d/lib/python3.12/site-packages/build123d/operations_generic.py", line 426, in fillet
build123d-1  | 2024-11-30T18:25:48+01:00     new_part = target.fillet(radius, list(object_list))
build123d-1  | 2024-11-30T18:25:48+01:00                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
build123d-1  | 2024-11-30T18:25:48+01:00   File "/opt/build123d/lib/python3.12/site-packages/build123d/topology.py", line 991, in fillet
build123d-1  | 2024-11-30T18:25:48+01:00     fillet_builder.Add(radius, native_edge)
build123d-1  | 2024-11-30T18:25:48+01:00 OCP.gp.gp_VectorWithNullMagnitude:

I am probably doing something stupid but I have no idea what.
Would someone be kind enough to shed some light on what the issue is?

@gumyr
Copy link
Owner

gumyr commented Dec 7, 2024

The svg is creating a weird object. Consider this:

svg_scale, thickness = 0.3, 4

svg = import_svg("./issue_805_blob.svg")
svg_vertices = svg.vertices()
svg_unique_vertices = []
for v in svg_vertices:
    if not any(v.distance_to(sv) < 0.01 for sv in svg_unique_vertices):
        svg_unique_vertices.append(v)

print(len(svg.edges()))  # 6
print(len(svg_vertices)) # 12
print(len(svg_unique_vertices)) # 6

with BuildPart() as blob:

    with BuildSketch(Plane((-135, -110)).rotated((0, 0, -4.6))):
        with BuildLine():
            p = Polyline(*svg_unique_vertices, close=True)
        make_face()
    extrude(amount=thickness / svg_scale)
    scale(by=svg_scale)

    fillet(blob.edges().group_by(Axis.Z)[-1], radius=1)

print(len(p.vertices())) # 6

Even though the object has 6 sides there are 12 vertices but when I make a Polyline from the unique vertices there are only 6 vertices as one would expect. The Polyline has straight sides so it isn't exactly the same as the original.
image

The VectorWithNullMagnitude is likely caused by these extra vertices but this is a OCCT error so it's hard to know exactly what the problem is.

There could be a problem with the SVG importer (which is external to build123d) - do you think there is still a problem to investigate here?

@gumyr gumyr added this to the Not Gating Release 1.0.0 milestone Dec 7, 2024
@gautaz
Copy link
Author

gautaz commented Dec 7, 2024

Hello @gumyr,

Thanks for this analysis.

The error root cause might also be how the SVG was generated.
I am currently using the Gimp export path feature to generate these SVGs (the blob one isn't the only one failing).

What is a bit unexpected for me is that the "blob" SVG is, if I am interpreting the SVG code right, composed of 8 bezier curves.
The first and last beziers are lines as both control points are placed on the vertices but this is not the case for all 6 beziers in between because all their first control points are placed somewhere else from the first vertex of the bezier.

And even if all the control points were placed on all the vertices, there would still be at least 8 vertices if I am counting right.
These vertices are:

979.00,259.14
1016.00,259.14
1192.00,483.00
1032.00,697.51
805.00,700.54
611.31,560.00
733.00,295.57
968.00,260.17

So ending with 6 vertices was a bit of a suprise to me.

I have highlighted the vertices directly in the SVG and 2 vertices are in fact aligned with two others which, sort of, masks them:
blob

I will try to see what happens when removing these "spurious" vertices from the SVGs I have tested before.
I will also try with a simpler SVG entirely hand tailored with only two bezier curves to see what happens.

In the meantime, as you said, the SVG importer is something that is taken off the shelf.
Is this importer also something implemented by Open CASCADE?

Should I try to file the issue on OCCT or somewhere else?

@gumyr
Copy link
Owner

gumyr commented Dec 7, 2024

I've already notified the author of the SVG importer and they are looking into it.

I did remove those extra vertices and still got the problem. Possibly due to the location of the control points.

@snoyer
Copy link
Contributor

snoyer commented Dec 8, 2024

It looks like there are 2 issues here...

First, while OCP considers a bezier edge where the first/last control overlaps with the start/end point to be valid, it really does not handle it well at all:

p1 = 0, 0
p2 = 5, 5
p3 = 10, 0
e = Edge.make_bezier(p1, p2, p3, p3)
print(f"{e.is_valid() = }")
print(f"{e.tangent_at(0) = }")
print(f"{e.tangent_at(1) = }")
e.is_valid() = True
e.tangent_at(0) = Vector(0.7071067811865, 0.7071067811865, 0)
Traceback (most recent call last):
  File "/home/snoyer/dev/cad/ocpsvg/examples/gh.py", line 123, in <module>
    print(f"{e.tangent_at(1) = }")
  File "/home/snoyer/.virtualenvs/ocpsvg-dev/lib/python3.10/site-packages/build123d/topology.py", line 475, in tangent_at
    return Vector(gp_Dir(res))
OCP.Standard.Standard_ConstructionError: gp_Dir() - input vector has zero norm

I'm assuming that something similar is happening internally within the filleting operation for it to throw the VectorWithNullMagnitude error.

Second, the shape itself may not be "nice" enough for OCP to deal with. With the following function we can bypass the above issue by nudging the coincident control points ever so slightly in the tangent direction:

def fix_edge(e: Edge):
    epsilon_d = 1e-9
    epsilon_t = 1e-9
    if e.geom_type == GeomType.BEZIER:
        bezier = e._geom_adaptor().Bezier()
        start, *controls, end = [
            Vector(bezier.Pole(i + 1)) for i in range(bezier.NbPoles())
        ]
        coincident_at_start = (start - controls[0]).length < epsilon_d
        coincident_at_end = (end - controls[-1]).length < epsilon_d
        if coincident_at_start and coincident_at_end:
            return Edge.make_line(start, end)
        else:
            if coincident_at_start:
                controls[0] += e.tangent_at(0 + epsilon_t) * epsilon_t
            if coincident_at_end:
                controls[-1] -= e.tangent_at(1 - epsilon_t) * epsilon_t
            return Edge.make_bezier(start, *controls, end)
    else:
        return e

Now if we run [a minimal variation of] the code sample again...

d = """
M 979.00,259.14
C 979.00,259.14 1016.00,259.14 1016.00,259.14
  1192.00,436.00 1192.00,483.00 1192.00,483.00
  1062.56,678.37 1048.26,689.66 1032.00,697.51
  818.00,705.91 805.00,700.54 805.00,700.54
  618.26,569.37 615.07,566.81 611.31,560.00
  714.72,297.67 723.99,296.88 733.00,295.57
  928.00,264.00 968.00,260.17 968.00,260.17
  968.00,260.17 979.00,259.14 979.00,259.14
Z
"""
with BuildPart() as blob:
    with BuildSketch():
        for ocp_wire in ocpsvg.wires_from_svg_path(d):
            wire = Wire(ocp_wire)
            wire = Wire(map(fix_edge, wire.edges()))
            add(wire)
        make_face()
    extrude(amount=100)
    print(f"{blob.part.is_valid() = }")
    fillet(blob.edges().group_by(Axis.Z)[-1], radius=1)

We get ValueError: Failed creating a fillet with radius of 1, try a smaller value or use max_fillet() to find the largest valid fillet radius instead of OCP.gp.gp_VectorWithNullMagnitude

@gautaz
Copy link
Author

gautaz commented Dec 8, 2024

Thanks both of you for these clarifications.

I have added the fix_edge function from @snoyer and tried to find the maximum radius that would fit and it appears that this radius is lower than 0 🙂.

So I have tried to found a minimal example to demonstrate this second issue without needing fix_edge and I have come to this one:

import logging
import ocpsvg
from build123d import *

logging.basicConfig(level=logging.INFO)


d = """
M 0.00,0.00
C 0.00,100.100 100.00,100.0 100.00,0.0
  100.00,-100.00 0.00,-100.00 0.00,0.00
Z
"""


with BuildPart() as blob:
    with BuildSketch():
        for ocp_wire in ocpsvg.wires_from_svg_path(d):
            wire = Wire(ocp_wire)
            add(wire)
        make_face()
    extrude(amount=100)
    fillet(blob.edges().group_by(Axis.Z)[-1], radius=0)

This ends up with:

Traceback (most recent call last):
  File "/models/./blob3.py", line 24, in <module>
    fillet(blob.edges().group_by(Axis.Z)[-1], radius=0)
  File "/opt/build123d/lib/python3.12/site-packages/build123d/operations_generic.py", line 430, in fillet
    new_part = target.fillet(radius, list(object_list))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/build123d/lib/python3.12/site-packages/build123d/topology.py", line 1002, in fillet
    raise ValueError(
ValueError: Failed creating a fillet with radius of 0, try a smaller value or use max_fillet() to find the largest valid fillet radius

I would have said at first that this was most probably due to the fact that there is a discontinuity at both vertices (well, I am not familiar enough with bezier curves to tell), but looking at the model without trying to apply fillet, I am not so confident that this is the cause for this "0" radius:

2024-12-08_11-02

@gumyr I am not quite sure what is the best course of action for the original issue, @snoyer is right:

  • There is the fact that my original SVGs are in fact a bit convoluted, should OCCT deal with that when applying fillet/chamfer?
    Or shouldn't OCCT try to fillet the model until it cannot and just leave the problematic parts as is?
  • There is this second issue with the "0" radius which I have no clue why the maximum possible radius is, well, so minimal.

I feel like I have opened pandora's box, sorry.

EDIT: Looking at the definition of Bézier curves, there is no discontinuity in the shape as, at both vertices, the tangent vectors defining both Bezier curves are parallel ((0,100) and (0,-100)).

@snoyer
Copy link
Contributor

snoyer commented Dec 8, 2024

@gautaz, your latest minimal example with the 2 bezier curves should work, see this screenshot with radius=35

image

Maybe you need to update the ocpsvg package as I did push fixes earlier this week (I'm not sure they are relevant to the situation at hand but may as well make sure)

the OCC kernel can be a bit funny with fillets, for example for this random path drawn in Inkscape:

d = """
m 696.875,296.875
c 130.57365,-24.00515 280.94885,-73.9965 401.2901,8.80364 106.7063,84.65404 78.6898,235.8703 22.6071,340.41541 -86.8949,134.46831 -230.88529,-6.55489 -346.04657,25.37209
C 630.21748,698.41227 600.27391,533.70059 673.1101,441.67239 693.64812,396.89464 710.89482,346.26422 696.875,296.875
Z
"""

I can get it to fillet for radius=10
image
and radius=50
image
but radius=35 will throw the Failed creating a fillet with radius of 35 error 🤷

@gautaz
Copy link
Author

gautaz commented Dec 8, 2024

Hello @snoyer, I have just checked the version of ocpsvg that I am using in my containerized environment:

docker compose exec build123d cat /opt/build123d/lib/python3.12/site-packages/ocpsvg-0.3.2.dist-info/METADATA
Metadata-Version: 2.1
Name: ocpsvg
Version: 0.3.2
Requires-Python: >=3.9
Description-Content-Type: text/markdown
License-File: LICENSE
Requires-Dist: cadquery-ocp>=7.7.0
Requires-Dist: svgelements<2,>=1.9.1
Provides-Extra: dev
Requires-Dist: pytest; extra == "dev"

# OCPSVG

- works at the [`OCP`](https://github.com/CadQuery/OCP) level
- uses [`svgelements`](https://github.com/meerk40t/svgelements) to:
  - convert SVG path strings to and from `TopoDS_Edge`, `TopoDS_Wire`, and `TopoDS_Face` objects
  - import `TopoDS_Wire` and `TopoDS_Face` objects from an SVG document
- can be used to add SVG functionality (import and export) to higher level API

Is version 0.3.2 the right one?

@snoyer
Copy link
Contributor

snoyer commented Dec 8, 2024

Is version 0.3.2 the right one?

Yes, that's the latest one and the one I used for the examples in my previous post

@gautaz
Copy link
Author

gautaz commented Dec 8, 2024

And you are right, it works with radius=35:
2024-12-08_12-38

Wow, I need to check with a whole range of different radii to see what works.

EDIT: So it works for all non-zero radii from 35 to 1, 0 is just a corner case for the error message ValueError: Failed creating a fillet with radius of 0, try a smaller value or use max_fillet() to find the largest valid fillet radius which is in fact a bit misleading.

@gumyr
Copy link
Owner

gumyr commented Dec 9, 2024

The error message is just trying to be helpful - the complexities of OCCT Bezier and fillets are not fully accounted for. As described in the first tip here Tips, Best Practices and FAQ not everything that can be described can be built by OCCT. The art of this endeavour is working around limitations.

@gautaz
Copy link
Author

gautaz commented Dec 12, 2024

@gumyr Just to be clear, I find build123d impressive and the code really expressive when using it.
It has some rough edges and I just have to get use to them :-).

The error message is just trying to be helpful - the complexities of OCCT Bezier and fillets are not fully accounted for.

I totally agree with that but currently the message feels misleading to me.
The way I understand it, this message occurs when the issue is related to the radius value but not necessarily because the radius is too high.
Perhaps the message should reflect that.

Another pain point is that the message tells to use max_fillet() but I didn't find an example using this function (I was in fact unable to use it so I had to go through a whole try and error process...)
If such an example exists and I didn't find it (again, I am the kind of guy not seeing the butter in the fridge in front of me), perhaps adding a URL leading to the example directly in the message could help...

As described in the first tip here Tips, Best Practices and FAQ not everything that can be described can be built by OCCT. The art of this endeavour is working around limitations.

I also agree with this, I had already read the tips.
I can understand that one has to work around limitations, in particular around the limitations of the underlying library.
The problem for a newcomer is that these limitations are not clearly stated somewhere (or at least I didn't come up with such a list easily).
What would be the best way to make the distinction between a build123d bug and a limitation imposed by OCCT?

@gumyr
Copy link
Owner

gumyr commented Dec 12, 2024

The error message is just trying to be helpful - the complexities of OCCT Bezier and fillets are not fully accounted for.

I totally agree with that but currently the message feels misleading to me. The way I understand it, this message occurs when the issue is related to the radius value but not necessarily because the radius is too high. Perhaps the message should reflect that.

99% of the time the problem is that the radius is too large. This problem is very strange and doesn't reflect what most users see.

Another pain point is that the message tells to use max_fillet() but I didn't find an example using this function (I was in fact unable to use it so I had to go through a whole try and error process...) If such an example exists and I didn't find it (again, I am the kind of guy not seeing the butter in the fridge in front of me), perhaps adding a URL leading to the example directly in the message could help...

If one searches in the docs for max_fillet it comes up immediately.

As described in the first tip here Tips, Best Practices and FAQ not everything that can be described can be built by OCCT. The art of this endeavour is working around limitations.

I also agree with this, I had already read the tips. I can understand that one has to work around limitations, in particular around the limitations of the underlying library. The problem for a newcomer is that these limitations are not clearly stated somewhere (or at least I didn't come up with such a list easily).

The limitations in OCCT are not described anywhere but experienced users learn to avoid them after a time. build123d attempts to work around them and the docs provide general guidelines as how to avoid them but there is no way to list them all. CAD S/W is incredibly complex as it needs to work for arbitrary geometries which is very challenging so it's no wonder that OCCT has some (sometimes frustrating) limitations.

What would be the best way to make the distinction between a build123d bug and a limitation imposed by OCCT?

Sometimes it takes me a while to determine if a bug should be attributed to build123d or OCCT, I don't expect users to be able to do so.

@gautaz
Copy link
Author

gautaz commented Dec 12, 2024

Thanks for all these explanations.

As for max_fillet, sorry for the misundestanding, I was already aware that the function is detailed here.
I was asking for a usage example because trying to use it just by reading the reference documentation did not help me.
I understand though that this is because I am not fluent enough with build123d.

As for the issue itself, FMU there is not much build123d can do about it as it is more related to OCCT limitations, so I am closing it.
Feel free to reopen if needed.

@gautaz gautaz closed this as completed Dec 12, 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

No branches or pull requests

3 participants