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

Two sided materials should use Mix Shader instead of regular mix node #32

Closed
tdapper opened this issue Oct 24, 2020 · 6 comments · Fixed by #37
Closed

Two sided materials should use Mix Shader instead of regular mix node #32

tdapper opened this issue Oct 24, 2020 · 6 comments · Fixed by #37
Assignees
Labels
enhancement New feature or request

Comments

@tdapper
Copy link
Contributor

tdapper commented Oct 24, 2020

Since pretty much every property in the Principled BSDF shader could differ per side, I believe it makes more sense to have two BSDF nodes and mix them through a single "Mix Shader" node instead of mixing every single property with its own "Mix" node.

@tdapper tdapper changed the title Two sided materials should use Mix Shader instead of regular mix node. Two sided materials should use Mix Shader instead of regular mix node Oct 24, 2020
@dvhart dvhart added the enhancement New feature or request label Oct 24, 2020
@dvhart
Copy link
Collaborator

dvhart commented Oct 24, 2020

Thanks for the feedback. Making the implementation more generic makes sense, and I'm curious to see if this might impact Issue #25 (Backfacing not supported by Radeon Pro Render). This will take some research into the Blender material nodes and compatibility with render engines, etc.

@tdapper
Copy link
Contributor Author

tdapper commented Oct 24, 2020

It's more an educated guess, but the error message quoted in #25 appears to be about the "Backfacing" output socket not being supported, not about the mix node not being supported. Therefore, I would assume that it just doesn't change anything for that particular case.

Furthermore, while support for as many renderers as possible would be great, I wouldn't center general design decisions like this around the feature support of certain renderers. That is especially true for Radeon Pro Render, which has a bunch of shortcomings, also on other platforms, which I believe is why e.g. Cinema 4D has removed it from its core product after a few years. And if material quality settings were to be implemented as suggested in #31, the problems would show up in high quality materials only, I believe.

@dvhart
Copy link
Collaborator

dvhart commented Oct 26, 2020

After some experimentation, if we split the two sides into two Principled BSDF, the network becomes a little more than twice as complex due to the need to move the opacity after the Principled BSDF mixer.

A pipeline of sorts becomes evident here which we should probably refactor the material importing code to reflect: create a node network for a "laubwerk.side", then feed that into a node network for a "laubwerk.material", mixing two sides if present. This seems like it will lead to a nicely modular code base, but it will require some significant refactoring.

For reference, here is an example of a manually created two sided node network pipeline:

Screen Shot 2020-10-25 at 7 39 21 PM

dvhart added a commit that referenced this issue Oct 26, 2020
Laubwerk materials contain properties per side and properties for the
material overall. Update the material creation to be a multistage
pipeline with several optional stages (such as two-sided, alpha,
subsurface, etc.).

Refactor the Laubwerk material side specifc property handling into a
separate lbw_side_to_bsdf() function, simplifying the handling of the
two-sided materials.

Now that each side is represented as its own Principled BSDF shader, the
alpha needs to be handled later in the pipeline. Introduce a new
transparency stage to handle alpha.

Take note of the Laubwerk side properties Thicket currently ignores.

Fixes: Issue #32: Two sided materials should use Mix Shader instead of regular mix node

Signed-off-by: Darren Hart <dvhart@infradead.org>
dvhart added a commit that referenced this issue Oct 26, 2020
Laubwerk materials contain properties per side and properties for the
material overall. Update the material creation to be a multistage
pipeline with several optional stages (such as two-sided, alpha,
subsurface, etc.).

Refactor the Laubwerk material side specifc property handling into a
separate lbw_side_to_bsdf() function, simplifying the handling of the
two-sided materials.

Now that each side is represented as its own Principled BSDF shader, the
alpha needs to be handled later in the pipeline. Introduce a new
transparency stage to handle alpha.

Take note of the Laubwerk side properties Thicket currently ignores.

Fixes: Issue #32: Two sided materials should use Mix Shader instead of regular mix node

Signed-off-by: Darren Hart <dvhart@infradead.org>
@dvhart
Copy link
Collaborator

dvhart commented Oct 26, 2020

New material pipeline implemented in materials-pipeline branch:

material-pipeline

Close up render results:

material-pipeline-render

@dvhart dvhart self-assigned this Oct 26, 2020
dvhart added a commit that referenced this issue Oct 26, 2020
Laubwerk materials contain properties per side and properties for the
material overall. Update the material creation to be a multistage
pipeline with several optional stages (such as two-sided, alpha,
subsurface, etc.).

Refactor the Laubwerk material side specifc property handling into a
separate lbw_side_to_bsdf() function, simplifying the handling of the
two-sided materials.

Now that each side is represented as its own Principled BSDF shader, the
alpha needs to be handled later in the pipeline. Introduce a new
transparency stage to handle alpha.

Take note of the Laubwerk side properties Thicket currently ignores.

Fixes: Issue #32: Two sided materials should use Mix Shader instead of regular mix node

Signed-off-by: Darren Hart <dvhart@infradead.org>
@dvhart dvhart linked a pull request Oct 26, 2020 that will close this issue
dvhart added a commit that referenced this issue Oct 27, 2020
Laubwerk materials contain properties per side and properties for the
material overall. Update the material creation to be a multistage
pipeline with several optional stages (such as two-sided, alpha,
subsurface, etc.).

Refactor the Laubwerk material side specifc property handling into a
separate lbw_side_to_bsdf() function, simplifying the handling of the
two-sided materials.

Now that each side is represented as its own Principled BSDF shader, the
alpha needs to be handled later in the pipeline. Introduce a new
transparency stage to handle alpha.

Take note of the Laubwerk side properties Thicket currently ignores.

Fixes: Issue #32: Two sided materials should use Mix Shader instead of regular mix node

Signed-off-by: Darren Hart <dvhart@infradead.org>
@tdapper
Copy link
Contributor Author

tdapper commented Oct 29, 2020

@dvhart This looks great and yes, a pipeline like that makes sense (we also do it that way in our other plugins). One possible optimization (not sure if it has any performance implications, but it may simplify/clarify the material graph): Perhaps keep a list of texture input nodes around and just re-use a node when a texture is used multiple times (with the same color space setting). In the example above, e.g. the input node for the front BSDF color could be re-used as input to the Mix BSDF node.

@dvhart
Copy link
Collaborator

dvhart commented Oct 29, 2020

@tdapper I considered reuse and decided to initially keep the pipeline as simple as possible both visually and in the code. If this turns out to be a performance issue, or if memory usage with lots of plants/materials is a problem, this looks like a good candidate for consideration. If you think it's something we shouldn't lose track of, would you please open a new issue tracking that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants