object_skinify.py generates different results between Operating Systems Platforms #52536

New Issue

Albert · 2017-08-25T14:37:53+02:00

Albert commented

2017-08-25 14:37:53 +02:00

skinify_rigify.blend

Due to add-on's defaults on startup enabling "Solid shape" and "Fill gaps" on provided file is required.

System Information
Windows 10 64, Linux Ubuntu 64

Blender Version
2.79RC2 /object_skinify.py 0.9.2

Short description of error
skinify add-on generates good or corrupted mesh depending on host OS.

Exact steps for others to reproduce the error
create new rigify metarig, enter the pose mode, select all the bones. Go to Bone in the Property Panel, Select Solid Shape and Fill the Gaps on the Skinify Panel, press Add Shape.

As a result generated edges on different OS will have different vert idx.

Edges are generated based on armature traversal. Redundant verts are being merged by hardcoded idx before the skin modiifer is applied.

[skinify_rigify.blend](https://archive.blender.org/developer/F725250/skinify_rigify.blend) Due to add-on's defaults on startup enabling "Solid shape" and "Fill gaps" on provided file is required. **System Information** Windows 10 64, Linux Ubuntu 64 **Blender Version** 2.79RC2 /object_skinify.py 0.9.2 **Short description of error** skinify add-on generates good or corrupted mesh depending on host OS. **Exact steps for others to reproduce the error** create new rigify metarig, enter the pose mode, select all the bones. Go to Bone in the Property Panel, Select Solid Shape and Fill the Gaps on the Skinify Panel, press Add Shape. As a result generated edges on different OS will have different vert idx. Edges are generated based on armature traversal. Redundant verts are being merged by hardcoded idx before the skin modiifer is applied.

Albert commented

2017-08-25 14:37:53 +02:00

Changed status to: 'Open'

Albert commented

2017-08-25 14:37:53 +02:00

Added subscriber: @karab44

Brendon Murphy commented

2017-08-26 03:49:00 +02:00

Added subscriber: @BrendonMurphy

Brendon Murphy commented

2017-08-26 03:49:00 +02:00

hi, basically karab44 wrote skinify 0.89 on windows and wrote 0.90 on linux, causing a platform depend somehow?

that said, 0.89 is claimed not to work on linux either: 0.89 works win, 0.90 works with linux

I back tested 0.90 and issues exist in older builds, i forward ported 0.89 and it's good on rc2 windows
I only can test on win.
the solid shape code was changed between versions in the addon, the solid shape button causes the failed geometry especially in hands.

The animal rigify skin fine, rigify rigs fail, manual bastioni rigs fail with skinify: improper skinning for hands and arm taper towards wrist.
*so i could rule out if it was issue with rigify i tested with manual bastioni human lab and had same issues: rule out skinify interaction with rigify was broken.
I also checked bf commit logs, not really anything there to cause this between my test versions 9cd6b03 and rc2
i bisected to a possible commit, but then, the forward porting worked and backporting before the possible commit still failed, so i ruled that out.
here's diff of both versions: https://developer.blender.org/rBA8a3e171dabe636e089ed1f7beb2c09a101ab2afb

thanks

hi, basically karab44 wrote skinify 0.89 on windows and wrote 0.90 on linux, causing a platform depend somehow? that said, 0.89 is claimed not to work on linux either: 0.89 works win, 0.90 works with linux I back tested 0.90 and issues exist in older builds, i forward ported 0.89 and it's good on rc2 windows I only can test on win. the solid shape code was changed between versions in the addon, the solid shape button causes the failed geometry especially in hands. The animal rigify skin fine, rigify rigs fail, manual bastioni rigs fail with skinify: improper skinning for hands and arm taper towards wrist. *so i could rule out if it was issue with rigify i tested with manual bastioni human lab and had same issues: rule out skinify interaction with rigify was broken. I also checked bf commit logs, not really anything there to cause this between my test versions 9cd6b03 and rc2 i bisected to a possible commit, but then, the forward porting worked and backporting before the possible commit still failed, so i ruled that out. here's diff of both versions: https://developer.blender.org/rBA8a3e171dabe636e089ed1f7beb2c09a101ab2afb thanks

Brendon Murphy commented

2017-08-26 03:51:51 +02:00

Added subscriber: @ideasman42

Brendon Murphy commented

2017-08-26 03:51:51 +02:00

@ideasman42 any thoughts here?

Campbell Barton commented

2017-08-26 05:02:45 +02:00

The report has too many steps, at least narrow this down so we can tell if it's Blender or the script.

It looks like the code from 8a3e171dab depends on order in dictionary, I've asked @karab44 on IRC to try using PYTHONHASHSEED for deterministic hashing, but didn't hear back on this.

When looping over a dictionary, you cant expect predictable outcome, try putting calls to dict.keys/items/values inside a sorted(...).

Sorted dictionaries only work if memory locations arent used as keys, so if this doesn't help, best prepare the data from the script, before modifiers run as a static blend file, then we can run the modifier on it and see if its giving different results depending on the system.

The report has too many steps, at least narrow this down so we can tell if it's Blender or the script. It looks like the code from 8a3e171dab depends on order in dictionary, I've asked @karab44 on IRC to try using [PYTHONHASHSEED ](https://docs.python.org/3.5/using/cmdline.html#envvar-PYTHONHASHSEED) for deterministic hashing, but didn't hear back on this. When looping over a dictionary, you cant expect predictable outcome, try putting calls to `dict.keys/items/values` inside a `sorted(...)`. Sorted dictionaries only work if memory locations arent used as keys, so if this doesn't help, best prepare the data from the script, before modifiers run as a static blend file, then we can run the modifier on it and see if its giving different results depending on the system.

Albert commented

2017-08-29 12:55:42 +02:00

@ideasman42 can not confirm script bug at the moment. PYTHONHASHSEED and DETERMINISTIC_BUILD does not help nor OrderedDict.

So far provided cross platform solution. Time will show is it final fix or workaround.

Best Regards!
Albert

@ideasman42 can not confirm script bug at the moment. PYTHONHASHSEED and DETERMINISTIC_BUILD does not help nor OrderedDict. So far provided cross platform solution. Time will show is it final fix or workaround. Best Regards! Albert

Albert commented

2017-08-29 12:57:33 +02:00

object_skinify0899.py

including missing file from previous post

[object_skinify0899.py](https://archive.blender.org/developer/F738510/object_skinify0899.py) including missing file from previous post

blender-admin commented

2017-08-29 13:09:14 +02:00

This issue was referenced by eea66a6388

This issue was referenced by eea66a6388cdbf220a5b1fd03299a30415b1e0f2

blender-admin commented

2017-08-29 13:09:14 +02:00

This issue was referenced by 0ffb0c5d5d

This issue was referenced by 0ffb0c5d5d54a90007387973dd49afc98fb95bef

Brendon Murphy commented

2017-08-29 13:11:27 +02:00

Changed status from 'Open' to: 'Resolved'

Brendon Murphy closed this issue

2017-08-29 13:11:27 +02:00

Campbell Barton commented

2017-08-30 04:38:29 +02:00

This seems like a fragile workaround - does it work with 32 bit systems? seems like someone using BSD's will get some empty mesh or error too.

Would rather see this simplified into a static blend file with that a developer can check.

This seems like a fragile workaround - does it work with 32 bit systems? seems like someone using BSD's will get some empty mesh or error too. Would rather see this simplified into a static blend file with that a developer can check.

Campbell Barton commented

2017-08-30 04:41:31 +02:00

Changed status from 'Resolved' to: 'Open'

Campbell Barton reopened this issue

2017-08-30 04:41:31 +02:00

Brendon Murphy commented

2017-09-01 11:17:42 +02:00

hi, I will supply a .blend, after talking with @karab44 , it will have the 2 skinify scripts (this revision and the previous broken revision) in the text editor and 2 rigs for testing.
I'll set it up tonight and supply tomorrow.
thanks

hi, I will supply a .blend, after talking with @karab44 , it will have the 2 skinify scripts (this revision and the previous broken revision) in the text editor and 2 rigs for testing. I'll set it up tonight and supply tomorrow. thanks

Campbell Barton commented

2017-09-02 06:39:36 +02:00

blender/blender@2ca1f29748 may fix the issue.
8a3e171dab will need to be reverted first.

blender/blender@2ca1f29748 may fix the issue. 8a3e171dab will need to be reverted first.

Brendon Murphy commented

2017-09-03 14:01:34 +02:00

hi, @karab44 can you test this please.
@ideasman42
to test this addon is simple.
create a rigify rig "Human" (Meta Rig)
This result is from RC2 on win64 with current version of skinify, which is tested as working on the 3 major platforms. win, osx, linux 64bit.
run skinify with the following settings:

This result if from nightly build fcd7ac1 with the same settings:

I'm not sure if blender/blender@2ca1f29748 has caused this new issue.
I tested with the old version 0.8.9 with the same result.

A note:
this was a fun addon with many uses, proxy mesh for characters, test mesh for animations, quick set up, easy enough to use for all user levels.
we tested then found the cross platform issue ourselves, then in irc found testers, I'm happy to add a warning to the current version that "may not work as expected".
The issues is the hand mesh, and issue in old addon was the platform compatibility. We have solved these issues in the version 0.9.9 of skinify addon. Whilst the code may not be perfect, the times where it may fail are limited to hand rigs and the addon will work as expected in the majority of cases.

Can we just keep this working and add a warning.
Thanks.

hi, @karab44 can you test this please. @ideasman42 to test this addon is simple. create a rigify rig "Human" (Meta Rig) This result is from RC2 on win64 with current version of skinify, which is tested as working on the 3 major platforms. win, osx, linux 64bit. run skinify with the following settings: ![skinnify_v099_rc2.jpg](https://archive.blender.org/developer/F769183/skinnify_v099_rc2.jpg) This result if from nightly build fcd7ac1 with the same settings: ![skinnify_v099_sunday_3rdsept.jpg](https://archive.blender.org/developer/F769209/skinnify_v099_sunday_3rdsept.jpg) I'm not sure if blender/blender@2ca1f29748 has caused this new issue. I tested with the old version 0.8.9 with the same result. A note: this was a fun addon with many uses, proxy mesh for characters, test mesh for animations, quick set up, easy enough to use for all user levels. we tested then found the cross platform issue ourselves, then in irc found testers, I'm happy to add a warning to the current version that "may not work as expected". The issues is the hand mesh, and issue in old addon was the platform compatibility. We have solved these issues in the version 0.9.9 of skinify addon. Whilst the code may not be perfect, the times where it may fail are limited to hand rigs and the addon will work as expected in the majority of cases. Can we just keep this working and add a warning. Thanks.

Albert commented

2017-09-03 18:00:04 +02:00

hello!

@BrendonMurphy no worry, all good.

New results are invalid just because the result was received on the setup and calculations on the 'outdated/invalid' data(triggered by the cpp code update). The upcoming updated script should work fine for all platforms with the script data based on linux idx input only.(win and darwin input removed)

So the current script requires to update the idx data for all platforms to be set exactly the same as current idx settings for the linux OS .
However this should be additionally tested for all platforms.

I've been waiting with the script update untill all cross platform builds are being available for all architectures so expect the script update based on fixed solution soon.

Best Regards!
Albert

hello! @BrendonMurphy no worry, all good. New results are invalid just because the result was received on the setup and calculations on the 'outdated/invalid' data(triggered by the cpp code update). The upcoming updated script should work fine for all platforms with the script data based on linux idx input only.(win and darwin input removed) So the current script requires to update the idx data for all platforms to be set exactly the same as current idx settings for the linux OS . However this should be additionally tested for all platforms. I've been waiting with the script update untill all cross platform builds are being available for all architectures so expect the script update based on fixed solution soon. Best Regards! Albert

Albert commented

2017-09-03 18:29:34 +02:00

@BrendonMurphy
blender/blender@2ca1f29748

it's not an issue, it's the solution but the script relies on outdated/invalid input.

New update will no longer be platform dependent anymore. I'll update the script on Monday (2moro)

@BrendonMurphy blender/blender@2ca1f29748 it's not an issue, it's the solution but the script relies on outdated/invalid input. New update will no longer be platform dependent anymore. I'll update the script on Monday (2moro)

Campbell Barton commented

2017-09-03 18:37:02 +02:00

@karab44 note that master now uses KD-tree for remove doubles, see: blender/blender@459d76ec51 blender/blender@4d148471b6