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

MAYA-104985 Allow multiple export converters to co-exist in useRegistry export #721

Merged
merged 9 commits into from
Sep 10, 2020

Conversation

JGamache-autodesk
Copy link
Collaborator

@JGamache-autodesk JGamache-autodesk commented Aug 18, 2020

The material exporter now supports multiple exporter classes for the same Maya dependency node type.
A new parameter was added to the export command to specify a converter on export and the registry will choose the exporter that best matches the converter found in the export options.
A test exporter was written to make sure the mechanism works, but no new functionality was added to the current exporters.

@JGamache-autodesk JGamache-autodesk added the import-export Related to Import and/or Export label Aug 18, 2020
@JGamache-autodesk JGamache-autodesk changed the title MAYA-104985 Use rendex context on export MAYA-104985 Use render context on export Aug 18, 2020
…port_render_context

# Conflicts:
#	lib/mayaUsd/fileio/jobs/jobArgs.cpp
#	lib/mayaUsd/fileio/shading/shadingModeRegistry.h
#	plugin/adsk/scripts/mayaUsdTranslatorExport.mel
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.

The changes look mostly ok to me, with a few notes sprinkled throughout.

My main issue is that I think we need to clarify the terminology we're using and be explicit about where we're aligning with terms in USD and where (if anywhere) we're not. I think the intent is for the usage of "render context" here to align with the "renderContext" concept in UsdShade, so I don't think the replacement of "universal" with "preview" here is appropriate. For the most part I think I'd be comfortable with this if we just used "universal" instead.

lib/mayaUsd/fileio/jobs/jobArgs.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shaderWriterRegistry.cpp Show resolved Hide resolved
lib/mayaUsd/fileio/shaderWriterRegistry.cpp Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shaderWriterRegistry.h Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shaderWriterRegistry.h Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shading/shadingModeUseRegistry.cpp Outdated Show resolved Hide resolved
lib/usd/pxrUsdPreviewSurface/usdPreviewSurfaceWriter.cpp Outdated Show resolved Hide resolved
plugin/pxr/doc/README.md Outdated Show resolved Hide resolved
@JGamache-autodesk JGamache-autodesk changed the title MAYA-104985 Use render context on export MAYA-104985 Allow multiple export converters to co-exist in useRegistry export Aug 27, 2020
…port_render_context

# Conflicts:
#	lib/mayaUsd/fileio/shading/shadingModeRegistry.cpp
#	test/lib/usd/translators/CMakeLists.txt
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.

Looks good to me now! Thanks for re-working all this, @JGamache-autodesk! I think it made everything a lot clearer.

I had just a few notes on the tests that I was a little confused by.

lib/mayaUsd/fileio/shading/shadingModeRegistry.h Outdated Show resolved Hide resolved
lib/mayaUsd/fileio/shaderWriterRegistry.h Show resolved Hide resolved
test/lib/usd/CMakeLists.txt Outdated Show resolved Hide resolved
test/lib/usd/translators/CMakeLists.txt Show resolved Hide resolved
test/lib/usd/translators/CMakeLists.txt Show resolved Hide resolved
@JGamache-autodesk JGamache-autodesk added the ready-for-merge Development process is finished, PR is ready for merge label Sep 10, 2020
@kxl-adsk kxl-adsk merged commit 644514e into dev Sep 10, 2020
@kxl-adsk kxl-adsk deleted the t_gamaj/MAYA-104985/export_render_context branch September 10, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
import-export Related to Import and/or Export ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants