lost functionality - GeomBSplineSurface::Restore / Save

Need help, or want to share a macro? Post here!
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Syres
Veteran
Posts: 2893
Joined: Thu Aug 09, 2018 11:14 am

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by Syres »

@emills2 I've narrowed the error on loading Silk_Deom_03.FCStd to 1500 commits.

Last error free build loading demo file (unfortunately no hash was given for that build):

Code: Select all

OS: Windows 7 SP 1 (6.1)
Word size of FreeCAD: 64-bit
Version: 0.20.27518 (Git)
Build type: Release
Python version: 3.8.12
Qt version: 5.12.9
Coin version: 4.0.0
OCC version: 7.5.3
Locale: English/United Kingdom (en_GB)
first build with error loading demo file:

Code: Select all

OS: Windows 7 SP 1 (6.1)
Word size of FreeCAD: 64-bit
Version: 0.20.29045 (Git)
Build type: Release
Branch: master
Hash: af811b34b3 (June 2022)
Python 3.8.13, Qt 5.12.9, Coin 4.0.0, OCC 7.5.3
Locale: English/United Kingdom (en_GB)
So the best I can do is firstly work out approx hash for good build and compile on Linux to be sure. Then I'll have to do a Git Bisect between the two builds which will be a long job on my system but I'll try and give you a commit in the next couple of days.
emills2
Posts: 868
Joined: Tue Apr 28, 2015 11:23 pm

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by emills2 »

Syres wrote: Mon Mar 27, 2023 9:22 am So the best I can do is firstly work out approx hash for good build and compile on Linux to be sure. Then I'll have to do a Git Bisect between the two builds which will be a long job on my system but I'll try and give you a commit in the next couple of days.
Thanks for the detective work Syres. Like i said, it'd be nice to get the old functionality back, but i'd be fine with modifying my methods as well to fit the new save/restore system. I think in general it would be nice to be able to handle multiple surface patches within one shape for python feature. From Werner's comment it seems like that is already in the long term plan, just not implemented yet.

i'm wondering if perhaps i could delete/reset the attributes after computing the shape, so that the python feature doesn't have to try to save something awkward (i already do this with some grids to convert lists of Base.Vector to lists of floats, so that they save properly). but in this case, if i clear the Shape, wouldn't the object also disappear from the doc? I haven't tried yet, in case there was a recommended way to do it.

I haven't been very active with FreeCAD the last two years, but a user brought the issue to my attention, and it seemed worth investigating.

Thanks again.
wmayer
Founder
Posts: 20241
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by wmayer »

emills2 wrote: Sun Mar 26, 2023 11:01 pm
wmayer wrote: Sun Mar 26, 2023 10:44 pm https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L4544

Save & Restore is not yet implemented for any surface geometry types.
Is this "Save & Restore" something new in the last few years, which wasn't needed before? and somehow before it just worked?
I guess the problem must be Part::PropertyGeometryList that is has become less permissive. Actually the only use case of Part::PropertyGeometryList is the sketcher and this only works with points or curves but no surfaces and thus there was no pressure to implement Save() & Restore for the surface types.

So, when you say this has worked in the past then my guess is that Part::PropertyGeometryList silently ignored it when a geometry object hasn't implemented some functions.

Now throwing and not handling an exception causes the loading/saving mechanism to leave some written data in an inconsistent state. To fix this issue in a very first step the Part::PropertyGeometryList must become permissive to simply ignore a geometry when it raises a NotImplementedError.
In a second step all the missing Save/Restore methods must be implemented.
wmayer
Founder
Posts: 20241
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by wmayer »

OK found it. It's not that Part::PropertyGeometryList has changed its behaviour but that the surface geometries now raise an exception. Before that they did an assert(0) which depending on compiler flags caused a hard crash.

The changes are in git commit 049d8ae06c17

So, as said in the previous post the class Part::PropertyGeometryList should permit it that a geometry type doesn't implement Save/Restore.
emills2
Posts: 868
Joined: Tue Apr 28, 2015 11:23 pm

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by emills2 »

wmayer wrote: Mon Mar 27, 2023 1:02 pm OK found it. It's not that Part::PropertyGeometryList has changed its behaviour but that the surface geometries now raise an exception. Before that they did an assert(0) which depending on compiler flags caused a hard crash.

The changes are in git commit 049d8ae06c17

So, as said in the previous post the class Part::PropertyGeometryList should permit it that a geometry type doesn't implement Save/Restore.
Wonderful. thank Werner.

[edit] is this in master and will it eventually show up in weekly builds?
Syres
Veteran
Posts: 2893
Joined: Thu Aug 09, 2018 11:14 am

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by Syres »

I've asked uwe to backport this fix so it's in 0.20.3
emills2
Posts: 868
Joined: Tue Apr 28, 2015 11:23 pm

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by emills2 »

Syres wrote: Mon Mar 27, 2023 5:12 pm I've asked uwe to backport this fix so it's in 0.20.3
Thanks Syres, does that mean i'm better off with standard build? or should i keep updating my weekly build install?
wmayer
Founder
Posts: 20241
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by wmayer »

edit] is this in master and will it eventually show up in weekly builds?
I have merged the PR into master.
emills2
Posts: 868
Joined: Tue Apr 28, 2015 11:23 pm

Re: lost functionality - GeomBSplineSurface::Restore / Save

Post by emills2 »

wmayer wrote: Mon Mar 27, 2023 5:44 pm I have merged the PR into master.
Thanks Werner, i never cease to be amazed at the speed of response when i run into issues :) have a great day
Post Reply