unity3d-jp / MeshSync

A package for synchronizing meshes/models editing in DCC tools into Unity in real time.
Other
1.55k stars 174 forks source link

[For 0.18.x] change: add an option to use/ignore normals for checksum calculation. #920

Closed schinkowski closed 8 months ago

schinkowski commented 11 months ago

We get normals from blender. Sometimes there are floating point errors, which cause them to be different and cause meshsync to trigger syncs when nothing changed. I tried rounding the floats to avoid this but it still happens in some cases. I think the only way we can deal with this is to not use normals for the hash check to see if meshes have changed. I believe this is acceptable because when the mesh really changes, more than just normals would change, so this should not be a problem.

sindharta-tanuwijaya commented 11 months ago

We get normals from blender. Sometimes there are floating point errors, which cause them to be different and cause meshsync to trigger syncs when nothing changed. I tried rounding the floats to avoid this but it still happens in some cases. I think the only way we can deal with this is to not use normals for the hash check to see if meshes have changed. I believe this is acceptable because when the mesh really changes, more than just normals would change, so this should not be a problem.

Hm, but what happens if the user only updates the normals of one vertice ? Does anything else change in this case ?

schinkowski commented 11 months ago

We get normals from blender. Sometimes there are floating point errors, which cause them to be different and cause meshsync to trigger syncs when nothing changed. I tried rounding the floats to avoid this but it still happens in some cases. I think the only way we can deal with this is to not use normals for the hash check to see if meshes have changed. I believe this is acceptable because when the mesh really changes, more than just normals would change, so this should not be a problem.

Hm, but what happens if the user only updates the normals of one vertice ? Does anything else change in this case ?

Nothing changes but they can always do a manual sync which would send everything anyway.

seandillon92 commented 11 months ago

This will affect all other DCC tools. Is it possible to pass a flag to control when normals are used in the checksum?

We could also add a toggle on the Blender UI panel. With a tooltip or label to explain why this is necessary. It might be confusing for users otherwise

sindharta-tanuwijaya commented 11 months ago

Nothing changes but they can always do a manual sync which would send everything anyway.

But that's not intuitive for users who just want to adjust the normals and see the shading results in real time, right ? I think it may make more sense to implement what seandillon92 suggested, with false as the default to cover the other DCC tools.

We could also add a toggle on the Blender UI panel. With a tooltip or label to explain why this is necessary. It might be confusing for users otherwise

schinkowski commented 11 months ago

Nothing changes but they can always do a manual sync which would send everything anyway.

But that's not intuitive for users who just want to adjust the normals and see the shading results in real time, right ? I think it may make more sense to implement what seandillon92 suggested, with false as the default to cover the other DCC tools.

We could also add a toggle on the Blender UI panel. With a tooltip or label to explain why this is necessary. It might be confusing for users otherwise

I've changed it so we don't use normals for blender now, there is an additional PR needed to make this work: https://github.com/Unity-Technologies/MeshSyncDCCPlugins/pull/382/files For blender the changes the user makes to the normals don't matter because we automatically calculate normals every update, which is what's causing the problem in the first place. So we also don't need to clutter up the UI with an option for this.

sindharta-tanuwijaya commented 11 months ago

This needs to go to the next minor version because there is a change in the cpp file