Fix emission calculation for spatial shader

[maxim-sheronov]

I think, that current emission calculation is incorrect, or at least not convenient.
Current logic sums color and value from texture. That means, that you can't use texture as mask to make some parts of mesh emitting. Or you can use black color on texture to not emit, but color become useless. And you can't use it as tint to colorize emission.

I suggest to use texture as mask by multiplying it with color instead of summation.

Also I changed hint color for case, when you don't need a texture.

[Zireael07]

and here I was wondering why my emission mask does nothing in 3.0, thanks man!

[fracteed]

Excellent! I have been meaning to create an issue for this. Look forward to trying it out :)

[reduz]

I thought it might be more useful the current way, but if you prefer this way I'm fine too. PR looks perfect.

[reduz]

I was thinking about it, and I'm not sure if this proposal is better. The rationale is the following:

• Emission and Emission texture in general are not used together, this is rare
• If using a multiplication, you will need to set emission to white besides setting a texture for it to work, I don't think this is very intuitive.
• The only benefit I see this could have is controlling the intensity of the emission, but you can already do this with the energy param

So I think after thinking about it for a while, that this PR is a bad idea, it just makes emission more restricted.

[ghost]

Isn't the current behavior similar to how emission map works in Unity?

[maxim-sheronov]

@reduz, I think the main benefit of multiplying is ability to change Color of emission using texture as mask.
And without multiplication you just can not colorize your emission by script. I think @Zireael07 and @fracteed was talking about that case.

And other engines works this way.
@tagcup, Unity works like in this PR, they uses multiplication.

[Zireael07]

I was mostly talking about being able to use black/white masks to control which parts of the mesh do emission (eg. only a lamp or a decorative border)

[maxim-sheronov]

@Zireael07 yes, but to have control on light color of that emission you need multiplication of emission texture and emission color

[ghost]

Then this would also mean that's how it works with Substance, right?

[reduz]

I'm still not convinced that it's more intuitive, I would probably just add an option to select the behavior..

[ghost]

I'm not exactly sure how this works with Unity or Substance (is there an official doc about it?), but based on this discussion, the current behavior in Godot apparently matches Khronos shader in their gltf exporter repo.

[ghost]

The current Godot implementation is also the one that physically makes more sense: in the absence of emission you should only get albedo (which is the reflected environmental light), and when you turn on an additional light source (which is the emission) it should just add to albedo.(electromagnetic waves add without interacting, neglecting quantum mechanical effects for bosons. also in reality, when you turn on the light source it will heat the object up changing transport properties a little such that the albedo will be slightly different but shouldn't be much.)

[fracteed]

@reduz I would imagine that this is directly related to #10534

[maxim-sheronov]

@reduz, I really don't know in what case it is convinient to have sum of emission color and texture. Maybe you can give an example of such use case? If it can be helpful in some situation, maybe to have an option is better idea. Because with current implementation it is impossible to do coloring of emission with mask.

@tagcup About Unity you can see here
You can see on screenshots, that when they use texture, they set color to white, because of multiplying.

[ghost]

I don't know whick picture you're referring to exactly but this one (from that page) is mostly black:

https://docs.unity3d.com/uploads/Main/StandardShaderEmissiveMaterialInspector.png

[Zireael07]

@tagcup: That's because it's partially a mask. Now if I were to apply this same picture as emission texture in Godot, the whole mesh would be emissive right now in 3.0 (and properly masked in 2.1.3).

[reduz]

Given there are good reasons for both use cases, I will do the following

• Given how Godot exports the parameters, the current usage is more intuitive for creating materials, it allows setting a texture and making sure it works on the first try
• Also given there is a valid use case in changing the emission mask color, I will add this as an extra option in the material.

[ghost]

@Zireael07 I must've misunderstood what Godot is currently doing then. So according to what you're saying, Godot is currently adding albedo as emissive color on top of the given emissive texture

[maxim-sheronov]

@tagcup, as @Zireael07 sad, result is almost black. But on the right side you can see almost black texture with small green and orange squares, and to the left you will see white color

@tagcup, in current implementation Godot use color from texture plus emission color (and maybe plus Albedo). So when you are using emission texture, you can left emission color black. But in Unity you must set it to white, or other color to do coloring.

@reduz, good to know, that there will be an option to select behavior. Thanks. Also I could do this option in this PR. Or do you want to do it yourself?

[maxim-sheronov]

@reduz , I have just added flag to select to use multiplication or not. Could you see is it good enough option to accept?

[hpvb]

Can you please squash the clang-format commit? It doesn't make much sense for the public git history. Other than that, @reduz is the multiplication flag that was implemented after your comment a good fix, should this be merged in this form?

[Zireael07]

What are we doing with this? Still unable to mask my emissions as of alpha 2...

[HummusSamurai]

I think this will be merged eventually since it adds a flag for users to choose whether they want additive or multiplicative mode.