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

[mtoh] Split pre and post scene passes so camera guides go above Hydra. #526

Merged
merged 1 commit into from
Jun 3, 2020

Conversation

marsupial
Copy link
Contributor

@kxl-adsk kxl-adsk requested a review from huidong-chen May 26, 2020 11:18
@kxl-adsk kxl-adsk added the mtoh Related to legacy Maya to Hydra plugin. label May 26, 2020
@huidong-chen huidong-chen requested a review from pmolodo May 26, 2020 19:15
}

MHWRender::MClearOperation& clearOperation() override {
mClearOperation.setMask(MHWRender::MClearOperation::kClearNone);
return mClearOperation;
}

MSceneFilterOption renderFilterOverride() override {
return MSceneFilterOption(MHWRender::MSceneRender::renderFilterOverride() | kRenderPostSceneUIItems);
Copy link

@huidong-chen huidong-chen May 26, 2020

Choose a reason for hiding this comment

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

Can you return kRenderPostSceneUIItems directly? From API doc, MSceneRender::renderFilterOverride() returns kNoSceneFilterOverride which equals 0.

@@ -66,13 +66,17 @@ class HdMayaManipulatorRender : public MHWRender::MSceneRender {
: MHWRender::MSceneRender(name) {}

MUint64 getObjectTypeExclusions() override {
return ~MHWRender::MFrameContext::kExcludeManipulators;
return MHWRender::MSceneRender::getObjectTypeExclusions() | ~(kExcludeManipulators | kExcludeHoldOuts);

Choose a reason for hiding this comment

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

Is this change relevant to the issue? Why do you exclude holdouts?

MHWRender::MSceneRender::getObjectTypeExclusions() returns 0, so there is no need to use it in the bitmask op.

@huidong-chen
Copy link

I don't know well enough about mtoh implementation to review the impact to any existing workflow, adding @elrond79 to review too.

@marsupial
Copy link
Contributor Author

I just called through to the base classes as I didn't know they returned 0. So I can test to see if removing them still works.

The other change is relevant, as kExcludeHoldOuts is what winds up drawing the camera guides from what I've seen.

Copy link
Contributor

@pmolodo pmolodo left a comment

Choose a reason for hiding this comment

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

Haven't tested the code, but the code changes make sense to me, assuming that kExcludeHoldOuts is where camera guides are grouped. (Which seems odd, but I'll take your word for it!)

@huidong-chen
Copy link

Yes, a bit odd. Maybe adding a comment to explain the purpose of excluding holdout would be better.

@huidong-chen
Copy link

I just called through to the base classes as I didn't know they returned 0. So I can test to see if removing them still works.

The other change is relevant, as kExcludeHoldOuts is what winds up drawing the camera guides from what I've seen.

@marsupial How is the test doing?

@marsupial
Copy link
Contributor Author

Sadly no time! Monday for sure.

@marsupial marsupial force-pushed the PR/mtoh-cam-guides branch from 0dd4077 to aeb87c4 Compare June 2, 2020 00:22
@kxl-adsk
Copy link

kxl-adsk commented Jun 3, 2020

@marsupial I see you (force) pushed changes two days ago to the branch in your fork. We are not receiving notification for this action from GitHub. For future PRs, please leave a comment in the PR when reviewers should review new changes...or you can re-request the review from a specific person which will send a notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mtoh Related to legacy Maya to Hydra plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants