Intra v0.19 File Loading Problems with Cosmetic Items

Discussions about the development of the TechDraw workbench
User avatar
wandererfan
Posts: 3144
Joined: Tue Nov 06, 2012 5:42 pm

Intra v0.19 File Loading Problems with Cosmetic Items

Postby wandererfan » Wed Nov 06, 2019 12:49 pm

I have introduced a file incompatibility between involving CosmeticVertex/CosmeticEdges/Centerlines. You may get "invalid XML errors" if loading a file from before approx mid Oct with the current software.

Files written with the current software should restore with the current software.
Files written with the current software should restore with older software.
Files written with older software will not restore with the current version.

File that don't use CosmeticVertex/CosmeticEdge/CenterLines/ExtentDimensions should be unaffected.

I don't have a conversion strategy at the moment.

The change that introduced the incompatibility was required to fix lost references to existing cosmetic items after cosmetic items were added/removed (thereby changing the index of the cosmetic item in the list).

Sorry folks.

wf
aapo
Posts: 93
Joined: Mon Oct 29, 2018 6:41 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby aapo » Wed Nov 06, 2019 7:29 pm

wandererfan wrote:
Wed Nov 06, 2019 12:49 pm
I have introduced a file incompatibility between involving CosmeticVertex/CosmeticEdges/Centerlines. You may get "invalid XML errors" if loading a file from before approx mid Oct with the current software.

I don't have a conversion strategy at the moment.
I don't think that the version inconsistency or lack of conversion matters much, at least not for me. In practice, if the Cosmetic Items fail, it's rather simple to delete them (with a working FreeCAD version), and then re-add them with the newest version. I don't mind re-adding a few dimensions every now and then, that happens a lot anyway; due to the infamous Toponaming issue. However, I'd be grateful for a fix to the crash when loading an offending file. IMO, it would be enough to detect and delete any offending Cosmetic Items at load time, re-adding them is really a rather small problem from user perspective.

Sorry folks.
Thanks for your continued efforts with improving TechDraw. At least for me, I've saved a ton of effort by using the new features you've developed, and I hope other users are also as grateful about them as I am. So, thanks a lot! :D
chrisb
Posts: 19505
Joined: Tue Mar 17, 2015 9:14 am

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby chrisb » Wed Nov 06, 2019 8:57 pm

Fully agree with all you said.
vocx
Posts: 1850
Joined: Thu Oct 18, 2018 9:18 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby vocx » Wed Nov 06, 2019 11:05 pm

wandererfan wrote:
Wed Nov 06, 2019 12:49 pm
...
Sorry folks.
My opinion is that this is not a problem. As long as it is within a development version, breaking compatibility like this is to be expected.

Once the new tool appears on a released version, then we should be much more careful about introducing and managing such incompatibilities. But within the development version? Nah, breaking it is fine.
User avatar
wandererfan
Posts: 3144
Joined: Tue Nov 06, 2012 5:42 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby wandererfan » Thu Nov 07, 2019 1:36 am

vocx wrote:
Wed Nov 06, 2019 11:05 pm
Once the new tool appears on a released version, then we should be much more careful about introducing and managing such incompatibilities.
v0.19 is the first version with Cosmetic features, so we're safe there.

If anybody has a lot of work tied up in a file that won't load, let me know. I've put in a temporary backdoor to allow skipping Cosmetic features when loading a document.
User avatar
wandererfan
Posts: 3144
Joined: Tue Nov 06, 2012 5:42 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby wandererfan » Thu Nov 07, 2019 1:44 am

aapo wrote:
Wed Nov 06, 2019 7:29 pm
However, I'd be grateful for a fix to the crash when loading an offending file. IMO, it would be enough to detect and delete any offending Cosmetic Items at load time, re-adding them is really a rather small problem from user perspective.
It isn't so much the items that are a problem, but the new attributes. The file loader starts looking for, say, "CLPoints" in Document.xml, then runs right to the end of the file because "CLPoints" isn't there.

To load a file with the old Cosmetics, create a "restoreCosmetic" parameter and set it to "false' as below. Remember to set it back to "true" (or delete it) after loading.
restoreCosmetic.png
restoreCosmetic.png (92.48 KiB) Viewed 185 times
aapo
Posts: 93
Joined: Mon Oct 29, 2018 6:41 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby aapo » Thu Nov 07, 2019 7:02 am

wandererfan wrote:
Thu Nov 07, 2019 1:44 am
It isn't so much the items that are a problem, but the new attributes. The file loader starts looking for, say, "CLPoints" in Document.xml, then runs right to the end of the file because "CLPoints" isn't there.

To load a file with the old Cosmetics, create a "restoreCosmetic" parameter and set it to "false' as below. Remember to set it back to "true" (or delete it) after loading.
Thanks, that'll certainly make it easier to deal with the old files. I looked at the code in your pull request, and maybe I understood it wrong; but won't deleting the "restoreCosmetic" parameter cause the function there to return 'false', and to not load the Cosmetics if the parameter is deleted? To me it seems that the parameter must be explicitly set to 'true' in order to restore the Cosmetics.

EDIT: Sorry, nevermind. The 'bool bPreset' is correctly set to 'true' in your code. I was sent off the track as the function GetBool() expects 'true' or 'false' as the preset parameter, and you wrote '1l' instead, but I suppose that after implicit conversion it should work as intended.
User avatar
wandererfan
Posts: 3144
Joined: Tue Nov 06, 2012 5:42 pm

Re: Intra v0.19 File Loading Problems with Cosmetic Items

Postby wandererfan » Thu Nov 07, 2019 11:48 am

aapo wrote:
Thu Nov 07, 2019 7:02 am
I was sent off the track as the function GetBool() expects 'true' or 'false' as the preset parameter, and you wrote '1l' instead, but I suppose that after implicit conversion it should work as intended.
The dangers of cut/paste on old code. Thanks for catching.