gumyr / build123d

A python CAD programming library
Apache License 2.0
432 stars 81 forks source link

Adjacent edges return no common vertices #474

Closed MatthiasJ1 closed 7 months ago

MatthiasJ1 commented 8 months ago

This version is different from the one in the Discord in that it doesn't use OCCT directly, only build123d functions to remove any unknowns.

labels = []
r = Rectangle(10,10)
ordered_edges = r.face().outer_wire().order_edges()

for i in range(4):
    e1 = ordered_edges[i]
    e2 = ordered_edges[(i+1) % 4]
    n_common = len(set(e1.vertices()).intersection(e2.vertices()))
    labels.append(Pos(e1.center()) * Text(f"e{i}", 2))
    labels.append(Line(e1.center(), e2.center()))
    labels.append(Pos((e1.center()+e2.center())/2) * Text(f"{n_common}", 1))
labels.append(Text("# common verts", 1))
Screenshot
gumyr commented 7 months ago

From discord:

I'm taking a look at the chamfering performance issue on M1 (#369) and I've run into an odd bit of behavior I was hoping someone with more experience of the backend might understand. Any ideas as to why this is claiming only one pair of edges share a common vertex?

from build123d import *
from typing import Iterable, Tuple, T
import itertools, ocp_vscode
from OCP.TopExp import TopExp, TopExp_Explorer

labels = []
r = Rectangle(10,10)
ordered_edges = r.face().outer_wire().order_edges()

def pairwise_looped(it: Iterable[T]) -> Iterable[Tuple[T,T]]:
            a, b = itertools.tee(it)
            return itertools.zip_longest(a, b, fillvalue=next(b, None))

for i,(e1, e2) in enumerate(pairwise_looped(ordered_edges)):
        l = Line(e1.center(), e2.center())
        v = Vertex()
        has_common = TopExp.CommonVertex_s(e1.wrapped, e2.wrapped, v.wrapped)
        labels.append(Pos(e1.center()) * Text(str(i), 3))
        labels.append(l)
        labels.append(Pos(l.center()) * Text(f"{i}: {has_common}", 1))

ocp_vscode.show_all()

image A more "direct" approach like has_common = set(e1.vertices()).intersection(e2.vertices()) gives the same result.

gumyr commented 7 months ago

I've done some more exploration on this. Consider the following modified code:

from build123d import *
from ocp_vscode import *

labels = []
r = Rectangle(10, 10)

unordered_edges = r.edges()
wire_edges = r.face().outer_wire().edges()
sorted_edges = r.edges().sort_by(r.face().outer_wire())
ordered_edges = r.face().outer_wire().order_edges()

for i in range(4):
    # e1 = unordered_edges[i]
    # e2 = unordered_edges[(i + 1) % 4]
    # e1 = wire_edges[i]
    # e2 = wire_edges[(i + 1) % 4]
    e1 = sorted_edges[i]
    e2 = sorted_edges[(i + 1) % 4]
    # e1 = ordered_edges[i]
    # e2 = ordered_edges[(i + 1) % 4]

    e1_toopods_vertices = set(v.wrapped.TShape() for v in e1.vertices())
    e2_toopods_vertices = set(v.wrapped.TShape() for v in e2.vertices())
    n_common = len(set(e1_toopods_vertices).intersection(e2_toopods_vertices))
    labels.append(Pos(e1.center()) * Text(f"e{i}", 2))
    labels.append(Line(e1.center(), e2.center()))
    labels.append(Pos((e1.center() + e2.center()) / 2) * Text(f"{n_common}", 1))
labels.append(Text("# common verts", 1))

show_all()

Here I've done two things: 1) Created e1 and e2 four different ways, starting from simply extracting edges to getting ordered_edges (ordered_edges effectively goes through all these steps). 2) Created sets of the vertex TShape which is the raw OOCT object without any Location information to know if they are really the same object or not.

The results are that the first three options for creating e1 and e2 all result in the there being 1 common vertex between the edges as expected. The last option creates order edges but also potentially changes the winding order of the edge as follows:

        ordered_edges = [
            e if e.is_forward else e.reversed() for e in self.edges().sort_by(self)
        ]

resulting in only 1 pair of edges sharing a common vertex. The e.reversed() method is creating new edges with new vertices all the the direction "fixed" which is breaking the connection back to the original vertices.

Unfortunately, exploring topology is quite complex. The Shape.is_same and Shape.is_equal methods (which wrap the OCCT IsSame and IsEqual methods) might be helpful to you.

MatthiasJ1 commented 7 months ago

Reversed shouldn't be creating new edges and new vertices. OCCT has a built-in mechanism for doing this properly with Shape.Orientation