minetest
minetest copied to clipboard
Ambient light and server control for it
This PR allows to add ambient light for environment per player that exists independently from the natural and artificial light sources. It is controlled by the new property ambient_light = <ColorSpec> of player:set_lighting. It makes possible to adjust brightness and color of poorly eliminated places e.g. caves, nether biome and any other closed spaces.
Youtube Demo Video https://m.youtube.com/watch?v=Lfk4XTqnZEU
Some old screenshots
https://github.com/minetest/minetest/assets/25750346/23f3d464-dff2-4e70-bd1d-5c1490c6e0eb
{luminance = 8, color = "green"}
{luminance = 5, color = "yellow"}
{luminance = 3, color = "red"}
This PR is Ready for Review.
How to test
- Enable shaders in the settings.
- Select
devtestgame and start the world. - Enter
/set_lightingcommand that will show the menu with scrollbars adjusting some lighting parameters. - Adjust the red, green, blue channels of the ambient light.
I feel like the names of the
forceUpdateMapblockMeshes()function and them_update_mapblock_meshesmember variable flag aren't descriptive enough. In the same time, they're quite long already and I don't have any better ideas.
I think they sound fine. What this method does is enable that flag notifying in such way updateDrawList() about the need of adding the mesh update tasks (addUpdateMeshTask()) for the most actual m_drawlist. The same happens but only the one mapblock mesh is updated when e.g. a player made cracks on nodes.
Another thing I would like to know, while I am not really concerned about it, how expensive is the forced mapblock meshes update?
That causes a slight delays within a few seconds since as I said above it updates meshes of all currently loaded mapblocks.
Adding a fullbright mode where everything is visible without torches is a thing which should be done right away since 5.4.2013 or since even before that date. I think a configurable ambient light makes a fullbright mode possible.
https://dev.minetest.net/TODO#Other https://github.com/minetest/minetest/issues/6509 https://github.com/minetest/minetest/pull/6844
This does not seem to look the same with shaders enabled / disabled:
Enabled:
Disabled:
I don't know what we can do about that / if we can do something about that.
We could make this a shader-only feature (like other lighting parameters). Then we could also stop caring about the performance of this if shaders aren't available.
Minetest might eventually want to require shaders (but that would require knowing what the extent of breakages would be).
I'm also starting to doubt whether min(x, 1) is the correct thing to do... This effectively means that "light" is capped at "full intensity" on all channels (as opposed to capping the "reflected" light). This enables fullbright (by setting ambient light to full white) and "reduces tint", which is why I think it's good.
We could add a second color after multiplying with texture color for said "tint".
@HybridDog should the light color be clamped after or before multiplying with the texture color, or something else entirely?
We could make this a shader-only feature (like other lighting parameters). Then we could also stop caring about the performance of this if shaders aren't available.
I don't know exactly how the mapblock meshes, i.e. the vertex buffers, are generated; I assume they are not generated every frame and a change in ambient lighting would need a regeneration of all mapblock meshes for the change to take effect if the ambient light cannot be configured with a shader uniform. Perhaps Irrlicht has an option to set the ambient light in the fixed function pipeline, which would be faster than regenerating mapblock meshes.
@HybridDog should the light color be clamped after or before multiplying with the texture color, or something else entirely?
It should not be clamped if the shader outputs colours as float values and post processing is enabled since the second stage shaders already clamps the colour. If it outputs 8-bit colours, which can happen if floating point textures are unsupported by the GPU I think, or if post processing is disabled, it should clamp the colour at the end before storing it in gl_FragData[0].
Clamping RGB values directly gives bad results, as explained at https://bottosson.github.io/posts/gamutclipping/. I think clipping colours in the OKLab colour space or a similar colour space could give good results but may be difficult to implement. hdr-toys implements some gamut mapping algorithms. A simpler approach, which may also give better results than a simple RGB clamping (I haven't tested it thoroughly), is clamping the saturation in the YCbCr colour space and then the RGB values:
// Clamp the saturation in YCbCr before a clamp in RGB for a better colour
// preservation
vec3 clamp_saturation(vec3 color)
{
vec3 yuv = rgb_to_ycbcr(color.rgb);
yuv.yz = clamp(yuv.yz, vec2(-0.5), vec2(0.5));
return ycbcr_to_rgb(yuv);
}
color.rgb = clamp_saturation(color.rgb);
color.rgb = clamp(color.rgb, vec3(0.), vec3(1.));
Thanks for the reply.
I don't know exactly how the mapblock meshes, i.e. the vertex buffers, are generated; I assume they are not generated every frame and a change in ambient lighting would need a regeneration of all mapblock meshes for the change to take effect if the ambient light cannot be configured with a shader uniform.
Yes. (It does not need a full regeneration of the meshes, though: Just updating the vertex colors should be enough.)
Perhaps Irrlicht has an option to set the ambient light in the fixed function pipeline, which would be faster than regenerating mapblock meshes.
It seems I gave up on looking for this too quickly: There is indeed a setAmbientLight function. Just calling this seems to do nothing, though; we might also have to enable lighting, or to install a dummy light source with only ambient color.
I'm also not sure whether this does what we want, though. The problem is that we set vertex colors to emulate light in the fixed function pipeline. These vertex colors would be multiplied with the ambient light if I'm not mistaken, when we would really want them to be added to the light, then multiplied with the texture color. If something is pitch black, it will stay pitch black, for example.
It should not be clamped if the shader outputs colours as float values and post processing is enabled since the second stage shaders already clamps the colour.
That seems reasonable. However, I'm confused by what our shader is currently doing:
- It's clamping the RGB (which you shouldn't do)?
- It's clamping before applying tonemapping? Does this not defeat the purpose of tonemapping?
- It's applying linear RGB -> sRGB conversion, but were the colors we're working with ever converted from sRGB -> linear RGB in the first place? (Should we convert ambient light from sRGB to linear RGB?)
If it outputs 8-bit colours, which can happen if floating point textures are unsupported by the GPU I think, or if post processing is disabled, it should clamp the colour at the end before storing it in
gl_FragData[0].
Ick. Fair enough.
Clamping RGB values directly gives bad results, as explained at https://bottosson.github.io/posts/gamutclipping/. I think clipping colours in the OKLab colour space or a similar colour space could give good results but may be difficult to implement. hdr-toys implements some gamut mapping algorithms. A simpler approach, which may also give better results than a simple RGB clamping (I haven't tested it thoroughly), is clamping the saturation in the YCbCr colour space and then the RGB values: [...]
Thanks for the suggestion, I'll give this a try.
Side note: If we don't do RGB clamping, then this feature can not be used to implement fullbright, but that's probably how it should be; ambient light simply does not work like that. The implementation of fullbright would be very similar, though.
@nauta-turbidus thoughts?
I'm also not sure whether this does what we want, though. The problem is that we set vertex colors to emulate light in the fixed function pipeline. These vertex colors would be multiplied with the ambient light if I'm not mistaken, when we would really want them to be added to the light, then multiplied with the texture color. If something is pitch black, it will stay pitch black, for example.
It sounds wrong to me, too. The vertex colour should be added to and not multiplied by the ambient colour.
[The second stage shader is] clamping the RGB (which you shouldn't do)?
As far as I know, Minetest currently uses only colours with values in $[0,1]$ (i.e. SDR), so the clamping may be redundant. Perhaps it was added there.
It's clamping before applying tonemapping? Does this not defeat the purpose of tonemapping?
"tonemapping" in Minetest is just a visual effect and does not map HDR values to SDR since the input is already SDR. Before this commit the clamping happened after the tonemapping; I don't know why the order is changed now.
It's applying linear RGB -> sRGB conversion, but were the colors we're working with ever converted from sRGB -> linear RGB in the first place? (Should we convert ambient light from sRGB to linear RGB?)
It's converted from sRGB (approximated by 2.2 gamma) to linear RGB and later back to sRGB, so only bloom and the exposure adjustment are done in linear RGB.
I've rebased the branch and deleted the ambient light calculation for the vertex buffers of the mapblock meshes by such way allowing to calculate it only in the shaders. The reason of that is animate() behaves undefinitely. It can update the ambient light, but can not do it on some mapblocks. I struggled to find out it today, but unfortunately I failed. Besides, I think it should be a "shader feature", so needing users to enable shaders to run it.
As to clamping the output colors, I can confirm what it is even not just recommended, it is even necessary. Because without that, it provides a such result:
With that:
Work well for nodes and objects, quite intuitive API (aside of alpha, but more on that later). It however doesn't work for the particles. As you can see, despite a fullbright (255, 255, 255) ambient light, particles are rendered as black when no other source of light (like the sun) is present.
Now to the alpha... I guess throwing an error is fine, and leaving it as a field that can be expected to be there and yet does nothing would be even worse, but I still find it funny that modder has to provide a specific value, as any other errors out. Again though, I have no better idea than that, and this is quite foolproof I think.
Now to the alpha... I guess throwing an error is fine, and leaving it as a field that can be expected to be there and yet does nothing would be even worse, but I still find it funny that modder has to provide a specific value, as any other errors out. Again though, I have no better idea than that, and this is quite foolproof I think.
You are right that this is a bit of a hack. It stems from the fact that our "colorspecs" and Irrlicht's SColor support alpha, but we don't need or want alpha here. Hence we enforce that the alpha be "unused" / that colors be opaque.
The proper solution would be to have "RGB-only" colorspecs / SColor, of which our current colorspecs and SColor would be extensions, but I think that is out of scope for this PR.
It should be noted that in practice, the modder does not "need to provide a specific value" - opaque colors are the default for colorspecs. If you pass {r = r, g = g, b = b}, a will default to 255, as it should, and you will be fine. If you pass "green" or "#00FF00", you will also get an opaque color. If you pass "green#42", expecting the #42 (alpha) to do something special, you will get an error, as you should. (The only case where there isn't (and can't be) a default, and which hence is slightly ugly, is when you use colorspec integer form: 0x00FF00 will throw an error, since alpha is 0. You would have to use 0xFF00FF00.)
As for the particles, I think one option would be to add a particle shader. (This might be a good idea for performance-related reasons anyways.) I might make a PR for it.
@Andrey2470T excepting the particles, the only thing I'm unsure about concerning this PR is when to do the clamping. I think I have to agree with HybridDog that if we want to have somewhat physically realistic light, we need to do the clamping after applying the ambient light, rather than clamping the light color. The upside is that strongly colored ambient light remains strongly colored, even in full brightness. (Whereas when clamping the light, you get the somewhat "weird" effect of colors only "shining through" the darker a place is.) The downsides are that clamping after applying the light to the texture (1) makes using this for proper fullbright impossible (2) modders may prefer the "physically incorrect" behavior.
I hence propose what I think may be an interesting "middle ground":
- Ambient light is physically accurate. It is not clamped before being applied. The final colors are already clamped.
- We add another "clamped light" color, which is added to the light color, the result of which is clamped before being applied. The ambient light is added after this.
- (Of course this would need to be documented properly.)
Thoughts?
@appgurueu In my opinion, ambient light is inherently "physically inaccurate". There's no such thing in real world.
The main thing that we should want about ambient light is that it
- is flexible and
- doesn't break other (actual) light sources
This PR feels like it fulfills both these, but I'm not 100% sure about (2), and I don't really understand how would your proposed middle ground work, how would it look, and how it would be better than what we have now in this PR.
@appgurueu In my opinion, ambient light is inherently "physically inaccurate". There's no such thing in real world.
Indeed ambient light is intended as an approximation of "scattered" light. I think it also makes sense to choose the "better" approximation among two approximations.
Still, it is a well-defined term in computer graphics, and we probably don't want to diverge from the established light models when we use the same terms.
I don't really understand how would your proposed middle ground work, how would it look, and how it would be better than what we have now in this PR.
My proposed middle ground is to extend the API to support two colors, say ambient_light and clamped_light.
The color of a fragment would then (very roughly) be calculated as (min(light + clamped_light, 1) + ambient_light) * texture_color. That is, clamped light is added to normal light, then clamped; ambient light is just added and not clamped.
The final color is clamped (much later) anyways.
As "special cases", my proposal supports:
- Fullbright as clamped light = (1, 1, 1), ambient light = (0, 0, 0).
- Arbitrary "clamped light" (what this PR currently implements) by setting clamped light = (0, 0, 0).
- Arbitrary ambient light, by setting clamped light = (0, 0, 0).
In general, my proposal would support a mix of clamped light (which "complements" existing light sources so to speak) and ambient light, which is unconditionally added on top.
For an example of what the difference between "ambient" and "clamped" light may look like, see Andrey's comment here. In particular, see how the "tint" of the ambient light (in this case full red) goes away in broad daylight:
This may or may not be what modders want. My proposal would give them more flexibility, at the risk of being more confusing and slightly more complex.
@appgurueu OK, I'm almost convinced.
What would be the result of setting clamped_light = {1,1,1} and ambient_light = {1,1,1}?
What would be the result of setting
clamped_light = {1,1,1}andambient_light = {1,1,1}?
(min(light + clamped_light, 1) + ambient_light) * texture_color = (1 + ambient_light) * texture_color = 2 * texture_color - it would double the RGB components for the texture color. (If we want to give some advice in the docs: It's probably usually not desirable to have ambient light + clamped light > 1 for any component.)
What would be the result of setting
clamped_light = {1,1,1}andambient_light = {1,1,1}?
(min(light + clamped_light, 1) + ambient_light) * texture_color = (1 + ambient_light) * texture_color = 2 * texture_color- it would double the RGB components for the texture color. (If we want to give some advice in the docs: It's probably usually not desirable to have ambient light + clamped light > 1 for any component.)
Since clamped_light is added to light from other sources, it is very easy to exceed 1 on the sum by adding the ambient_light and "overbrighten" the texture_color anyway...
@appgurueu I thought, may it be better to just set the alpha value to 255 always independently on what this value a modder provides in ObjectRef::l_set_lighting instead of throwing out the error? If the alpha is ignored, it is ignored by just omitting this parameter. If throw out the error, it means the engine expects from a modder setting 255 value and in simple words it is already not ignored.
As to the applying the ambient light to particles, I don't think it needs in an additional particle shader. The ambient light can be applied in Particle::updateLight method where the end vertex color is calculated. When it is added, it needs in a check whether enable_shaders is true. However, I need to find out how to apply it, since 'blend_light' is used here instead 'final_color_blend'.
As to clamping the output colors, I can confirm what it is even not just recommended, it is even necessary. Because without that, it provides a such result
Both with and without clamping looks bad in my opinion, for example because of the gamma-incorrect rendering. If light is clamped early, shades are lost. My personal preference of the two options is no clamping.
No ambient light
With early clamping and #ffffff ambient light
Without early clamping and #ffffff ambient light
My proposed middle ground is to extend the API to support two colors, say ambient_light and clamped_light.
I think having these two colour settings may be confusing or difficult to understand for the mod creator.
~~As evidenced by the issue by the "particles are unaffected" issue the approach of this PR is currently quite wrong.~~ Why isn't the ambient light applied in a post-processing step? We have the infrastructure for this so it should be used.
Unless the ambient light intentionally does not apply to clouds. Haven't seen that mentioned anywhere.
Edit: As pointed out on IRC, post-processing can't make light where there isn't. So what I said doesn't work.
For the record, with a sufficiently flexible shader pipeline, this probably could be applied in a post-processing step (consider separate textures holding renders using the raw texture data and the light values to be applied, which are then merged in post-processing). That would most likely be premature optimization though. Simply adding some light in the fragment shader is insanely cheap, especially compared to the rest of the logic, so I see no need for this to be in post-processing.
FWIW from the IRC conversation my opinion is that this PR should restrict itself adding an ambient light level (not color!) that's simply added to light calculations. Seamless to implement with particles too, because it's essentially identical to glow. Color tint adjustments can then be done via #14091.
FWIW from the IRC conversation my opinion is that this PR should restrict itself adding an ambient light level (not color!) that's simply added to light calculations.
I disagree. Ambient light is standard in lighting models, and for a reason.
Color tint adjustments can then be done via #14091.
That's still less flexible: You can not have a different ambient light color and light color that way.
Color tint adjustments can then be done via #14091.
As appgurueu suggests ambient light is a thing for a reason. Consider irl example: direct sunlight is white/yellow while ambient light (light scattered by the atmosphere) is blue. This makes shadows bluish. I also plan using this PR for my Perfect City game, where I'd use white/yellow for artificial light inside buildings and orange light for sodium street lights outside. #14091 won't let me do that.
Why exactly are you now testing with #000 and #fff only? It's as if this only had a boolean option. There's no need at all that the minimum and maximum allowed values always provide sensible (in your opinion) results. You simply cannot predict into what kind of environment the modder or game developer is applying the lighting. It's in their control. You need to trust them.
When getting more and more into physically accurate effects, you start to have to be careful with the magnitude of values you use if you want sensible results. Just like in color: If you want green, you do not want to use #00ff00 even though it's "maximum green". If you want to make an object move quickly, you don't assume the maximum speed available makes sense.
In the API, the most important thing is that the modder's intent is clear. This means that industry standard terms and concepts should be used when possible, and if the modder, with such clear intent, wants an insane looking result, you can only assume that's exactly what they want.
I like the option of having both, actual ambient light and a minimum light level available separately.
It would be preferable to do these types of color transformations in the linear color space in order to avoid unwanted hue changes (which generally look very ugly), or use some other method to similar effect.
A simple yellow ambient light won't let you simulate sodium street lights well because their light spectrum desaturates colors. My personal preference is light calculation with linearised sRGB values, and ambient light whose "color" is a 3x3 matrix and whose brightness depends on the direction of the surface's normal vector. Directional ambient light can be implemented with spherical harmonics or with spherical gaussians. Matrix lights make it possible to configure the saturation and hue of light color and I expect directional ambient light to look good in Minetest because it makes it possible to shade the six sides of a node with different brightness.
Matrix lighting is easy to implement for ambient light with (probably) costly sRGB EOTF and OETF executions: matrix_ambient_lights.diff.txt
Here are screenshots for two extreme-case examples.
A matrix which sets the red and green channel to the same value:
A matrix which sets all channels to the same value, making the ambient light grey:
A simple yellow ambient light won't let you simulate sodium street lights well because their light spectrum desaturates colors.
Hmmm sad. HybridDog, thank you for your explanation, but I know too little about 3D programming and light models to understand these changes. Would be perfect if the lua API could do the same thing.
Why exactly are you now testing with #000 and #fff only?
Who is?
There's no need at all that the minimum and maximum allowed values always provide sensible (in your opinion) results. You simply cannot predict into what kind of environment the modder or game developer is applying the lighting. It's in their control. You need to trust them.
Agreed. You're seeing this the wrong way around however. The reasoning is not that "hey, the edge cases should provide 'sensible' results"; instead, the reasoning is "with a particular implementation - clamping the total light before applying it - #fff lets you implement fullbright". In general, this lets you implement kind of "complementary" lights to existing light, that do not exceed full brightness together with the existing light. It is however not how ambient light generally works, hence why I was proposing a "middle ground" of parameterizing ambient light both before and after clamping, giving modders maximum control, and whacking the fullbright feature request at the same time. Whether it's the best solution for that, or whether fullbright should be an entirely different feature, is still to be discussed however.
In the API, the most important thing is that the modder's intent is clear. This means that industry standard terms and concepts should be used when possible, and if the modder, with such clear intent, wants an insane looking result, you can only assume that's exactly what they want.
I agree with you (and HybridDog) here. This "complementary" / "clamped" ambient light should not be called ambient light. Ambient light is generally not clamped. I slightly regret making the suggestion of clamping.
I like the option of having both, actual ambient light and a minimum light level available separately.
Technically redundant but sure, why not, given that we already have minimum light level related things.
It would be preferable to do these types of color transformations in the linear color space [...]
I agree with HybridDog here that fixing this is likely to exceed the scope of this PR:
Side note: To be precise, the addition is wrong/inaccurate because colours are in the non-linear sRGB colour space, but this is another issue which cannot be fixed in this Pull Request.
From brief samples, it seems to me that Minetest generally does way too many things in sRGB which it should be doing in linear color space. Meticulously working through this probably isn't trivial.
I was under the impression that slapping in some conversions in at the wrong places is likely to just break things in different ways, but perhaps I'm wrong.
My personal preference is light calculation with linearised sRGB values, and ambient light whose "color" is https://github.com/minetest/minetest/issues/13864 and whose brightness depends on the direction of the surface's normal vector.
This is an interesting proposition.
Either way, it would be great if we could move forward with one solution or another (rather than bikeshedding this to death), even if not perfectly satisfying to all parties, since this seems to be a very popular feature (which makes sense given that it lets you drastically change the ambience of a scene). I would like to see this in 5.9, especially given that the implementation itself is not particularly complex (though the discussion surrounding it is a bit heavier). It is one of the features giving the most "bang for the buck" in terms of implementation complexity.
To permit later changes which are not backwards-compatible, it is possible to mention in the documentation that the ambient light options are experimental and may be changed in the future.
In my opinion, the purpose of a fullbright mode is to see the whole world without placing torches. This visibility can be implemented with and without clamping.
@HybridDog I don't feel like your "grey" matrix ambient light looks good. It makes stuff grey that isn't. Grey light shouldn't recolor stuff.
In my opinion, the purpose of a fullbright mode is to see the whole world without placing torches. This visibility can be implemented with and without clamping.
Fair enough, though I think everything bleeding strongly into white would not really be a satisfying fullbright mode for me, and the weaker you make the ambient light, the less you get the "full" part and the more the brightness varies...
@HybridDog I don't feel like your "grey" matrix ambient light looks good. It makes stuff grey that isn't. Grey light shouldn't recolor stuff.
Well, it'd be just an option which you don't have to use. All "simple" ambient light usage should still be encodable (via diagonal matrices).
And in a "dystopian" setting (totally not half of all game jam games), such a feature might very well make sense.
this will be awesome to see this in Minetest engine
It's not perfect, but it works, and various effects can be achieved.
Particles work now.
Edit: If we keep bikeshedding to death what is the perfect solution, we'll never do anything. This works, doesn't break anything, etc. Considering that the rest of the lighting models has serious problems, I wouldn't demand from this PR any more advanced implementation than what it has now. This looks sane for sane values, and allows a wide range of graphical effects.