Thicket-Blender / thicket

Thicket: Laubwerk Plants Add-on for Blender
GNU General Public License v2.0
65 stars 9 forks source link

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

Closed tdapper closed 4 years ago

tdapper commented 4 years ago

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.

dvhart commented 4 years ago

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 commented 4 years ago

It's more an educated guess, but the error message quoted in https://github.com/Thicket-Blender/thicket/issues/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 https://github.com/Thicket-Blender/thicket/issues/31, the problems would show up in high quality materials only, I believe.

dvhart commented 4 years ago

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 commented 4 years ago

New material pipeline implemented in materials-pipeline branch:

material-pipeline

Close up render results:

material-pipeline-render

tdapper commented 4 years ago

@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 commented 4 years ago

@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?