Autodesk / sitoa

Arnold plugin for Softimage
Apache License 2.0
33 stars 16 forks source link

closure shader isn't inspectable #33

Closed caron closed 6 years ago

caron commented 6 years ago

So I haven't got to use SItoA 4.x much at all so I only just noticed this issue. When running 'Inspect Material' or clicking on the icon of a Material in the explorer an empty property page shows. Maybe we need to show the closure port in the UI so you can at least click on the port to jump to the Standard Surface shader.

Thoughts?

JenusL commented 6 years ago

Best would be to make it show the actual material instead of the closure node. Maybe it's possible with some callback or something.

caron commented 6 years ago

Maybe a callback on the closure node which on inspection it always just inspects what's connected to the closure port. I need to test this.

caron commented 6 years ago

@JenusL , can you check this commit I have in my fork? Not sure if you have added me as a remote? If so you should see the new branch otherwise I could send you a pull request.

This actually helps me work out how the ongoing feat/ui-gen-improvements can be done with PutLogic. Let me know if there are any code style issues here too, multi-line strings injecting JScript in C++...

JenusL commented 6 years ago

@caron I think it looks good! I would just respect the JS coding styles so that if you print the Logic in Softimage, it looks nice. Here's the updated Logic with new lines and indentation:

         L"function OnInit() {\n"
         L"   var src = PPG.closure.Source;\n"
         L"   if (src != null) InspectObj(src.Parent);\n"
         L"}"
caron commented 6 years ago

Did you test inspecting the node in various ways? I tested in the explorer, clicking on the material icon on the object, the closure node icon itself. I also tested right click 'Inspect Material...' in the viewport and lastly double clicking on the node in the Render Tree.

Not sure if there are others which I am missing...

JenusL commented 6 years ago

Yeah I tried inspecting it in as many ways as I could think of and I didn't have any problems. The only thing I can think is that this would be nice to have in the other shaders that only has closures as inputs (the PPG is empty). passthrough and ray_switch_shader comes to mind. Or do you think that we want those empty PPGs to pop up, so that we know they're there? You decide :)

caron commented 6 years ago

hmm, I have never seen the passthrough shader before... Ideally, we could have the closure port type show similar to the color port with a connector button so you can walk the graph through the PPG but block the closure type from allowing color types connect to it, I will have to research this. I will open another ticket once I figure out what we can actually do with custom types in the render tree/PPG.

JenusL commented 6 years ago

I've used both shaders. Passthrough is pretty much the same as the one that's in Softimage (Color4_Passthrough). It's mainly used to connect aovwrite* shaders in to the eval ports. I agree that it would be good to see the connection in the PPG but this is a very minor issue. I suggest we spend some time on other things first.

caron commented 6 years ago

Ya, I am closing this one out once we get #40 merged

caron commented 6 years ago

40 has been merged