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.

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

Reader alembic uv normal support #1055

Closed
wants to merge 11 commits into from

Conversation

nyue
Copy link
Contributor

@nyue nyue commented Oct 23, 2023

No description provided.

nyue added 6 commits October 22, 2023 09:52
Remove unused code

Remove commented out debugging print out
Also updated the baseline test image for Alembic.

Signed-off-by: Nicholas Yue <[email protected]>
Copy link
Member

@Meakk Meakk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some first comments.

plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
plugins/alembic/module/vtkF3DAlembicReader.cxx Outdated Show resolved Hide resolved
nyue added 5 commits October 30, 2023 10:22
Mostly basic code clean up and improvement.

There are additional changes that contains logic changes. Those
changes will be in another commit.

Signed-off-by: Nicholas Yue <[email protected]>
I am keeping this available for now in anticipation of
edge cases where we do need to perform the operations.

When the code is use with my production assets, we might
encounter such edge cases.

When the follow release of f3d comes along and we still
have not encounter such edge cases, I can remove it.

Signed-off-by: Nicholas Yue <[email protected]>
@mwestphal
Copy link
Contributor

@nyue did you need any help finishing this one ? Should we fix the CI issues for you ?

@nyue
Copy link
Contributor Author

nyue commented Nov 11, 2023

Yes I do. I believe @Meakk will circle back to looking at the CI side of things as per Discord discussion.

@Meakk
Copy link
Member

Meakk commented Nov 13, 2023

I'm currently very busy but I haven't forgot :)
@mwestphal if you have time, please take over, otherwise I'll try to do this soon.

@mwestphal
Copy link
Contributor

Sure, Ill take a look.

@mwestphal
Copy link
Contributor

@nyue you need to give me authorisation to push on your fork. Alternatively I can create a new PR.

points->SetPoint(i, positions->get()[i].x, positions->get()[i].y, positions->get()[i].z);
auto this_face_vertex_count = original_data._indices[i].size();
duplicated_data._indices[i].resize(
this_face_vertex_count, Alembic::Abc::V3i(-9999, -9999, -9999));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does -9999 correspond to @nyue ?


polys->SetData(offsets, connectivity);
auto P_map_iter = data._attributes.find("P");
assert(P_map_iter != data._attributes.end());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this actually happen @nyue ?

@mwestphal
Copy link
Contributor

closing in favor of #1069

@mwestphal mwestphal closed this Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants