Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redo light.cpp. #4873

Merged
merged 1 commit into from Dec 28, 2016

Conversation

Projects
None yet
@sofar
Copy link
Member

commented Dec 9, 2016

Remake the light_decode_table.

The table starts out without pre-filled in values since those
are always discarded by the code apparently. We calculate a
pseudo curve with gamma power function, and then apply a new
adjustment table.

The adjustment table is setup to make the default gamma of 2.2
look decent: not too dark at light level 3 or so, but too dark
at 1 and below to be playable. The curve is much smoother than
before and looks reasonable at the whole range, offering a
pleasant decay of light levels away from lights.

The display_gamma setting now actually does something logical:
the game is darker at values below 2.2, and brighter at values
above 2.2. At 3.0, the game is very bright, but still has a good
light scale. At 1.1 or so, the bottom 5 light levels are virtually
black, but you can still see enough detail at light levels 7-8,
so the range and spread is adequate.

I must add that my monitor is somewhat dark to begin with, since
I have a hc screen that doesn't dynamic range colors or try to
pull up black pixels for me (it is tuned for accurate color and
light levels), so this should look even better on more dynamic
display tunings.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

before and after shots, with display_gamma = 2.2

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

Note that I've removed all the left over crud to make this code a lot less confusing.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

idk, maybe the light_LUT table is still needed somehow. If only someone explained what it all does.

@juhdanad

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

The skybox also uses light_LUT. This makes sunrise brighter.
(on the right side you can see the new commit)
light
Relevant code fragment: https://github.com/minetest/minetest/blob/master/src/game.cpp#L4263
(where time_brightness is (incorrectly IMO) fetched from light_LUT)
Skybox light should have its own table.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

I like it, will try ingame.
Update: Feels so good ingame, much more vibrant to me, will look much more.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

(EDIT This comment refers to the first version of this PR)

I appreciate this work and support a noticeable increase in brightness, but this is too extreme a change.
Some will like this due to everything being so bright, but depth and atmosphere is important.

(EDIT Screenshots removed)

^ Looking ar Fixer's screenshots too i notice that many areas look 'flat' with now too uniform lighting.

(EDIT Screenshots removed)

^ Notice the sharp cutoff between darkness and low light levels which creates ugly diamonds around windows, and creates sharply defined areas of light. This is why the low light levels need to fade out more gradually into darkness.

If this light curve is roughly linear, i suggest more of an S-curve that flattens out a little at low and high light levels and is steeper than linear in the mid-levels.

Please keep the skybox table unchanged, that really doesn't need changing.

How is the darkness of moonlight affected? if at all, it should not be brighter. I'm concerned by juhdanad's screenshot where the terrain is brighter and looks worse.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

Please keep the skybox table unchanged, that reallu doesn't need changing.

I can't - the skybox is using the exact same table as the game.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

How is the darkness of moonlight affected?

I don't see a real difference myself, it looks pretty bleak - you can walk outside at night, but it's pretty dark still.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

I think we'll need a second unchanged light table for the skybox. Sky light levels are lovely as they are.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

@paramat The current values aren't final IMHO, I spent way too long tuning them and had to post it so I can actually try it around on a few servers to compare.

What is important to note, however, is that EVERY LIGHT SOURCE in the game is ~13-14. And that is purely because the game is so damn dark. What needs to happen is that people start making lights with light levels 7-12 and get that atmoshpere back, because right now nobody has any choice at all - a light level of 7 is just not even worth it.

I do intend to tune the curve more. I'm contemplating making an effective in-game measured curve and tuning it further with that actual data so the curve not only has some meaning but is reproducable.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

I think we'll need a second unchanged light table for the skybox. Sky light levels are lovely as they are.

Quite possible, yes. Of course, we can tune the textures as well to get the same effect.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

I did a lot of sight seeing today and I'm very positive about this PR. It looks much more better with it 👍.
Pics with this PR, ATI HD6870, config in next post. Enjoy, there are lots of different pics, with different lighting.
https://i.imgur.com/rPvMZ6E.png
https://i.imgur.com/sIAKZzC.png
https://i.imgur.com/aAKtsK3.png
https://i.imgur.com/Ai6UfWH.png
https://i.imgur.com/WswlXor.png
https://i.imgur.com/sXPy6mJ.jpg
https://i.imgur.com/1kPxzCa.jpg
https://i.imgur.com/6I4wXpn.png
https://i.imgur.com/ZKPWHmc.png
https://i.imgur.com/F1Xg7JE.jpg
https://i.imgur.com/nFB2iDR.jpg
https://i.imgur.com/9vSqvhC.png
https://i.imgur.com/H1sUtRY.png
https://i.imgur.com/IL8OEz1.png
https://i.imgur.com/njc4wTV.png
https://i.imgur.com/gKvz2k3.jpg
https://i.imgur.com/crWkCE1.jpg
https://i.imgur.com/Ul3Y7BD.png
https://i.imgur.com/5WuTBWS.jpg
https://i.imgur.com/URll0Dv.png
https://i.imgur.com/HeHVyvv.jpg
https://i.imgur.com/AWMzGUU.jpg
https://i.imgur.com/ga3tTKR.png
https://i.imgur.com/LMcGFeo.jpg
https://i.imgur.com/5JuQk4A.png
https://i.imgur.com/COitUI0.png
https://i.imgur.com/F63f957.jpg
https://i.imgur.com/C8qKsaw.png
https://i.imgur.com/ITypkpD.jpg
https://i.imgur.com/nDLJfQW.jpg
https://i.imgur.com/YMEOuOb.png
https://i.imgur.com/heU9lFT.jpg
https://i.imgur.com/Dxlw1EF.png
https://i.imgur.com/zjIK9jh.png
https://i.imgur.com/FoalZhm.png
https://i.imgur.com/MZfJswX.jpg
https://i.imgur.com/WHmk1Vk.png
https://i.imgur.com/XvWKG1n.png
https://i.imgur.com/84eWFjm.png
https://i.imgur.com/w787kiD.png
https://i.imgur.com/OU8Ter8.png
https://i.imgur.com/j01tEEg.jpg
https://i.imgur.com/bSBh322.png
https://i.imgur.com/kOqdS6n.png
https://i.imgur.com/Fih7o2U.png
https://i.imgur.com/HGB4Syv.png
https://i.imgur.com/eA295Gh.png
https://i.imgur.com/UwDS0HM.png
https://i.imgur.com/kVJI3cj.png
https://i.imgur.com/aIt3B8u.jpg
https://i.imgur.com/CAhlGUO.jpg
https://i.imgur.com/6T6IAN4.png
https://i.imgur.com/LcpHAPF.jpg
https://i.imgur.com/FUFiiCS.jpg
https://i.imgur.com/JpDMyt1.png
https://i.imgur.com/xhtzCsJ.jpg
https://i.imgur.com/XqpSQrE.png
https://i.imgur.com/EwhaCZ6.png
https://i.imgur.com/LBmswnt.png
https://i.imgur.com/Ygp6ai3.png
https://i.imgur.com/a43NoLO.png
https://i.imgur.com/O2rTIFv.jpg
https://i.imgur.com/AYmkmKK.jpg
https://i.imgur.com/cFzJfL2.png
https://i.imgur.com/749ICfe.jpg
https://i.imgur.com/CsV7xs0.jpg
https://i.imgur.com/Zgy4aCR.png
https://i.imgur.com/tqOPKnn.png
https://i.imgur.com/h5Wz4FH.png
https://i.imgur.com/oVwPpkx.jpg
https://i.imgur.com/JDp08SH.png
https://i.imgur.com/juRGGxe.jpg
https://i.imgur.com/zqH0J2Y.png
https://i.imgur.com/1o4wdli.png
https://i.imgur.com/rBD73Z4.png
https://i.imgur.com/CV6IeTJ.png
https://i.imgur.com/WxdGVuN.png
https://i.imgur.com/o9rwZnL.png
https://i.imgur.com/yoWXW22.png
https://i.imgur.com/jQHJchr.png

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

@paramat Sofar was using "Tone Mapping" that greatly enhances colours and brightness/darkness in his screenshots. Check my screenshots for more default view - with just basic shaders and waving.
zjik9jh_test
Sofar's screenshot:
sofar
My config I used during testing:

anisotropic_filter = true
bilinear_filter = false
creative_mode = true
enable_damage = true
enable_waving_leaves = true
enable_waving_plants = true
fast_move = true
free_move = false
fsaa = 0
leaves_style = fancy
mainmenu_last_selected_world = 7
maintab_LAST = multiplayer
menu_last_game = minetest
mip_map = true
node_highlighting = box
server_announce = false
server_dedicated = false
trilinear_filter = false
viewing_range = 240
fps_max = 0
address = minetest.org
remote_port = 30006
fixed_map_seed = 
mg_name = valleys
tone_mapping = false
mainmenu_last_selected_TP = 1
noclip = true
texture_path = 
sound_volume = 0.7
enable_shaders = true
fog_start = 0.7
enable_bumpmapping = false
enable_parallax_occlusion = false
enable_waving_water = false
generate_normalmaps = false 
@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2016

note I generally have tonemapping enabled - this explains why my screenshots are a bit brighter than Fixers.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

Did comparison of scene depending on time of date, the only difference is shadows and lightbox at transitional period, everything else looks the same. Upper image is this PR, bottom is default game.
AT 12:00
with3-12-00
without3-12-00

