sketch Validation tool bug

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help

Also, be nice to others! Read the FreeCAD code of conduct!
User avatar
easyw-fc
Posts: 2611
Joined: Thu Jul 09, 2015 9:34 am

Re: sketch Validation tool bug

Postby easyw-fc » Thu Nov 15, 2018 4:32 pm

abdullah wrote:
Thu Nov 15, 2018 4:02 pm
It is not really zero length, one can not create such line segment.

The problem is that the length is greater than gp::Resolution(), but higher than the resolution we use to save files.

.....

EDIT: gp::Resolution() is 2.22e-308, I am positive we cannot increase the resolution of saving to that value. Currently the resolution for saving is 1e-12.
I wrote zero length but I meant length less than a resolution I use to kicad (which is something like 10 micron if I remember correctly)...

A nearly zero length could be generated also from the validation tool or from constraintnator..
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: sketch Validation tool bug

Postby abdullah » Thu Nov 15, 2018 4:41 pm

easyw-fc wrote:
Thu Nov 15, 2018 4:32 pm
I wrote zero length but I meant length less than a resolution I use to kicad (which is something like 10 micron if I remember correctly)...

A nearly zero length could be generated also from the validation tool or from constraintnator..
This is a good point. Depending on the solution implemented, two things may happen:
a) the validation algorithm or constrainator will receive an exception, they will need to learn to handle it. The good part is that as Exceptions are getting specialised, these algos may handle it the problem properly.
b) we may need to add a new validation routine to "fix" this when it happens.
User avatar
easyw-fc
Posts: 2611
Joined: Thu Jul 09, 2015 9:34 am

Re: sketch Validation tool bug

Postby easyw-fc » Thu Nov 15, 2018 5:58 pm

abdullah wrote:
Thu Nov 15, 2018 4:41 pm
This is a good point. Depending on the solution implemented, two things may happen:
a) the validation algorithm or constrainator will receive an exception, they will need to learn to handle it. The good part is that as Exceptions are getting specialised, these algos may handle it the problem properly.
b) we may need to add a new validation routine to "fix" this when it happens.
Is there a way to strip out a geometry from a sketch through python?
ATM when I sanitize a sketch I need to copy all the geometries excluding the 'nearly zero length' geos to a new sketch, creating it from the sanitized geo. I also have difficulties on how to handle a copy of constraints, if these are related to a geo to be removed.
wmayer
Site Admin
Posts: 14585
Joined: Thu Feb 19, 2009 10:32 am

Re: sketch Validation tool bug

Postby wmayer » Thu Nov 15, 2018 6:24 pm

A line with two identical endpoints (zero length line) is saved onto a file (they are not fully identical, but yes within the tolerance we save). When restoring, this line which is part of a sketch, throws an exception and the whole sketch is lost.
At the time when the sketch was saved it must have been a valid geometry because otherwise OCCT had thrown an error that it cannot create the line segment. Since saving & loading a project should restore the same geometry the save mechanism is incorrect when it writes out double values with too few decimals. So this should be fixed.

However, when restoring from a broken project then a geometry which OCCT fails to create should not abort the whole reading process but only the creation of this element and then continue with the next. It's always better to restore at least something than nothing.
Things that I can think of:
1. PREVENT: Check at saving that two points are at least within the tolerance gp::Resolution(), if not
1.1. Move the endpoint so that the distance is gp:Resolution() AND
1.2. Warn the user somehow?
IMO this is superfluous because the fact that OCCT created the geometry means that it must be valid and cannot be degenerated.
The logical solution is either not to allow to create such segment (so one gets an error on creation), or to actually increase the resolution in the xml file.
When looking at float.h then DBL_EPSILON is defined as 2.2204460492503131e-016 (smallest such that 1.0+DBL_EPSILON != 1.0) and this means that ZipStream.precision must be set to 16.
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: sketch Validation tool bug

Postby abdullah » Thu Nov 15, 2018 6:24 pm

easyw-fc wrote:
Thu Nov 15, 2018 5:58 pm
abdullah wrote:
Thu Nov 15, 2018 4:41 pm
This is a good point. Depending on the solution implemented, two things may happen:
a) the validation algorithm or constrainator will receive an exception, they will need to learn to handle it. The good part is that as Exceptions are getting specialised, these algos may handle it the problem properly.
b) we may need to add a new validation routine to "fix" this when it happens.
Is there a way to strip out a geometry from a sketch through python?
ATM when I sanitize a sketch I need to copy all the geometries excluding the 'nearly zero length' geos to a new sketch, creating it from the sanitized geo. I also have difficulties on how to handle a copy of constraints, if these are related to a geo to be removed.
App.ActiveDocument.Sketch.delGeometry(9)

But apparently there are more geometries with problems that number 9.
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: sketch Validation tool bug

Postby abdullah » Thu Nov 15, 2018 6:38 pm

wmayer wrote:
Thu Nov 15, 2018 6:24 pm
When looking at float.h then DBL_EPSILON is defined as 2.2204460492503131e-016 (smallest such that 1.0+DBL_EPSILON != 1.0) and this means that ZipStream.precision must be set to 16.
wmayer wrote:
Thu Nov 15, 2018 6:24 pm
However, when restoring from a broken project then a geometry which OCCT fails to create should not abort the whole reading process but only the creation of this element and then continue with the next. It's always better to restore at least something than nothing.
Duly noted the change in ZipStream. Thanks for the verbose output that you got it from DBL_EPSILON.

About the failure on restore, I thought exactly about that. There is however a big problem with sketches, because a shifted geometry will create a mess of constraints.

Blutly, for everything else probably the best option is not to restore it and just "continue". For a Sketch, the best option is to create something in place. Something similar is probably the safest.

Now, probably you understand where I am coming from with those crazy ideas.

One idea to solve this could be to create a new Exception "GeometryRestoreError". This could be used later at Sketch level to create a placeholder geometry.

How does it sound?
User avatar
easyw-fc
Posts: 2611
Joined: Thu Jul 09, 2015 9:34 am

Re: sketch Validation tool bug

Postby easyw-fc » Thu Nov 15, 2018 9:13 pm

abdullah wrote:
Thu Nov 15, 2018 6:24 pm
App.ActiveDocument.Sketch.delGeometry(9)
thx :D
abdullah wrote:
Thu Nov 15, 2018 6:24 pm
But apparently there are more geometries with problems that number 9.
9 & 11 it seems are 'nearly zero length'
I cannot find any more issues

a small tricky tool for my constrainator ;)

Code: Select all

def sanitizeSketch():
    ''' simplifying & sanitizing sketches '''
    edge_tolerance  = 0.01
    
    doc = FreeCAD.ActiveDocument
    sel = FreeCADGui.Selection.getSelection()
    if len(sel)==1:
        if 'Sketcher' in sel[0].TypeId:
            idx_to_del=[]
            for i,g in enumerate (sel[0].Geometry):
                #print(g,i)
                if 'Line' in str(g):
                    #print(g.length())
                    if g.length() <= edge_tolerance: # 0.1:
                        print(g,i,'too short')
                        idx_to_del.append(i)
            j=0
            if len(idx_to_del) >0:
                print(u'sanitizing '+sel[0].Label)
            for i in idx_to_del:
                sel[0].delGeometry(i-j)
                j+=1
User avatar
easyw-fc
Posts: 2611
Joined: Thu Jul 09, 2015 9:34 am

Re: sketch Validation tool bug

Postby easyw-fc » Thu Nov 15, 2018 9:27 pm

abdullah wrote:
Thu Nov 15, 2018 6:38 pm
How does it sound?
a feature request if I can...
'cause you are working on the sketcher, would it be possible to expose the Validation tools (find missing coincidences and find open vertexes) to python?
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: sketch Validation tool bug

Postby abdullah » Fri Nov 16, 2018 4:27 am

easyw-fc wrote:
Thu Nov 15, 2018 9:13 pm
a small tricky tool for my constrainator ;)

Code: Select all

def sanitizeSketch():
    ''' simplifying & sanitizing sketches '''
    edge_tolerance  = 0.01
    
    doc = FreeCAD.ActiveDocument
    sel = FreeCADGui.Selection.getSelection()
    if len(sel)==1:
        if 'Sketcher' in sel[0].TypeId:
            idx_to_del=[]
            for i,g in enumerate (sel[0].Geometry):
                #print(g,i)
                if 'Line' in str(g):
                    #print(g.length())
                    if g.length() <= edge_tolerance: # 0.1:
                        print(g,i,'too short')
                        idx_to_del.append(i)
            j=0
            if len(idx_to_del) >0:
                print(u'sanitizing '+sel[0].Label)
            for i in idx_to_del:
                sel[0].delGeometry(i-j)
                j+=1
Sure we can add this to constrainator routines. It does not solve the problem with serialisation, but it may help not arriving to the problem.
easyw-fc wrote:
Thu Nov 15, 2018 9:27 pm
a feature request if I can...
'cause you are working on the sketcher, would it be possible to expose the Validation tools (find missing coincidences and find open vertexes) to python?
the missing coincidence detection is part of constrainator:
ActiveSketch.detectMissingPointOnPointConstraints

The open vertex is not there yet.
issue #3693
abdullah
Posts: 3174
Joined: Sun May 04, 2014 3:16 pm

Re: sketch Validation tool bug

Postby abdullah » Fri Nov 16, 2018 6:31 am

wmayer wrote:
Thu Nov 15, 2018 6:24 pm
However, when restoring from a broken project then a geometry which OCCT fails to create should not abort the whole reading process but only the creation of this element and then continue with the next. It's always better to restore at least something than nothing.
I am working towards this. How does this sound to you?
https://github.com/abdullahtahiriyo/Fre ... l/new/3690

Basically:
1. New exception RestoreError, to note that it was a problem to recover an element and a best effort was made.
2. Make App::Property be aware of whether it should disregard the element or add it (relevant for sketches).
NOTE: I am still missing to set this to true in SketchObject.cpp
3. Make elements do a best effort to recover, only implemented to LineSegment
4. Enable lists, in this case only implemented for PropertyGeometryList to do a partial recover on certain errors

If this would be acceptable, I have another question:

Look at the end of void PropertyGeometryList::Restore(Base::XMLReader &reader)

I am trying to convey to the user a message that the Restore was not complete, so that after restore, eventually a pop-up would appear warning the user.

Doing a THROW at each function in the stack (see stack in the previous post), while allowing to continue restoring is not efficient the way I am imagining it (a bool that throws an exception on each function only to catch it in the caller and set another boolean and throw another exception at the end of the function).

Would it be possible to make Restore return an int or a boolean, so that we can mark that a partial restore took place?

Thanks in advance.