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-122528 - MayaUSD should support UFE lights #2266

Merged
merged 11 commits into from
Apr 22, 2022
Merged

Conversation

vlasovi
Copy link
Collaborator

@vlasovi vlasovi commented Apr 7, 2022

Create new class ufe::UsdLight inheriting from Ufe::Light to represent usd lights in UFE

lib/mayaUsd/ufe/UsdLight.cpp Show resolved Hide resolved
test/lib/ufe/testLight.py Outdated Show resolved Hide resolved
@vlasovi
Copy link
Collaborator Author

vlasovi commented Apr 11, 2022

@ppt-adsk I wrapped the new code using UFE Light into UFE_V4_FEATURES_AVAILABLE macro, but the perflight still fails. I can also add a check against UFE_PREVIEW_VERSION_NUM, but this looks like a temporary solution that would require an additional PR in the future. How should I proceed to fix the preflights?

williamkrick
williamkrick previously approved these changes Apr 12, 2022
@vlasovi vlasovi added the ready-for-merge Development process is finished, PR is ready for merge label Apr 19, 2022
Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

Undo / redo implementation needs to use UsdUndoableItem / UsdUndoBlock, but you're really close.

self.inchesToCm = 2.54
self.mmToCm = 0.1

def _StartTest(self, testName):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be called in setupClass() or setUp().


def _StartTest(self, testName):
cmds.file(force=True, new=True)
mayaUtils.loadPlugin("mayaUsdPlugin")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary, done by the (poorly-named) isMayaUsdPluginLoaded().

@@ -0,0 +1,257 @@
#!/usr/bin/env python
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would have been good to get some undo / redo testing done as well. Perhaps next pull request.

namespace ufe {

template <typename ValueTypeIn>
class SetValueUndoableCommandImpl : public Ufe::SetValueUndoableCommand<ValueTypeIn>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately we can't implement undo / redo in USD by setting the previous value on undo, because this is incomplete. If you imagine a situation where the edit target is set to a higher-priority layer which has no current opinion, undo should completely remove the opinion, NOT set the new opinion to the previous value.

Fortunately we have helper classes for this which make it easy to do the right thing. See the existing

template <typename Cmd> class UsdUndoableCommand : public Cmd

and
class UsdUndoableCommand : public Ufe::UndoableCommand

and their uses in the code base, and pick the one that's right for you.

return Ufe::Light::Invalid;
}

float getLightIntensity(const UsdPrim& prim)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it useful for these free functions to be visible in the MayaUsd::ufe namespace? Or should they be private in the unnamed namespace for this translation unit? No preference either way, just wondering.

@ppt-adsk ppt-adsk removed the ready-for-merge Development process is finished, PR is ready for merge label Apr 20, 2022
@vlasovi vlasovi added the ready-for-merge Development process is finished, PR is ready for merge label Apr 20, 2022
@seando-adsk seando-adsk added the ufe-usd Related to UFE-USD plugin in Maya-Usd label Apr 22, 2022
@seando-adsk seando-adsk merged commit 5965302 into dev Apr 22, 2022
@seando-adsk seando-adsk deleted the vlasovi/MAYA-122528 branch April 22, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge ufe-usd Related to UFE-USD plugin in Maya-Usd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants