Page MenuHome

added emission strength to the principled BSDF node.
Needs ReviewPublic

Authored by Alex Strand (astrand130) on May 29 2019, 4:14 AM.

Details

Summary

I added an emission strength value to the principled BSDF node. The node currently felt incomplete without it requiring the user to add a color blend node set to multiply with a value in order to control emission strength (a bit of a paper-cut if you ask me).

It wasn't that big of a task to do but it will definitely speed up workflows and perhaps even make creating exporters with emission strength (like GLTF) simpler.

Changes include:

  • Added emission strength socket to the principled node
  • Removed hard coded strength from the cycles implementation of principled emission and replaced it with a shader input
  • Updated eevee GLSL code to include an emission strength multiplier

Here is a comparison:


(the very slight washed out difference with cycles is due to bounce lighting)

Thank you for your time!

Diff Detail

Event Timeline

Alex Strand (astrand130) updated this revision to Diff 15667.EditedMay 29 2019, 4:56 AM

Edit: cycles socket for emission_strength default value has been updated to 1.0. (doesn't actually seem to have an effect but doing it just to be safe)

Alex Strand (astrand130) retitled this revision from I added emission strength to the principled BSDF node. to added emission strength to the principled BSDF node..May 29 2019, 6:20 AM
Brecht Van Lommel (brecht) accepted this revision.

This is a good improvement, we can add it to 2.81.

This revision is now accepted and ready to land.May 29 2019, 12:12 PM

This is a good improvement, we can add it to 2.81.

There's absolutely no chance to squeeze it in 2.80? Because now single color socket looks half-baked there making addition of it kinda pointless, because you still have to use Emissive shader to have enough control of it. It would be so much better to see this as a complete feature in milestone release of 2.8.

Brecht Van Lommel (brecht) requested changes to this revision.May 29 2019, 6:28 PM

No, we don't make exceptions, we were already supposed to freeze in mid May.

source/blender/nodes/shader/nodes/node_shader_bsdf_principled.c
43

I forgot about this, but there are some hardcoded socket indexes in this file. These will need to be updated.

This revision now requires changes to proceed.May 29 2019, 6:28 PM

Fixed hard-coded socket indices.

oops accidentally replaced the entire diff. (I don't know how to use git, sorry let me figure this out.)

After some troubleshooting I just did a fresh installation of the blender source and applied the patches.

This revision fixes:

  • Offsets later hard-coded slot indices by 1 to accommodate the new emission strength slot.
  • Cycles: When the value of emission strength value is set to 0.0 node inputs will override the branch out of the emission component.
Alex Strand (astrand130) marked an inline comment as done.May 30 2019, 3:31 AM
Alex Strand (astrand130) added inline comments.
source/blender/nodes/shader/nodes/node_shader_bsdf_principled.c
43

Should be fixed now, thank you for pointing that out.

Alex Strand (astrand130) marked an inline comment as done.May 30 2019, 3:31 AM

Can i suggest that the default values follow what the SubSurface inputs are? the emission strength would be 0 and the default emission colour be white (instead of the inverse?)

Can i suggest that the default values follow what the SubSurface inputs are? the emission strength would be 0 and the default emission colour be white (instead of the inverse?)

That might have been the ideal way to do it, unfortunately I'm afraid that could break older files that only use the emission Color, when the emission strength would default to 0 the material would lack emission. And since this feature seems to be coming post 2.80 that would probably be a much bigger deal then if that change was made during the beta.

Can i suggest that the default values follow what the SubSurface inputs are? the emission strength would be 0 and the default emission colour be white (instead of the inverse?)

That might have been the ideal way to do it, unfortunately I'm afraid that could break older files that only use the emission Color, when the emission strength would default to 0 the material would lack emission. And since this feature seems to be coming post 2.80 that would probably be a much bigger deal then if that change was made during the beta.

@Brecht Van Lommel (brecht) do you have any thoughts about this?

It's possible to initialize the strength to 1 only if the color is black. See versioning_cycles.c, there's other code there that does this kind of versioning patches for nodes.

It's possible to initialize the strength to 1 only if the color is black. See versioning_cycles.c, there's other code there that does this kind of versioning patches for nodes.

Thanks, I'll take a look at it and see what I can do.

Apologies for the inactivity. I've been dealing with classes. Once 2.80 releases I'll make sure to get the backwards compatibility finished and address any remaining issues so it can make it into 2.81.

Congratulations on the 2.80 Release! I hope SIGGRAPH went well.

Can i suggest that the default values follow what the SubSurface inputs are? the emission strength would be 0 and the default emission colour be white (instead of the inverse?)

It's possible to initialize the strength to 1 only if the color is black. See versioning_cycles.c, there's other code there that does this kind of versioning patches for nodes.

I was wondering if this could be a separate patch as it requires adding changes to another file which might introduce its own bugs to fix. The emission strength socket is already complete and I've been using blender with it for a while and haven't run into problems. In it's current state it should work perfectly with existing materials. Since the default material emission strength is 1.0 and its default color is still black there would be no visible changes to the existing emission results.

If the improvements in the defaults can be done separately this patch should be ready for review, and if there are no more issues a possible commit to 2.81.

Thank you.

Alex Strand (astrand130) updated this revision to Diff 17358.EditedThu, Aug 22, 3:28 AM

All conflicts should be fixed and this patch has been made ready for review and merge to 2.81.

@Brecht Van Lommel (brecht) I have some questions. I'm working on the defaults improvements as suggested by @Carlo Andreacchio (candreacchio) using versioning_cycles.c but I'm not sure what to set MAIN_VERSION_ATLEAST() to.

How do I know what version to set it to?

Currently I have:

if (!MAIN_VERSION_ATLEAST(bmain, 281, 0)) {
 for (bNode *node = ntree->nodes.first; node; node = node->next) {
  principled_emit_strength_input(node);
 }
}

With principled_emit_strength_input() as:

static void principled_emit_strength_input(bNode *node)
{
  if (node->type == SH_NODE_BSDF_PRINCIPLED) {
    bNodeSocket *emit_color_socket = nodeFindSocket(node, SOCK_IN, "Emission");
    bNodeSocket *emit_strength_socket = nodeFindSocket(node, SOCK_IN, "Emission Strength");
    float *strength_value = cycles_node_socket_float_value(emit_strength_socket);
    float *color_value = cycles_node_socket_rgba_value(emit_color_socket);

    /* If emission is already used, the strength should be set to one */
    if ((socket_is_used(emit_color_socket)) || !is_zero_v3(color_value)) {
      *strength_value = 1.0f;
    }
  }
}

I'm going to try to test it on a variety of files from the current build-bot. For now I don't want to cause potential compatibility problems, so this initial patch here won't break anything.

You increase the subversion in BKE_blender_version.h, and then use that new value in MAIN_VERSION_ATLEAST instead of 0