Open Vertices

Have some feature requests, feedback, cool stuff to share, or want to know where FreeCAD is going? This is the place.
Forum rules
Be nice to others! Read the FreeCAD code of conduct!
mopy
Posts: 20
Joined: Fri Oct 08, 2021 4:58 am

Open Vertices

Post by mopy »

Hi,

Let's say you have 3 Vertices A, B, C where A is coincident constraint to B and B is coincident constraint to C. That should mean that A is also coincident to C.
But that point is still considered an open vertex (sketch.OpenVertices)

I can't make sense of this behavior.

Regards,
Rolf
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Open Vertices

Post by openBrain »

This already has been discussed on another thread.
I think the will behind is to help user discover everything that could be a problem for extruding the sketch, including Y-junctions.

So what is your preferred solution :
* Feature do not report the multiple coincident points ?
* Button is renamed "Find issues" ?

Complaining is fine, proposing is better.
chrisb
Veteran
Posts: 53930
Joined: Tue Mar 17, 2015 9:14 am

Re: Open Vertices

Post by chrisb »

I found a related issue: Finding missing coincidences shows in the attached sketch 1 open, where 0 would be correct, and 2 would be consistent.
Bildschirmfoto 2021-10-15 um 09.39.14.png
Bildschirmfoto 2021-10-15 um 09.39.14.png (88.72 KiB) Viewed 2309 times
Attachments
openVertices.FCStd
(5.28 KiB) Downloaded 32 times
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
User avatar
ragohix769
Posts: 565
Joined: Sat Jul 18, 2020 7:04 am
Location: Rome - Italy

Re: Open Vertices

Post by ragohix769 »

openBrain wrote: Fri Oct 15, 2021 7:20 am So what is your preferred solution :
* Feature do not report the multiple coincident points ?
* Button is renamed "Find issues" ?
IMHO: feature (or some sanity check for sketcher) should be understand that coincident points are *not* open.
After #ElonMuskBuyTwitter I'm no more on Twitter, that's really enough :-(
=> Now you can find me here on #Mastodon: https://mastodon.uno/@opensoul - I hope more people do the same :-)
drmacro
Veteran
Posts: 8868
Joined: Sun Mar 02, 2014 4:35 pm

Re: Open Vertices

Post by drmacro »

Coincidence is simply not complete if the vertexes involved are just geometrically equal. While they may physically reside at the same coordinate in 3D space, unless defined as coincident, they aren't. It could be considered a shared property of the vertexes.

In fact, if they are physically co-located, but not marked coincident and they are otherwise constrained, the sketch, assuming no other rule violatons, will pad just fine. But, in this case the solver is not instructed to maintain the coincidence of the vertexes, so chaos may ensue.
Star Trek II: The Wrath of Khan: Spock: "...His pattern indicates two-dimensional thinking."
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Open Vertices

Post by wmayer »

mopy wrote: Fri Oct 15, 2021 3:44 am Let's say you have 3 Vertices A, B, C where A is coincident constraint to B and B is coincident constraint to C. That should mean that A is also coincident to C.
This is correct but the transitivity that A and C are coincident is not explicitly stored in the sketch as otherwise this will raise an error because of redundant constraints.

Code: Select all

doc = App.newDocument()
body = doc.addObject("PartDesign::Body", "Body")

sketch = body.newObject('Sketcher::SketchObject','Sketch')
sketch.Support = (doc.getObject('XY_Plane'),[''])
sketch.MapMode = 'FlatFace'

sketch.addGeometry(Part.LineSegment(App.Vector(-55.965607,-9.864289,0),App.Vector(-55.600571,-9.387639,0)),False)
sketch.addGeometry(Part.LineSegment(App.Vector(-55.735817,-9.067246,0),App.Vector(-55.600571,-9.387639,0)),False)
sketch.addConstraint(Sketcher.Constraint('Coincident',1,2,0,2)) 
sketch.addGeometry(Part.LineSegment(App.Vector(-55.600571,-9.387639,0),App.Vector(-55.058266,-9.677831,0)),False)
sketch.addConstraint(Sketcher.Constraint('Coincident',2,1,0,2)) 

# This constraint would cause a redundancy
#sketch.addConstraint(Sketcher.Constraint('Coincident',2,1,1,2)) 

doc.recompute()
But that point is still considered an open vertex (sketch.OpenVertices)
The implementation doesn't do what the function name promises:

