-
Notifications
You must be signed in to change notification settings - Fork 171
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.
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
[ABC-312] Fix Face scope constant attributes #569
Conversation
Co-authored-by: thomas-tu <45040865+thomas-tu@users.noreply.github.com>
Co-authored-by: thomas-tu <45040865+thomas-tu@users.noreply.github.com>
…com/Unity-Technologies/com.unity.formats.alembic into ABC312-Fix-face-constant-attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was randomly trying to generate more test data and stumbled onto one possible issue.
In Houdini I created a mesh with 4 triangles and a Cd
attribute per triangle:
- Triangle 0, Cd value = [1, 0, 0, 0]
- Triangle 1, Cd value = [0, 1, 0, 0]
- Triangle 2, Cd value = [0, 0, 1, 0]
- Triangle 3, Cd value = [0, 0, 0, 1]
This is what it looks like in Houdini:
If Import the alembic in Unity and apply the Alembic/Overlay
material this is what I see:
To be completely honest, I don't really know what I'm doing 😅, but I feel like the colors shouldn't appear to blend like this. I would assume to the same thing as in Houdini
I've attached the alembic file and the Houdini scene used to generate it in case it's useful (it's a .zip
because Git doesn't want .hip
/.abc
files...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found one more problematic case 🙈. The facecolor appears incorrectly "blended" if a mesh contains polygons with varying numbers of sides (ex: if a mesh contains a three-sided polygon and a four-sided polygon).
The first polygon appears correct and the others are "blended":
Houdini | Unity |
---|---|
I've attached four files that can be used for testing facecolor_samples.zip :
- plane_triQuad_facecolor_1_24fps.abc = 1 three-sided polygon, 1 four-sided polygon
- plane_triNgon_facecolor_1_24fps.abc = 1 three-sided polygon, 1 five-sided polygon
- plane_quadNgon_facecolor_1_24fps.abc = 1 four-sided polygon, 1 five-sided polygon
- plane_ngonTriQuad_facecolor_1_24fps.abc = 1 three-sided polygon, 1 four-sided polygon, 1 five-sided polygon
I believe that we/Unity create a sub-mesh for every type of polygons. For example: if an alembic mesh contains both triangles and quads, all triangles will be grouped in a submesh and all quads will be grouped in another sub-mesh.
I'm not sure what the behavior is for polygons with more than 4 sides (ie: are they grouped by polydon-side-count or all together in a single sub-mesh)
I'm really not confident about the quality of this information though. I feel like someone reliable said this to me but it's in a very dusty corner of my memory, haha. I'm including it just in case helps for debugging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA approved 👍!
I'm a bit limited in testing and knowledge around this area but every case I could think of seems to work now using the Color/Cd attribute + Alembic/Overlay material 🎉
Tested on:
- 2023.2.0b15 (Windows only)
noreply@unity3d.com seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Co-authored-by: thomas-tu <45040865+thomas-tu@users.noreply.github.com>
…com/Unity-Technologies/com.unity.formats.alembic into ABC312-Fix-face-constant-attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dumb question: Why is there actual code in this header file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran it and it seems to work. The code logic follows from what I can tell. Just some wording stuff.
[TestCase("cube_face")] | ||
[TestCase("cube_point")] | ||
[TestCase("cube_vertex")] | ||
public void VertexRgb_AreProcessedCorrectlyForScope(string scope) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming of the test functions could be clearer
public void VertexRgb_AreProcessedCorrectlyForScope(string scope) | |
public void Import_InScope_VertexRgb_Colors_AreAsExpected(string scope) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True ! changed here f30b011
Co-authored-by: Jun Loke <jun.loke@unity3d.com>
Purpose of this PR
Ticket/Jira #:ABC-312
When exporting the alembic with face scope, the custom attributes aren't processed as intended. This PR addresses this issue by introducing a method that uses the face count to produce an index array for the attributes.
Testing
Functional Testing status:
Manual testing :
attributeScopeCubes
into a sceneNotice how the "cube_face" is now colored, each face with a color (this is the same result obtained when importing the cubes in Houdini).
new
MeshSchema
test class added :-
VertexRgb_AreProcessedCorrectlyForScope
-
VertexRgba_AreProcessedCorrectlyForScope
-
VertexUV_AreProcessedCorrectlyForScope
-
VertexNormals_AreProcessedCorrectlyForScope
along with the missing data to test rgba, uv and normals.
Performance Testing status: N/A
Overall Product Risks
Complexity:
LowHalo Effect:
MinimalDocumentation & UX Writing
Additional information
Note to reviewers:
Reminder: