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

Add displays section to SceneInspector #921

Closed
johnhaddon opened this issue Jul 18, 2014 · 9 comments
Closed

Add displays section to SceneInspector #921

johnhaddon opened this issue Jul 18, 2014 · 9 comments
Labels
scene Issues with GafferScene

Comments

@johnhaddon
Copy link
Member

No description provided.

@johnhaddon
Copy link
Member Author

It seems like the best way of listing the displays would be to use the label that is displayed in the Displays node (Interactive/Beauty) etc, but that information doesn't get passed through the graph at the moment.

Currently, displays are passed through the graph as "display:imageFileName" items in the globals, mapping to IECore.Display objects. I can't quite remember the history of that, but it was either done before we had labels for displays, or I used the filename as a key because that way it's impossible to get multiple displays writing to the same file.

Now I need to get the label through the graph to the SceneInspector, I'd like to change those keys to be "display:label", which I think is a bit more intuitive anyway. Is that OK with everyone?

@andrewkaufman
Copy link
Contributor

So then if you have back to back Displays nodes adding the same label but different file paths, does the last node win? Or do they both make it through somehow?

@johnhaddon
Copy link
Member Author

As it is now, the last one to use a particular filename wins (and labels are irrelevant), and with what I'm suggesting, it'd be the last one to use a particular label that would win. I think that makes sense, because people would think "I'm redefining my diffuse AOV" rather than "I'm adding another diffuse AOV" - but do you think otherwise?

@andrewkaufman
Copy link
Contributor

No, I agree (and always have actually, its quite annoying in IERendering that the labels mean less than the file paths). If they want to add another diffuse aov, they should label it something different (otherDiffuse).

@johnhaddon
Copy link
Member Author

OK, cool. Then I'll make that change. In an ideal world I think I'd rename label to name and name to filename just to make things even clearer - is that too much of a backwards compatibility issue at this point or are you emboldened by our new tolerant loading?

@andrewkaufman
Copy link
Contributor

I would like that change, but I'll let @davidsminor and @danieldresser decide if it's too scary. I thought you were against it because it doesn't match IECore.Display?

@johnhaddon
Copy link
Member Author

Yeah, but then we can go on a rampage and rename IECore.Display to IECore.Output and change the name to fileName there too. If it's too much though, just changing the data output by the node is fine for my current purposes and doesn't have knock on effects that I'm aware of...

@danieldresser
Copy link
Collaborator

Sounds entirely reasonable to me. I haven't yet had a chance to start on an IE render pass node, so I'm not concerned about compatibility yet. David might be affected more.

@davidsminor
Copy link
Contributor

I'm ok with stuff getting renamed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scene Issues with GafferScene
Projects
None yet
Development

No branches or pull requests

4 participants