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

In D4971#113787, @Carlo Andreacchio (candreacchio) wrote:

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.

In D4971#113806, @astrand130 wrote:

In D4971#113787, @Carlo Andreacchio (candreacchio) wrote:

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.

In D4971#113927, @Brecht Van Lommel (brecht) wrote:

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.

In D4971#113788, @Carlo Andreacchio (candreacchio) wrote:

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

In D4971#113927, @Brecht Van Lommel (brecht) wrote:

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.

@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

@astrand130 hi, are you planning to update this patch? Looks like it is close to being finished.

Hi, I would really appreciate this feature, specially to play well with GLTF export pipelines

Apologies for the delay. Life has been a little messy. I am working on updating this to the new versions of Blender right away!

In D4971#201823, @Brecht Van Lommel (brecht) wrote:

The implementation looks good, but I do worry about add-on compatibility.

Just checking in to see if this needs to be merged in 2.91 soon so the addons can be adapted. I can revert the defaults but it would seem like an inconsistency and papercut UX inconvenience for legacy exporter add-ons when most people (not using GLTF) already have to set up their materials manually or use a custom solution due to broken/outdated infrastructure and the lack of PBR support in most interchange formats anyways. Would be a more ideal solution to update the exporters that support principled shader emission.

Thanks!

Reaching out to the comunity for input on compatibility and defaults: https://blender.community/c/rightclickselect/0cgbbc/

@Brecht Van Lommel (brecht) Are we able to get this merged in 2.91 soon?

It's my plan to look at this in the coming weeks. If you want to update the add-ons to take into account emission strength (which we'll need to do regardless), that would help speed things up.

In D4971#208943, @Brecht Van Lommel (brecht) wrote:

It's my plan to look at this in the coming weeks. If you want to update the add-ons to take into account emission strength (which we'll need to do regardless), that would help speed things up.

Awesome! Not very familiar with Python. I can hack together some simple scripts, will have to learn a bit to do that but I will take a crack at it. Thanks!

Status Update: Working on updating the exporters.

Collada is almost done, just need to fix a clamping issue on import. The format is not the best candidate to receive emission strength due to it's "texture OR color" restriction.

Obj is under way. The current MTL code produces/loads fundamentally broken materials, I decided it would probably be best to rewrite most of it and support a PBR extension to drop this legacy technical debt before adding emission strength if time permits, otherwise I'll just copy the emission multiplier to the current MTL code.

Created tool to inspect Obj Mtl validity using TinyObj https://github.com/astrand130/ObjTest

My PC is currently under maintenance so I will return to my main work on this as soon as possible (Sep 4). Thanks!

@Brecht Van Lommel (brecht) I have investigated other exporters and am currently working on updating them. It has come to my attention that material export functionality in some exporters (particularly OBJ and FBX) are in rather poor condition and I would like to fix them. Before I am able to do that it would be very helpful if this patch could get merged and D8777 could get reviewed soon so these addons and exporters can be updated. Thank you.

Obj MTL Importer/Exporter has been updated. https://developer.blender.org/D8868

@Julien DUROURE (julien) this needs updates in glTF import/export. As noted above @astrand130 is working on updating importers and exporters, but as I understand it there is no glTF code yet, so up to you two to figure out who does it.

New issue created for glTF emission strength here: https://github.com/KhronosGroup/glTF-Blender-IO/issues/1216

I am going to need to update the Collada importer to handle emission strength via RGBM encoding similar to how I am doing for OBJ in another patch.

Other than that OBJ and Collada is done. I will look into adding it to FBX tomorrow.

Thanks!

@Brecht Van Lommel (brecht) I finished adding it to the following importers/exporters (ready for review):

DAE: https://developer.blender.org/D8955
FBX: https://developer.blender.org/D8953
OBJ: https://developer.blender.org/D8868

Would it be a better idea to set the socket ID to 22 (next unused socket id) rather than 18? It's breaking all existing scripts that reference socket 18 otherwise. Or does the visual order depend on the socket ID?

As explained here, scripts should use the name, there is no such thing as a unique numeric socket ID.
https://devtalk.blender.org/t/implement-new-node-parameters-without-breaking-existing-scripts/15433/6

Hello. Importer & exporter glTF is now using the new socket ( see feca8c528979 and f3cf3a16e989 )