Fix T62351: Material Ouptut selection broken
AbandonedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Mar 8 2019, 2:54 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)
Clément Foucault (fclem)

Maniphest Tasks

T62434: EEVEE not using correct World Output node
T62351: Material Ouptut selection broken

Summary

We were allowing a (shader) output node to become the current output
(NODE_DO_OUTPUT) on selection regardless of the target render engine
this node specifies.

Now it checks the current render engine and only allows the output node
to be come the current output if it matches the specified target.

This also tweakes the logic in ntreeShaderOutputNode to not force an exact match of output target
With former behavior it was impossible to switch to an output node (with target "ALL")
and see its effects when also another output node (with target "Cycles") was present.
Also the Property Editor would never switch to it.

Also now updates current output node for all nodetrees on render engine change.

Diff Detail

Repository

rB Blender

Branch

T62351 (branched from master)

Build Status

Buildable 3100
Build 3100: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Mar 8 2019, 2:54 PM

Harbormaster completed remote builds in B3085: Diff 14075.Mar 8 2019, 2:54 PM

Just checking if this is the right direction.

In regards to the bugreport what could/should still be done:

changing the render engine should find an appropriate (material) output node [if the current one is not matching] and make that the current output?
currently [in master] the logic in cycles seems a bit off [when you have multiple outputs, you can switch between two "Cycles" ones or two "All" ones, but if one of them is "Cycles" and one of them is "All", the cycles one is always preferred...]

Philipp Oeser (lichtwerk) mentioned this in T62351: Material Ouptut selection broken.Mar 8 2019, 2:56 PM

Philipp Oeser (lichtwerk) mentioned this in T62434: EEVEE not using correct World Output node.Mar 11 2019, 9:20 AM

tweaked logic ntreeShaderOutputNode to not force an exact match of output target

With former behavior it was impossible to switch to an output node (with
target "ALL") and see its effects when also another output node (with
target "Cycles") was present.
Also the Property Editor would never switch to it.

Harbormaster completed remote builds in B3098: Diff 14109.Mar 11 2019, 10:47 AM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Mar 11 2019, 10:49 AM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

update current output node for all nodetrees on render engine change
(not quite sure how costly this is in gigantic production files though...)
so should fix T62434 now, too

Harbormaster completed remote builds in B3100: Diff 14114.Mar 11 2019, 2:17 PM

Philipp Oeser (lichtwerk) added a task: T62434: EEVEE not using correct World Output node.Mar 11 2019, 2:17 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

I'm not sure about this. The bug reports seems to be about an issue in Eevee that does not exist in Cycles, so it should be solvable without these changes?

I don't think node edit behavior should depend on the active render engine like this. Also when Cycles is the active renderer can you see Eevee in LookDev mode and want to change the active output for that.

In principle you can also have two windows showing different scenes with different renderers, so changing the node tree based on that can't work for both.

This revision now requires changes to proceed.Mar 13 2019, 2:23 PM

OK, we had these things reported:

no updates when tweaking nodes in a cycles part of a tree (when another output node is set NODE_DO_OUTPUT T62351)
Eevee is not respecting the output nodes target (is always using whatever is NODE_DO_OUTPUT T62434)

My goal was to to force NODE_DO_OUTPUT to match the renderengine, but I see this is not the right direction.

Will investigate alternatives then:

forcing updates (even in part of a tree which is not connected to a 'current' NODE_DO_OUTPUT output node -- or would this not be considered a bug then? T62351)
force Eevee to pick a matching output (similar to how cycles does this with get_output_node() / ntreeShaderOutputNode() T62434)

However, for the second case, what would happen if you are setting up a tree with a 'cycles' output node: could you not use Eevee in Lookdev mode then?

Also when Cycles is the active renderer can you see Eevee in LookDev mode and want to change the active output for that

needs some more thinking (at least on my side...)

forcing updates (even in part of a tree which is not connected to a 'current' NODE_DO_OUTPUT output node -- or would this not be considered a bug then? T62351)

Maybe we could do updates for any nodes connected to either the active Cycles or Eevee node. Still a bit too much than needed, but not a big problem I think.

force Eevee to pick a matching output (similar to how cycles does this with get_output_node() / ntreeShaderOutputNode() T62434)

However, for the second case, what would happen if you are setting up a tree with a 'cycles' output node: could you not use Eevee in Lookdev mode then?

Also when Cycles is the active renderer can you see Eevee in LookDev mode and want to change the active output for that

If there is only a Cycles output node, then Eevee should fall back to using that, and vice versa.

Philipp Oeser (lichtwerk) mentioned this in D4630: Fix T62434: EEVEE not using correct World Output node.Apr 2 2019, 4:50 PM

Philipp Oeser (lichtwerk) mentioned this in rB96de11c2c67e: Fix T62434: EEVEE not using correct World Output node.Apr 2 2019, 5:14 PM

this was fixed already