AT 18:30
with3-18-30
without3-18-30

AT 19:00
with3-19-00
without3-19-00

AT 24:00
with3-24-00
without3-24-00

Another nice night example:
with1
without1

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

Yeah just saw on IRC logs that sofar's screenshots have tonemapping.
Nevermind my suggestion for a second table for skybox, once the light curve is tuned well we probably won't need a second table.

In a way Minetest lighting will inevitably be fairly dark, because it is limited to 16 nodes and the light must gently merge into darkness, which means it must be somewhat exponential at the lower end.
So that is why i suggest an s-curve that keeps a gentle exponential low end, then rapidly rises in the middle and flattens near the top (or is close to linear as in the graph below) so that light doesn't fall off so rapidly near a light source.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

Measured light intensity before/after in image editor (brightness level in HSB mode), based on this https://imgur.com/a/6r7BX. Take with grain of acid.
zhahq0aterstststs

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

^ Good example of how an s-curve will be better and how this is too steep at around 3.

Shadows in daytime will need to be considered too, for example i have increased the density of rainforest so that some places are dark, it would be bad to lose this darkness.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2016

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

Too bright, but will get tuned.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 10, 2016

Yeah, my intent is to go for an S- curve in a next revision. The "step" between 4 and 10 will be larger, and outside that it will be less or the same. Gonna have to get some time though.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 10, 2016

This is the effective light curve (give or take roundage) of the latest push.

  • 0-2 are unchanged
  • 3-5 are a lot darker (by 20 or so)
  • between 5 and 8 the curve is a lot steeper
  • the 9-15 range is only very slightly adjusted, it's more flat

This takes out the sharp drop that was at light level 2. It's an S-like curve but only at the bottom half. I didn't want the top half to be curved much since we don't have any issues there I feel.

Interesting to see how people like this.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

Graph looks good.
I agree the top half being linear is fine.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

I made a mod to create an underground chamber with a white floor where i can either place a meselamp or dig a shaft to the surface to let in a column of sunlight.
Here i am using the current light table, more screenshots coming.
Interesting to discover that LIGHT_SUN 15 has the same brightness as LIGHT_MAX 14, both are 255:

sun_nosmooth

^ This is with a column of sunlight falling in the centre, level 15, with level 14 as edge-neighbours.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 10, 2016

That's the method I've been using as well. You can clearly see that the light level attenuates really fast - anything lower than 9-10 is really dark already.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 10, 2016

Your values are effectively:

255, 250, 203, 159, 127, 103, 84, 71, 64, 59, 52, 40, 31, 24, 15, 10

Edit: how I measure this: open image in gimp, desaturate (luminocity), then dropper with average sample radius of 10 or so.

@ShadowNinja

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

I like the extra brightness. IMO, light sources are very weak in Minetest.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

Latest version of PR.
Meselamp or single column of sunlight on pure white blocks underground.
No shaders, all other visual settings disabled.

Old.
New.

Lamp, no smooth:

lamp_nosmooth

new_lamp_nosmooth

Lamp, smooth:

lamp_smooth

new_lamp_smooth

Sun, no smooth:

sun_nosmooth

new_sun_nosmooth

Sun, smooth:

sun_smooth

new_sun_smooth

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

grow forests underground with just torches

They weren't growing saplings with torches, saplings grew in any light level, even darkness. When we fixed this we chose the same required light level as every other crop and plant has needed right from the start

As i wrote above, we have meselamps now in default for doing this, they're not expensive to make and mese ore is common. On IRC (i'm staying away) you are giving the impression players cannot grow trees underground, they can, just not in an unreasonable way with torchlight, which has never been able to grow any other plant.

We made the function global so that it can be overriden to require a lower light level.

@Zeno-

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

Why don't you join IRC then. The commit/change has changed minetest completely and not in a good way IMO

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Here's the original issue minetest/minetest_game#704 which was supported.
PR minetest/minetest_game#705 but Megaf and Ryan-Nolan objected in issue minetest/minetest_game#724
So we made the can grow function global for overriding in minetest/minetest_game#729
Note that only 2 people objected and more supported the change.
Feel free to open a MTGame issue, maybe we could add mese torches?

@Zeno-

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

The subject line of those PRs don't even suggest the huge and game changing implications :/

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Oops linked to engine issues, fixed.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Sorry, i have chronic anxiety and am feeling too nervous to be arguing on IRC at the moment.
Also, from reading IRC logs you are being a little aggressive towards me.

We could possibly add a level 14 mese torch to Game, as meselamps are quite bulky and not as pretty sitting inbetween plants and saplings.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Just to be clear, this is not a case of torches once being bright enough to grow something and now they're not. From the beginning torches have never (correctly) been bright enough to grow plants, it's just that the light checks were missing from saplings, cactus and papyrus.
Fixing bugs like this will inevitably irritate someone.
Mese ore is not rare at all (Meseblock is) and to grow plants in darkness i think it's reasonable to go through a little more effort than just using torches.

@Zeno-

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

paramat, mese is not common enough to do what's required. Plus, mese lamps are UGLY

Edit: even though it's not optimal because old worlds will probably remain broken, at least make another light source that can grow trees and looks more organic :/

@Wayward1

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

FWIW, I think juhdanad's suggestion seems reasonable:

http://irc.minetest.ru/minetest-dev/2016-12-27#i_4771652

<juhdanad> There could be a 'small torch' for dimmer lighting. And normal torch became more expensive then.
...
<juhdanad> Yes, normal torches can remain untouched if we introduce another variant. And normal torch would cost two coals from now on to compensate the brighter visible light.

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

We have some time to figure out the changes needed to balance mintest_game, but I think it's time to merge this change now.

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2016

I'm not sure about two torches... I like more this block from darkage as a dimmer kind of MESE for decoration: https://github.com/davisonio/darkage/blob/master/textures/darkage_lamp.png
No idea what to do with torch...

Redo light.cpp.
Remake the light_decode_table.

The table starts out without pre-filled in values since those
are always discarded by the code apparently. We calculate a
pseudo curve with gamma power function, and then apply a new
adjustment table.

The adjustment table is setup to make the default gamma of 2.2
look decent: not too dark at light level 3 or so, but too dark
at 1 and below to be playable. The curve is much smoother than
before and looks reasonable at the whole range, offering a
pleasant decay of light levels away from lights.

The `display_gamma` setting now actually does something logical:
the game is darker at values below 2.2, and brighter at values
above 2.2. At 3.0, the game is very bright, but still has a good
light scale. At 1.1 or so, the bottom 5 light levels are virtually
black, but you can still see enough detail at light levels 7-8,
so the range and spread is adequate.

I must add that my monitor is somewhat dark to begin with, since
I have a `hc` screen that doesn't dynamic range colors or try to
pull up `black` pixels for me (it is tuned for accurate color and
light levels), so this should look even better on more dynamic
display tunings.
@rubenwardy

This comment has been minimized.

Copy link
Member

commented Dec 27, 2016

Please can we move the discussion of torches no longer growing plants to a MTG issue? It's unrelated to this PR as this PR only changes appearance not behaviour

@sofar

This comment has been minimized.

Copy link
Member Author

commented Dec 27, 2016

If there are no further comments I'll merge this in a bit.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 28, 2016

👍 May still need tuning but merge is ok by me to get a lot of testing. Torches, lava and flames will be too bright for a short while, we can tune those to be visually brighter than now, but with lower light levels.

@sofar sofar merged commit 094a5a7 into minetest:master Dec 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sofar sofar deleted the sofar:light branch Dec 28, 2016

@beyondlimits

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2016

Although the new curve looks (based on game screenshots) better than previous, for me it's still a bunch of arbitrarily chosen values. Can I know what formula it's based on?

@DS-Minetest

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2016

i think, the light is a bit too bright now

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2016

I fight it perfect now. Can we simply adjust the default gamma to counter what some folks are seeing?

@beyondlimits

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2016

I have looked at the code and I regret now.

 *  @note This function is not, currently, a simple linear to gamma encoding
 *        because adjustments are made so that a gamma of 1.8 gives the same
 *        results as those hardcoded for use by the server.

How server can have any use of graphical settings?

And shouldn't be the code as simple as:

void set_light_table(float gamma)
{
	gamma = rangelim(gamma, 1.0, 3.0);

	for (int i = 0; i <= LIGHT_MAX; i++) {
		light_LUT[i] = (u8)(255 * powf((i + 1) / (LIGHT_MAX + 1), 1.0 / gamma));
	}
}

AFAIK no way to overflow an u8.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 29, 2016

i think, the light is a bit too bright now

I fight it perfect now. Can we simply adjust the default gamma to counter what some folks are seeing?

Torches are too bright now yes, but remember that we are going to make changes to node light levels in MTGame, meselamp will stay at 14 to be the default 'max brightness' light and growlamp for plants. Torches, fire and lava will have their light level reduced to something more visually suitable, while still being perhaps slightly visually brighter than before.

@Zeno-

This comment has been minimized.

Copy link
Contributor

commented Dec 30, 2016

Torches, fire and lava will have their light level reduced to something more visually suitable, while still being perhaps slightly visually brighter than before.

Possibly

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jan 2, 2017

rubenwardy added a commit that referenced this pull request Jan 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.