Page MenuHome

NodeWrangler - add texture for principled: extend func, clean
Needs ReviewPublic

Authored by Dawid Huczyński (tibicen) on Nov 25 2018, 8:26 PM.

Details

Summary
Ref T58039
node.nw_add_textures_for_principled

Additions:

  • tags;
  • proporities while importing textures (tex_coordinate, projection type);
  • AO tex (sets nodes but doesn't connect to main material);
  • Alpha nodes;
  • add "adjustment" node layer, for cleaner layout (by function)(if gamma nodes would destroy other exports i can agree the can be removed, but from my experience working with pbr always force you to modify spec, metal, rough textures.

Changes:

  • add reversed() in linke 2734, so "fancy_matal_nor" will be set as normal not metal;
  • Split Normal and Bump (when both, combines them) I would really like to set default strenght to 0.3 but left at default for now;
  • Cleanup texture nodes generation (it was smart to sort normal, but i think this one is cleaner for look: import texNode > naming etc > assigning by type;
  • layout: node locations are relative now (the margin is only fixed now).

TODO (if this changes would be considered good direction):

  • when alpha included change eevee material opacity options
  • separate gamma nodes add as a separate func so it can be checked in files import proporietes

PS. I'm new and i hope diff index refers to certain branch i was working in BA blender2.8 branch.

Diff Detail

Event Timeline

The direction seems ok to me, though I'm not maintaining addons. I'm not sure who is maintaining this addon now, so assigning this code review to last developers who worked on it.

But please remove adjustment nodes that break exporters, and check that after your changes export/import of shaders still works with file formats like FBX or OBJ.

Multiplying AO into base color is also wrong and should not be done.

Brecht Van Lommel (brecht) requested changes to this revision.Dec 5 2018, 3:46 PM
This revision now requires changes to proceed.Dec 5 2018, 3:46 PM

Think it would be best for @Florian Meyer (testscreenings) to review this as the texture setup is his code.

Import - Export Consistency

I did roundtrips for gltf, fbx, colldada for the standard setup. None of them did a good job of preserving the shader. It would be nice if it were achievable to remedy this. Adding those adjustment nodes would only complicate that. The goal should be to make the import as convinient as possible while preserving the_correct_PBR_setup.
My suggestion would be to remove the gamma node for roughness (isn't a colorramp more usefull anyway?) as well as the multiply node connecting AO and base color.

Standard export:

gltf import:

  • - ignores roughness
  • - ignores displacement
  • + preserves uv mapping (without mapping node)

colldada import:

  • - only preserves base color
  • - adds a random, unconnected rgb input

fbx import:

  • - ignores uv mapping
  • - ignores displacement
  • + preserves base color, roughness, normal

Additional map types

It is however a good idea to also import the additional AO and alpha textures if they are present. Just leave them unconnected.
For the alpha texture you could add an enum in the import settings wether to connect to the transmission slot or create the mix/transparency setup or leave it unconnected. I suggest this because i am unsure how to best do this myself.
With the same logic you could add a checkbox whether to insert adjustment nodes (colorramp between roughness, ...) or leave it as a vanilla_import.

In any case you should make sure that the node layout remains readable. At the moment the alpha setup is placed somewhere in the middle wehen i import:

Otherwise it looks good :)

Hi Florian, thanks for comprehensive feedback!
I had an intensive period at work, but now I'm back.

  1. I've updated the layout placement for nodes.
  2. I will add enum option for having adjustment layers or no (problably with some warning that it will break import export options).
  3. multiplying AO with base is generated but not connected, only base is connected to princiled.
  4. As for transparency: I've had some issues with Transmission (maybe i don't know how to use it properly), but it is still influenced by regular roughness map. Mostly alpha maps are just regular transparency maps, and that works best with mixing the transparency node with principled. If you can show me some real life working example of alpha included in transmission, that would be helpful. Otherwise I'm rather keen on having alpha only mixed with transparency. (Possibly transmission would need to have some specific purpose texture map itself?)

Nevertheless i will try to clean it a little bit more and update later today or tomorrow.

  1. I've added add_adjust_nodes option, and a warning if checked:
  1. Cleand up layout, and optional color_ramps (off by default):

If during discussion we'll end up needing alpha map > transmission option then I can add additional option, but I don't see use of it (I might be bloody wrong :D)

About glTF export/import losing roughness - roughness is supported, and will be stored in the G channel of a shared occlusion/roughness/metalness texture. There may be some issues merging textures on export, although I think much of that has been fixed recently. See the suggested node structure in the in-progress documentation: https://developer.blender.org/D4082.

Has there already been discussion of how to handle AO, or whether an input may later be included on the Principled BSDF node? We're struggling with how to identify and export AO in the glTF exporter: https://github.com/KhronosGroup/glTF-Blender-IO/issues/123. Suggestions for how to align with other addons would be helpful. :)

@Florian Meyer (testscreenings) Hi, back again. I wonder If we could add cleaned up code or wait with future decisions on how to handle AO and warning for breaking the export.