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

RenderPassEditor : Camera visibility and matte columns #5733

Conversation

murraystevenson
Copy link
Contributor

This adds new columns to the Render Pass Editor, providing inclusion and exclusion control over camera visibility and holdout mattes for each render pass.

image

Putting this up in order to perform some user testing with the CI build before we fully commit to these columns and their underlying options.

@murraystevenson murraystevenson self-assigned this Mar 16, 2024
Copy link
Member

@johnhaddon johnhaddon left a comment

Choose a reason for hiding this comment

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

Thanks Murray! Really great to see another demonstration of how easy it is to slot new columns into the RenderPassEditor.

All feels pretty good to me, but I do have one query about the behaviour : would there be any merit in making cameraExclusions and matteExclusions work more like exclusions, in that they would clobber any included descendants? An exclusion of /chars will currently clobber an inclusion of /chars/robot because it is a pruning operation. But the same camera inclusion/exclusion results in /chars/robot being visible to the camera...

I can see two arguments :

  1. The underlying operations are different, so we shouldn't try to match the outward semantics. Pruning visibility (whether by actual pruning or setting scene:visible = false) fundamentally can't be overridden in descendants, whereas camera/matte can. That's the way of things, so the current behaviour is natural.
  2. We're defining new higher-level concepts here, and should give them all matching semantics, regardless of the underlying implementation.

I think I'm reasonably on board with 1, but mostly because of our decision to mix the inclusions/exclusions in with any existing attributes from the scene. If we weren't doing that, and were clobbering the scene attributes, I think the consistency of 2 might be better. Perhaps we should at least include an Info panel in the description describing the mechanism whereby this stuff is applied in terms of attributes?

doc/examples/rendering/renderPassEditorArnold.gfr Outdated Show resolved Hide resolved
@murraystevenson murraystevenson force-pushed the renderPassEditorCameraAndMatteInclusions branch from f51b16d to e650081 Compare March 20, 2024 00:10
@murraystevenson
Copy link
Contributor Author

All feels pretty good to me, but I do have one query about the behaviour...

Yeah, it's a tricky one. On the face of it, there's real merit to aligning all the Inclusions and Exclusions columns so they all behave consistently - one concept to grasp and then you're off and running. Though the underlying mechanisms are different, and we may lose out on some of the flexibility that the renderer attributes provide...

For comparison, I've pushed an extra commit that implements the exclusions clobber descendants behaviour. Might be worth getting a bit of user input on this and we can settle on an approach once I'm back from the frozen north.

@murraystevenson murraystevenson force-pushed the renderPassEditorCameraAndMatteInclusions branch from e650081 to 74ceb3e Compare April 22, 2024 18:12
@murraystevenson murraystevenson changed the base branch from 1.3_maintenance to 1.4_maintenance April 22, 2024 18:12
Removed the RenderPassVisibility box and replaced with RenderPassEditor defined camera visibility and matte overrides.
@murraystevenson murraystevenson force-pushed the renderPassEditorCameraAndMatteInclusions branch from 74ceb3e to 8367fd3 Compare April 22, 2024 18:17
@murraystevenson
Copy link
Contributor Author

Merging the non-clobber approach.

@murraystevenson murraystevenson merged commit 29a8309 into GafferHQ:1.4_maintenance Apr 22, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants