Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Manifold Decompose doesn't preserve vertex properties #786

Closed
harmanpa opened this issue Apr 9, 2024 · 4 comments · Fixed by #794
Closed

Manifold Decompose doesn't preserve vertex properties #786

harmanpa opened this issue Apr 9, 2024 · 4 comments · Fixed by #794
Labels
bug Something isn't working

Comments

@harmanpa
Copy link
Contributor

harmanpa commented Apr 9, 2024

Calling Decompose on a Manifold made up of multiple disconnected meshes divides it into the separate Manifolds, but only containing vertex positions and not including other vertex properties if defined (unless I'm mistaken?).
@pca006132
Copy link
Collaborator

Probably a bug because decompose was there before vertex properties.

@pca006132 pca006132 added the bug Something isn't working label Apr 10, 2024
@harmanpa
Copy link
Contributor Author

I'd be happy to try to implement and test a solution, do you know of anywhere in the code that also filters the vertex properties by index that might be similar @pca006132 ?

@elalish
Copy link
Owner

elalish commented Apr 13, 2024

That would be excellent, thanks! It might help to look at CompactProps.

If you get excited about cleaning up Decompose, there's another issue open about it: #745 (comment). Ideally it shouldn't take too much to make Manifold::Decompose act more like CrossSection::Decompose

@harmanpa
Copy link
Contributor Author

It turned out that the only cause of this was that meshRelation_.numProp was not set in Manifold::Impl::GatherFaces while the vertProperties and properties were. However I've added a test and associated test utility (PR incoming) to check the properties in a resulting mesh match those set via SetProperties.

harmanpa added a commit to harmanpa/manifold that referenced this issue Apr 25, 2024
@harmanpa
Fix for elalish#786 by setting numProps in GatherFaces method. Added …
03848fa 
…test for Decompose with properties, and ExpectProperties function in test_main in order to check properties in a mesh match those set by SetProperties.
@harmanpa harmanpa mentioned this issue Apr 25, 2024
Merged
harmanpa added a commit to harmanpa/manifold that referenced this issue Apr 26, 2024
@harmanpa
Revert "Fix for elalish#786 by setting numProps in GatherFaces method…
86e6d42 
…. Added test for Decompose with properties, and ExpectProperties function in test_main in order to check properties in a mesh match those set by SetProperties."

This reverts commit 03848fa.
elalish pushed a commit that referenced this issue Apr 26, 2024
@harmanpa
Ensure properties maintained through Decompose operation (#794)
c9810fd 
* Fix for #786 by setting numProps in GatherFaces method. Added test for Decompose with properties, and ExpectProperties function in test_main in order to check properties in a mesh match those set by SetProperties.

* Correct formatting of ExpectProperties

* Correct more formatting

* Removed ExpectProperties and instead use existing test methods

* Call RelatedGL for each Decomposed Manifold
elalish pushed a commit that referenced this issue May 3, 2024
@harmanpa
C bindings: pass context to set_properties and warp operation callbac…
5301542 
…ks (#795)

* Fix for #786 by setting numProps in GatherFaces method. Added test for Decompose with properties, and ExpectProperties function in test_main in order to check properties in a mesh match those set by SetProperties.

* Correct formatting of ExpectProperties

* Correct more formatting

* Add x_context variants of set_properties and warp functions in cbindings, along with tests

* Revert "Correct more formatting"

This reverts commit 6e409cf.

* Revert "Correct formatting of ExpectProperties"

This reverts commit 4d5ee88.

* Revert "Fix for #786 by setting numProps in GatherFaces method. Added test for Decompose with properties, and ExpectProperties function in test_main in order to check properties in a mesh match those set by SetProperties."

This reverts commit 03848fa.

* Remove _context variants of functions and make context argument the default for all callbacks. Add bounds checks to warp test and volume and surface area checks to level set test

* Correct formatting

* Use M_PI not M_PIf as Windows/Mac don't have it

* Use glm not C math

* Mac build sensitive to glm::pow<float,float> vs glm::pow<float>

* Use std::pow for pow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure properties maintained through Decompose operation harmanpa/manifold
3 participants
@harmanpa @elalish @pca006132