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

[al] Mark outTime attribute dirty to force updating Maya bbox; fix incorrect time code for bbox #2251

Merged

Conversation

csyshing
Copy link
Collaborator

@csyshing csyshing commented Apr 6, 2022

Changes for this PR:

  • Mark geometry dirty explicitly otherwise bbox won't move
  • Fix GetOutputTime() to return the correct time code

@seando-adsk seando-adsk added the al Related to AnimalLogic plugin label Apr 6, 2022
@seando-adsk seando-adsk requested a review from williamkrick April 6, 2022 14:32
@@ -1605,6 +1605,7 @@ MStatus ProxyShape::compute(const MPlug& plug, MDataBlock& dataBlock)
m_requestedRedraw = true;
MTime currentTime;
if (plug == outTime()) {
MHWRender::MRenderer::setGeometryDrawDirty(thisMObject());
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that you don't call MayaUsdProxyShapeBase and that the implementation of the two compute() methods has drifted relatively far apart.

  • I believe that MayaUsdProxyShapeBase::compute is already doing the right thing by testing if the computed plug is timeAttr, assuming that plug must be computed in order to compute outTimeAttr. Is my understanding correct?
  • What workflow can be tested to verify that the Maya bbox is correctly updated? Can we write an automated test that will catch this?
  • Is there anything in MayaUsdProxyShapeBase::compute that you should copy here? There is some code to support changing which purposes the proxy shape is displaying which could be relevant to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@williamkrick , those are good questions, I am pretty sure there must be good reasons to be implemented in that way in the first place, however I don't think I am the right person to give you an answer :-(

@williamkrick
Copy link
Contributor

@seando-adsk I have some questions but nothing that would prevent this from being merged.

@seando-adsk seando-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Apr 6, 2022
@seando-adsk seando-adsk merged commit 0889a01 into Autodesk:dev Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
al Related to AnimalLogic plugin 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.

4 participants