Code: Select all


std::vector<Base::Vector3d> SketchAnalysis::getOpenVertices(void) const
{
    std::vector<Base::Vector3d> points;
    TopoDS_Shape shape = sketch->Shape.getValue();

    Base::Placement Plm = sketch->Placement.getValue();

    Base::Placement invPlm = Plm.inverse();

    // build up map vertex->edge
    TopTools_IndexedDataMapOfShapeListOfShape vertex2Edge;
    TopExp::MapShapesAndAncestors(shape, TopAbs_VERTEX, TopAbs_EDGE, vertex2Edge);
    for (int i=1; i<= vertex2Edge.Extent(); ++i) {
        const TopTools_ListOfShape& los = vertex2Edge.FindFromIndex(i);
        if (los.Extent() != 2) {
            const TopoDS_Vertex& vertex = TopoDS::Vertex(vertex2Edge.FindKey(i));
            gp_Pnt pnt = BRep_Tool::Pnt(vertex);
            Base::Vector3d pos;
            invPlm.multVec(Base::Vector3d(pnt.X(), pnt.Y(), pnt.Z()),pos);
            points.push_back(pos);
        }
    }

    return points;
}
This function finds open vertexes and non-manifold vertexes. Both are problematic for a sketch because you cannot create a face from it. Actually the function should be called Open/non-manifold vertices or we make two functions and add two buttons.
mopy
Posts: 20
Joined: Fri Oct 08, 2021 4:58 am

Re: Open Vertices

Post by mopy »

Well, the least thing to do - as always. Document the behavior.

You could do something like this to find transitive relations:

Code: Select all

cs: Set[Tuple[GeoId, GeoId]]
cn: List[Set[GeoId]] = list()
for x, y in cs:
	done = False
	for j in cn:
		if (x in j) and (y in j):
			done = True
			break
		if (x in j) or (y in j):
			j.add(x)
			j.add(y)
			done = True
			break
	if not done:
		cn.append({x, y})
self._merge_joint_sets(cn)

def _merge_joint_sets(self, cn):
	for x in range(len(cn) - 1):
		for y in range(x + 1, len(cn)):
			if y > len(cn) - 1:
				break
			if not cn[x].isdisjoint(cn[y]):
				cn[x].update(cn[y])
				cn.pop(y)
				self._merge_joint_sets(cn)
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Open Vertices

Post by openBrain »

wmayer wrote: Fri Oct 15, 2021 11:37 am
mopy wrote: Fri Oct 15, 2021 3:44 am Let's say you have 3 Vertices A, B, C where A is coincident constraint to B and B is coincident constraint to C. That should mean that A is also coincident to C.
This is correct but the transitivity that A and C are coincident is not explicitly stored in the sketch as otherwise this will raise an error because of redundant constraints.
Actually coincident constraints doesn't come into play here. Specifically, the "Highlight open vertexes" function totally ignores the constraints.
It applies a simple rule that is "Highlight points where not exactly 2 vertices strictly are", which also means :
* It doesn't highlight point pairs having strictly equal coordinates, even if no coincidence is set between both
* It totally ignore the Tolerance value set above (which from UI point of view is a bit weird)
chrisb
Veteran
Posts: 53930
Joined: Tue Mar 17, 2015 9:14 am

Re: Open Vertices

Post by chrisb »

Here is a slightly modified sketch. It correctly pads perfectly, yet Validate Sketch shows a missing coincidence. It can of course be ignored here, and I prefer to have false positives instead of missed hits, but if there are a lot more of really missing coincidences they can only be fixed automatically when this one is (mal-)fixed too - which then needs editing in sketcher again.

Surprisingly by making the diagonal construction geometry the false Open vertex vanishes.

I think it would at least be worth a feature request, if not a minor bug report.
Attachments
openVertices.FCStd
(5.25 KiB) Downloaded 37 times
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
chrisb
Veteran
Posts: 53930
Joined: Tue Mar 17, 2015 9:14 am

Re: Open Vertices

Post by chrisb »

openBrain wrote: Fri Oct 15, 2021 1:07 pm It applies a simple rule that is "Highlight points where not exactly 2 vertices strictly are", which also means :
You probably looked into the implementation? Then it is a precision issue, where equality isn't necessarily transitive.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
Post Reply