material.enhancedLightmaps #5741
Conversation
|
I think I would make it simpler by now... material.lightMapMode = THREE.MultiplyMode; // default;
material.lightMapMode = THREE.AdditiveMode;
material.lightMapMode = THREE.OverlayMode; // I think this is the mode you guys useWhat do you think? |
|
I mean, that code doesn't work. I'm just suggesting that API instead. |
|
We also have material.combine which is for the |
|
@mrdoob Ahhh, misunderstood that ;-) Has been a long day... sounds great. I'll have a look at changing the PR to accomodate your proposed API |
|
Hm, regarding the lightMapMode - how would you pass in params like the "cutoff", "intensity" or "center"? |
|
Okay, so now it works as you suggested and uses pretty much the same mechanics as
|
|
The lightmap creation pipeline must be consistent with the lightmap rendering pipeline. @AVGP Do you have a reference for this lightmap creation/rendering technique? Why not implement something simple, instead, like |
|
@WestLangley Would you mind explain what you mean by that? So you want me to also add |
My first question was: Where did your lightmap blending technique come from? Based on your answer to that question, I may have follow-up questions.
No, I suggested using 2X instead. You still get your feature implemented. With the current approach, a lightmap color of 1 results in no change, and it only darkens. With 2X, a lightmap color of 0.5 results in no change, and the lightmap can lighten or darken the scene. |
|
Ah, sorry.
That comes from our personal experimentation and a link that I can't find anymore. I think it's derived from the Quake 2 sources, but I'm not sure anymore.
Looking around a bit, that indeed seems to be the common mode for lightmaps. Edit: Turns out, our original implementation is indeed wrong and was supposed to work like 2X... In any case: Thanks for the hint, I completely missed that and will change in favour of 2X |
|
..then again, the Overlay blending looks pretty good. Can we keep both? |
OK, that is what I expected.
Sorry, I do not think that would be appropriate. The lightmap creation pipeline must be consistent with the lightmap rendering pipeline. If a user uses a modeling tool to generate a lightmap, the shader used to render the lightmap must be consistent with the modeling tool's assumptions. To the best of my knowledge, 2X and 4X are common, but it would be great if you -- or someone else -- could verify that. I do like your idea of supporting lightmaps that can lighten a scene, as well as darken it. Thank you for the suggestion. : - ) |
AVGP
force-pushed
the
enhanced-lightmaps
branch
from
643f102 to
e2d916c
Compare
Oh, I agree. But as of now, Three.js ignored that and always used multiply. So why is it a problem to have more options? If you'd want to keep it consistent, then I'd argue that we should consider making 2X the default, but I think it should be an option to choose different blending modes.. To elaborate a bit more on this: We're having an internal model format, based on the JSON format for example. |
There is no problem with the default, 2X and, perhaps, 4X -- or any other commonly-used formats -- provided someone can demonstrate that they are commonly-used.
That would not be backward-compatible.
We cannot modify the library so it supports your proprietary approach. What if everyone wanted us to do that? |
Overlay blending is a normal blending mode. Just as additive blending, difference blending, etc. It isn't even about us... that can stay a customisation, fine. But I still disagree that having "multiply" and "2X" (and maybe even "4X") is fine, while adding other (standard) blending modes isn't. |
|
Ah wait, I think I see where we're missing each other... What you say is "proprietary" is supposed to be a "overlay blending" implementation, which, looking at it closely, seems to be borked. So for your original point no proprietary mode" we agree. I wonder if you'd be opposed to the (properly implemented) overlay mode. You haven't said anything against the additive blending so I assume you are okay with standard blending modes, correct? All I want is a few nice options for lightmap blending, so that everyone can benefit from it. :) |
|
The lightmap operations we should be supporting are the operations commonly used in the community -- Overlay mode and add mode are not in use as far as I know, so they should not be options. |
|
Ah, thanks for explaining that to me. It is a pretty big surprise to me, as I'm used to think of lightmaps as nothing else but a texture with luminance information it it, that is blended onto the diffuse texture. As far as I know that's how it's generally referred to in OpenGL, isn't it? I wondered why we're discussing the blend modes, as I (still) think one should be free to choose the blend mode that fits one's application and an engine shouldn't necessarily limit the available options (that's how I understood @mrdoob's comment #5741 (comment) as well). I'm glad that you take the patience to explain this to me, as I'm brand new in the Three.js world and happy to learn more, thanks! 😃 |
|
Your way of discussing it is fine. We were just cross-talking, and I was trying to get us all on the same page. There is no right way. : - ) We just need a way to identify which mathematics operations to use when the applying the lightmap. That is why the word I do not care what the names are, but I think we should support |
|
Ah, I'm coming from OpenGL game development, hence I was more used to And coming from game development, I've seen all sorts of operations to apply lightmaps - depending on the exact effect you want to obtain. As to justifying the subset, I can find this which mentions Anyway, I'll leave the code as is for now and let @mrdoob decide if he's happy with the supported operations or not. |
|
If I had known you were going to leave the code as-is, I would not have invested all the time in this discussion. For the record, I do not support what you call |
|
See, I appreciate that you made me aware of the missing 2X and that you For the sake of getting us more useful lightmaps now, I'll just remove 2014-12-10 21:09 GMT+01:00 WestLangley notifications@github.com:
|
|
@WestLangley Just to make it clear: I appreciate your explanations and I didn't aim at adding something "for us" into Three.js - I wanted more flexibility for Three.js and I got very confused between #5741 (comment) and your position. Sorry if I caused frustration or was a nuisance for you. |
That is not the reason your proprietary formula is not appropriate. The lightmap creation pipeline must be consistent with the lightmap rendering pipeline. That means the mathematics that is used to render the lightmap must be consistent with the mathematics that is used to create it. Since your shader code is not standardized, other users will not be creating lightmaps consistent with it. Remember, you are free to create your own
Yes, yes, And you are adding the ability to support lightmaps that both darken and lighten. A very good suggestion. |
And that's what the PR contains now and I'm fine with that. I'll give you my rationale for having more operations to try to clarify this bit:
We're not doing anything special when creating our lightmaps. They're normal lightmaps, generated with Blender. Just as you'd do when you blend multiple textures - there's no consistent way to apply them as it just depends on what you want to get out of it. IMHO you can get different, interesting results with using different ways of applying lightmaps (though most of the people will want I agree that a viable workaround is to go for a ShaderMaterial and that's why I removed the other modes from the PR to get Off-topic to this discussion: I really hope I didn't frustrate or annoy you. That was never my intention, I just didn't see the need for the consistency you brought up as it didn't quite fit my mental model of lightmaps. |
|
Strong debate is healthy. : - ) It does appear that there are remnants of You do not need to include the build files -- only the source. Please see the Wiki article How to Contribute to three.js. |
AVGP
force-pushed
the
enhanced-lightmaps
branch
from
ea9d204 to
aad1855
Compare
|
Okay, can you take another look? I think I've fully cleaned up the PR now... |
|
There are still remnants of overlay, plus tab/space problems. Please fork the current version, start with a clean PR, and try again. |
|
Just double checking... So lightmap goes before specular? (Seems like that's the only change) |
|
@mrdoob, that still wouldn't be right. The issue is this line here: It modulates the diffuse lighting by gl_FragColor.xyz. But that means that gl_FragColor.xyz needs to be at this point purely the diffuse surface color with no light map contributions. Thus you can not modify gl_FragColor before this point with the light map. And after this point, you can not use gl_FragColor as the surface color because it will now contain specular and other lighting contributions. Thus you need to pull out a diffuseColor. I can make a PR based on the diffuseColor adoption I've already done if that helps. |
|
Or you can combine lightmap into lights_phong_fragement.glsl... that could avoid a diffuseColor variable. |
That'd be great :) |
Very well said.
Yes. Exactly.
No. Image-based lights are additive. Light maps are an approximation, and are assumed in three.js to be multiplicative. It actually depends on how they are created, and requires further discussion. How they are rendered must be consistent with how they are created. |
|
@WestLangley wrote:
Remember that there is a different between emissive textures, which are purely additve and not mobulated, and light maps. They are actually different things. Emissive maps are light emitting from the surface outwards. Thus they are not modulated by the surface color. Light maps model incoming light to that surface and are thus reflected by the surface, thus they are modulated by the surface color. There is a lot of confusion between emissive maps and light maps -- they are distinct in this way. The reason why light maps work best when modulated by the surface is in the case of a detailed colored surface. If you do not modulate a light map by the surface color, the light will need to be as high resolution as the surface to capture the texture detail. But the light usually isn't varying this much, it may not even change at all compared to the colored surface texture resolution. If you modulate the incoming light by the surface texture color you can use very low resolution light maps and still get great results. I am speaking of experience here. |
|
@WestLangley The difference between emissive maps and light maps is similar to the difference between emissive and ambient -- or at least it should be. Emissive is purely additve to the result fragColor, where as ambient should be modulated by the surface color. Ambient light is also not treated properly in the current Three.JS phong shader in my opinion: In the above ambient is modulated by ambientLightColor -- that is not correct. The ambient of a material is not a surface value, it already has a surface color which is diffuseColor. Rather if there is a per-material ambient value, it should be considered to an incoming ambient light value -- thus it should be added to ambientLightColor (the global ambient) before the combined result is modulated by the surface color. Having ambientLightColor being separately modulated by per-material ambient is really strange and not physical. |
|
I can do a PR to add a proper emissiveMap to Three.JS -- we have one of these in our Clara.io fork of ThreeJS and it works great. This is a good compliment to a lightMap and will alleviate the confusion many people are running into trying to use a lightMap for both emissive and incoming light modeling purposes. |
|
@WestLangley I think that image-based lights are sometimes additive, such as in the case where the surface is purely reflectively, such as a mirror. But otherwise IBL contributions are modulated by the surface. Here is V-Ray doing IBL, we are providing the input, but it is V-Ray doing all the calculations: https://clara.io/view/a82f8a80-8a5e-4e61-8faf-3de9eb4313c2/render |
I said nothing about emissive maps.
No, they do not model incoming light. They are baked representations of reflected light. I wonder if you are using different terminology than three.js uses. There is a difference between a light map and image-based lighting. |
|
@WestLangley wrote:
When I mentioned emissiveMaps, I was responding to your comment that lightMaps are purely additive. In my understanding, they are not additive, rather they modulate the surface color. But if you want a map that is purely additive, then an emissiveMap is what you want. @WestLangley wrote:
This is the fundamental disagreement. Emissive maps are a model of outgoing light. LightMaps in my understanding are a model of incoming light, and need to get modulated by the surface color prior to be reflected. Many game engines do modulate the light map by the surface color (albedo.) Unity does, but I can not find easy proof. But in this documentation from Valve's Source engine on LightMaps, it states as the second sentence: "The color values in the surface's albedo are multiplied by the color values in its lightmap." |
No. The direct lights modulate the diffuse reflectance of the material, |
|
@WestLangley wrote:
The issue is this is not a physical thing. There is only one albedo/base color/diffuse color of an object. If we want to move towards a physical model, I'd recommend moving away from using this. It is not part of Physically-based shading models. |
I said they are assumed in three.js to be multiplicative. |
|
I have always recommended that a user set the |
|
@WestLangley I understand that some want flexibility. In physically-based renderers, they have done away with separate albedos for separate incoming lights types. Do you think most use cases can be replicated via using the per-material emissive color or an emissive map? This would align more closely to Unreal Engine 4's physically-based material system (and others): I can easily PR this as well. |
|
For the large majority of users, removing If you do so, please make the PR separate from other changes, which can be discussed in separate threads. In the PR, the ambient light color should modulate |
I agree. I never understood why that was needed. |
|
PR that removes |
|
Please remember to update the docs or at least ping me if you're pulling out public facing properties. I'm sending in a PR with the update to the docs. This broke the material browser in the docs which is how I found out about it. |
This is a suggestion that I think could be useful.
material.lightMapcurrently can only darken a material, not lighten it up.We're using a customised version of Three.js where lightMap is actually working like this:
Together with a few config options (center, falloff and intensity) that is wrapped into the
material.enhancedLightMapproperty. If this property is present, it triggers the shaders to work as described above.Because of the hiding behind the new property it won't break backwards compatibility.
What do you think? For us it's pretty helpful, if we're modelling things like sunlight coming through windows (example http://beta.archilogic.com/JAHT4L)