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

Pixar Mesh write translator/utilities refactoring #420

Merged
merged 47 commits into from
Jul 1, 2020

Conversation

HamedSabri-adsk
Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk commented Apr 14, 2020

PR Description

This PR is the continuation of work on refactoring mesh translator/utilities in PR #343.

This PR involves refactoring Pixar's mesh writer class ( PxrUsdTranslators_MeshWriter ) by creating utilities and providing a simpler interface:

  • All utilities for writing out mesh data are now housed under UsdMayaMeshWriteUtils namespace.
  • UsdUtilsSparseValueWriter is now directly passed as an optional argument to all utility functions responsible for writing mesh data.
  • Decoupled the logics for dealing with writing out joint and skin data. These utilities are now under UsdMayaJointUtil namespace.

Hamed Sabri added 10 commits April 10, 2020 09:07
…il namespace so they are accessible for others.
…doesn't do much work at the moment )

- Created meshWriteUtils that houses utility functions operating on writing mesh data.

- Moved below utility functions from meshReadUtils to meshWriteUtils since they operate on write.

  GetMeshNormals()
  GetSubdivScheme()
  GetSubdivInterpBoundary()
  GetSubdivFVLinearInterpolation()

- Moved isMeshValid() and _exportReferenceMesh() to meshWriteUtils
…DivTagsToUSDPrim

- Created two new template functions for writing attribute values. These functions now take UsdUtilsSparseValueWriter by reference and eventually would replace _SetAttribute functions in UsdMayaPrimWriter.

Note: Pixar uses UsdUtilsSparseValueWriter Utility class that manages sparse authoring of a set of UsdAttributes. UsdUtilsSparseAttrValueWriter is A utility class for authoring time-varying attribute values with simple run-length encoding, by skipping any redundant time-samples.
- vertex position
- face/vertex indices
- invisible faces

Removed the "TODO: Use memcpy()" and used memcpy to copy the data directly.
…ointUtil) that houses functions for writing out joint and skin data. This work had to be done since writing skinning data happens during writeMeshAttrs call. There are more work needs to be done to make this utilities reusable in other places but this is outside the scope of this PR.
cameraWriter
instancerWriter
jointWriter
nurbsCurveWriter
nurbsSurfaceWriter
particleWriter
- vertex position
- face/vertex indices
- invisible faces

Removed the "TODO: Use memcpy()" and used memcpy to copy the data directly.
@HamedSabri-adsk HamedSabri-adsk added core Related to core library import-export Related to Import and/or Export labels Apr 14, 2020
Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

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

Thanks @HamedSabri-adsk! The change looks great to me, but I just had one request to use the newly public getDagPath() in UsdMayaWriteJobContext::CreatePrimWriter() as well.

lib/mayaUsd/utils/util.h Show resolved Hide resolved
@HamedSabri-adsk HamedSabri-adsk requested a review from mattyjams May 25, 2020 16:41
HamedSabri-adsk and others added 2 commits May 28, 2020 07:23
…te_part2

PR II: Pixar Mesh write translator/utilities refactoring
…ny where, there is no test case around it, and original logic shouldn't have worked.
@HamedSabri-adsk HamedSabri-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Jun 11, 2020
Comment on lines 73 to 84

void wrapMeshWriteUtils()
{
// TODO: HS, June 11, 2020 This logics is not accurate. Revisit this later.
// There are no test cases around is not being consumed in any tests.
/*
scope s = class_<DummyScopeClass>("MeshWriteUtils", no_init)

.def("GetMeshNormals", &_GetMeshNormals)
.staticmethod("GetMeshNormals")

;
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

@HamedSabri-adsk: Can you please revert this change? True that the Python bindings for this are not used anywhere in maya-usd, but we have internal code that does this (using the old name):

from pxr.UsdMaya import MeshUtil
(normalsArray, interpolation) = MeshUtil.GetMeshNormals(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @mattyjams Sure will do. But before that I want to understand a few things:

1-
I can see in the original code that GetMeshNormals is not a static function and yet it in the python binding is declared as staticmethod. I am not an expert in Boost python binding but curious how this works?

PixarAnimationStudios/OpenUSD@be80acc

2- MeshUtil has now renamed MeshWriteUtils. I had to fix it in this commit:

a520d00

Does this mean that we should revert these name changes as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

For #1, we're binding MeshWriteUtils.GetMeshNormals() in Python to the _GetMeshNormals() function declared just above in wrapMeshWriteUtils.cpp, which is indeed declared static, and returns a boost tuple type:

_GetMeshNormals(const std::string& meshDagPath)

For #2, we do want the name changes so that files/classes/etc. are all in alignment, so that part should not be reverted. We can absorb the name change no problem, we just need a function with the new name to exist so that we can switch to it. :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattyjams Gotcha... see be3c545

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks!

…t used any where, there is no test case around it, and original logic shouldn't have worked."

This reverts commit 5803a46.
@kxl-adsk kxl-adsk merged commit 8b1c326 into dev Jul 1, 2020
@kxl-adsk kxl-adsk deleted the sabrih/MAYA-101460/pxr_mesh_write_part1 branch July 1, 2020 01:14
mattyjams added a commit to mattyjams/maya-usd that referenced this pull request Jul 8, 2020
After PR Autodesk#420, the crease sharpnesses were incorrectly being populated using
the edge IDs.
kxl-adsk pushed a commit that referenced this pull request Jul 9, 2020
…_export

fix for exporting mesh crease sharpnesses following PR #420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library import-export Related to Import and/or Export
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants