RenderPassEditor : Add Rename Render Pass menu item

[murraystevenson]

This adds a "Rename Selected Render Pass" menu item to the "Name" column's popup menu with the ability to rename a render pass within the edit scope it was originally created. This action covers simple cases, such as fixing a typo in a newly created render pass name.

References to the old render pass name outside of the originating edit scope are left unchanged by this action and would still need to be updated by the user, including those in other edit scopes.

[tomc-cinesite]

@murraystevenson - this looks like its really going to help people, thanks so much! This topic has come up a lot.

Q/unsolicited thought re:

... with the ability to rename a render pass within the edit scope it was originally created.

Sorry not been able to test a build of this as on mac so please take with a pinch of salt - is this item only enabled if the target edit scope is the one that made the pass? When I watched the gif I think I misunderstood the warning message a little until I read the above description.

As it said "target edit scope" it made me think I could rename passes midway through the graph and/or it might only change the name within the scope itself. What do you think to something like:

⚠️ References to this pass by name will need to be manually updated

Can this be used in "source" mode too?

[johnhaddon]

Thanks Murray! Implementation LGTM but I've done a bit of nitpicking/hand-wringing about the messaging. Not sure my suggestions are better, but might be worth considering - see comments inline.

@tomc-cinesite, I think I can answer a couple of your questions :

is this item only enabled if the target edit scope is the one that made the pass?

The operation only works if the target edit scope is the one that made the pass. But the menu item is enabled even the pass wasn't created in this scope - this lets us pop up a more informative warning/error instead of just greying out the menu item. The other option would be to grey out the icon but add a tooltip explaining the issue, but that seems less discoverable.

Can this be used in "source" mode too?

No - source mode never makes edits inside an EditScope. Although we could search for and edit the name in a RenderPasses node outside any edit scope, I'm not sure how useful that would be. And I think defining expectations around what else gets renamed automatically would be harder in source mode.

[johnhaddon]

Thanks for the update Murray - definitely feeling like worthwhile improvements. I made a few take-it-or-leave-it comments inline, but I think we're pretty much there.

This has got me thinking about the slightly more ambitious alternative though, which would use a new RenameRenderPasses (or ShuffleRenderPasses?) node within the EditScope to allow local renaming of passes created upstream. I suspect we've talked about it before and I've forgotten what our objections were. But right now I feel like it might be quite nice in conjunction with blue-background-styling on the cell, and an inspector-style history to allow introspection. Definitely not proposing we do that right now, but would be curious to know your thoughts...

[murraystevenson]

Thanks for the input, I do prefer where we've ended up! I've squashed in the last round of fixups and rebased. Last few changes are commented inline.

This has got me thinking about the slightly more ambitious alternative though, which would use a new RenameRenderPasses (or ShuffleRenderPasses?) node within the EditScope to allow local renaming of passes created upstream. I suspect we've talked about it before and I've forgotten what our objections were. But right now I feel like it might be quite nice in conjunction with blue-background-styling on the cell, and an inspector-style history to allow introspection. Definitely not proposing we do that right now, but would be curious to know your thoughts...

I do think there's a lot of value in a ShuffleRenderPasses node for more procedural approaches. I think the action here is more for the "fix up my typo" style renaming where you don't really want to preserve the history. A separate "Shuffle to..." action in the Render Pass Editor might distinguish the more procedural approach you're suggesting along with the blue-backgrounds and history to support it...

[johnhaddon]

Thanks Murray - updates LGTM. I suggest we resolve the to-throw-or-not-to-throw question in today's catch up.

[johnhaddon]

Thanks Murray - quick quibble about the Python bindings inline, but otherwise all LGTM.

[murraystevenson]

Thanks Murray - quick quibble about the Python bindings inline, but otherwise all LGTM.

Thanks! I've updated the Python bindings to return None for the editable case and rebased. Merging.