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

New lighting curve #5279

Merged
merged 3 commits into from Aug 16, 2017

Conversation

Projects
None yet
7 participants
@numberZero
Copy link
Contributor

commented Feb 19, 2017

Base issue: #5241

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2017

@C1ffisme @MarkuBu created this for the case you don’t like commit references. So... it’s time to play ;)

@MarkuBu

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2017

Right side is your patch, both with default values

bildschirmfoto vom 2017-02-19 18-56-48

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2017

Seems a bit darker, unsure. Post more screenshots, in different places, with different settings... data is needed to decide what is better.

Note: this patch is not intended to fix all the problems at once. It should just fix some obvious problems, like that weird adjustment table...

@MarkuBu

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2017

Yes, it's a bit darker. More screenshots tomorrow

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Feb 19, 2017

I did some walking around, not much, but here is what I feel:

  • Light curve looks noticeably dimmer but more spread out, less contrasting, washed out picture (mixed feeling, dislike night views, has better daylight shadows).
  • I've noticed there is proper darkness in caves now, you can't see stuff anymore (and I like this part).
  • Much brighter night sky (dislike)

default:
oldl2
this pr:
nlc2
default:
oldl
this pr:
nlc
default:
oldl3
this pr:
nlc3
default:
oldl5
this pr:
nlc5
default:
oldl6
this pr:
nlc6
default:
oldl4
this pr:
nlc4

@nerzhul

This comment has been minimized.

Copy link
Member

commented Apr 27, 2017

to close ?

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Apr 27, 2017

Unsure. Maybe polish a bit and merge this instead? What do you think, @nerzhul ?

Much brighter night sky (dislike)

Because it uses block light levels for whatever reason. That’s awkward.

I've noticed there is proper darkness in caves now, you can't see stuff anymore (and I like this part).

Many may dislike that, but... that’s correct)

Light curve looks noticeably dimmer but more spread out, less contrasting, washed out picture (mixed feeling, dislike night views, has better daylight shadows).

Non-trivial. Proper fix is to 1. dim artificial lighting greatly (thus reducing the range), but 2. add automatic brightness adjustment (so that the scene would not look too dark). Well, I would call that MT HDR)

@numberZero numberZero changed the title [for testing] New lighting curve New lighting curve Jun 3, 2017

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2017

@MarkuBu @Fixer-007 to merge or to close?

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2017

No need to rush it, keep it open for more opinions.

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2017

Well, people doesn’t seem to know of this PR.
BTW, with #5913, gamma correction may be moved to the post-processing stage, where it should reside, actually. That may change overall look slightly.

@MarkuBu

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2017

I'm sorry, I don't visit github very often in the last days and I don't have time to test this. From what I saw it doesn't makes a big difference

@paramat

This comment has been minimized.

Copy link
Member

commented Jun 5, 2017

The darker shadows are good, but that's a minor improvement, everything else looks worse.
This is going back to what was bad about the old lightcurve.
Light curve looks noticeably dimmer but more spread out, less contrasting, washed out picture.
Much brighter night sky.
^ I agree and dislike.

Proper fix is to 1. dim artificial lighting greatly (thus reducing the range), but 2. add automatic brightness adjustment

Absolutely no way.

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2017

@paramat can you try different settings? lighting_alpha and lighting_beta affect the curve considerably, and there is gamma correction as well.

@paramat

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

You haven't explained what alpha and beta do, can you add a description to the first post? 'sharpness' does not explain.
So essentially this adds parameters to customise the curve?

@paramat

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

Ok so a, b, c are factors of cubic, square and linear variation, but why are alpha and beta used in a complex way to define a, b, c?
Removed -1 as this is more testing. Can anyone find parameters that match current behaviour?

@paramat

This comment has been minimized.

Copy link
Member

commented Jun 6, 2017

Problem is that the curent light curve has a manually drawn and designed curve which may not be replicable by these parameters. The current curve is linear at the top and changes to exponential at the bottom, all for good reason.

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2017

@paramat

Ok so a, b, c are factors of cubic, square and linear variation, but why are alpha and beta used in a complex way to define a, b, c?

Actually, alpha and beta are derivatives of the lighting function at 0 and 1, respectively (before gamma correction); the values at these points are fixed so that’s enough to define a cubic polynomial. So, larger values give sharper dark or light spots (near minimal and maximal light level).

Problem is that the curent light curve has a manually drawn and designed curve which may not be replicable by these parameters. The current curve is linear at the top and changes to exponential at the bottom, all for good reason.

The current light curve may be good, if you stick to the default settings. But should you ever change anything, all these manual adjustments only mess things up, making lighting look very bad.

Also, the current lighting curve is anything but exponential. I do see 2 almost linear segments, but at lower light levels it is just something weird:
1
Here, blue lines are the default curve with different gamma: 1.0 (black), 1.8, 2.0, 2.2 (light blue). Others are of this PR, with different settings:

  • α=0.8, β=1.6, γ=1.4 [light green]: default settings, suitable for in-cave playing
  • α=0.0, β=1.0, γ=1.4 [dark green]
  • α=0.0, β=0.0, γ=1.0 [dark red]: the reference point
  • α=0.0, β=0.4, γ=0.9 [orange]: more like original.

Note: gamma correction should be the final stage, ideally after rendering. But currently (not in this PR) it is applied before manual adjustment what makes it almost useless.

@paramat

This comment has been minimized.

Copy link
Member

commented Jun 9, 2017

Good to see, and thanks.
I mean 'very roughly exponential' at low levels, as in a curve with rising gradient.
I do like the idea of the curve being customisable.
Here only dark red preserves the brightness at mid-levels that the current has. Light green does too but that has the wrong behaviour near darkness.

@numberZero numberZero force-pushed the numberZero:dev-lighting-curve branch from 2dbf154 to 3d9ec42 Jul 5, 2017

@paramat

This comment has been minimized.

Copy link
Member

commented Jul 6, 2017

The dark red line may be an acceptable replacement for our current curve, it also has the simplest alpha beta gamma values.

@numberZero numberZero force-pushed the numberZero:dev-lighting-curve branch from 3d9ec42 to 0dd5c91 Jul 7, 2017

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2017

@paramat Updated. Also tested these values, looks acceptable in-game.

@paramat

This comment has been minimized.

Copy link
Member

commented Jul 8, 2017

I'll test. This PR has good potential please keep open.
@sofar any comments?

@sofar

This comment has been minimized.

Copy link
Member

commented Jul 11, 2017

If I look at the before/after screenshots, it looks like there is a "green/gray" haze over all the images with this PR. Is this intentional? The cave scene most clearly shows this, but you can see the effect in the other images as well.

@nerzhul nerzhul merged commit 9c8fec8 into minetest:master Aug 16, 2017

1 check passed

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

@numberZero numberZero deleted the numberZero:dev-lighting-curve branch Oct 31, 2017

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2017

Just noticed that by default this seems to practically eliminate shadows beneath trees, which gives the whole scene a flat look. Is that just on my setup, or is anybody else seeing this? @paramat

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

Yes this has been bothering me too. Compare the blue lines (previous light curve) to the dark red line (current default) on the diagram above.
Because now the top few light values are of similar brightness and close to maximum brightness the subtle shadows under trees have become brighter. The previous curve had a steep linear falloff near maximum.
We can tune this by adjusting the default value for the gradient at maximum (beta).

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

However any increase in beta will make lighting generally darker, the current was chosen because it matches the brightness of the previous curve for the lower light levels, and the previous curve is popular due to how it makes everything brighter than the old classic MT light curve. So it's a balance, perhaps the orange line?

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2017

Just noticed that by default this seems to practically eliminate shadows beneath trees, which gives the whole scene a flat look. Is that just on my setup, or is anybody else seeing this? @paramat

I've already posted about this a little higher before merge. Yes, they are generally weaker.
Also, if you do want to change smth - do not touch complete darkness - it is finally awesome in caves, as it should.

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

The previous curve had weaker shadows than the old classic light curve, due to a less steep falloff near maximum light value. The latest customisable light curve made shadows weaker again. I had to add more branches to jungletrees to compensate and get some darkness back in jungles.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2017

α=0.8, β=1.6 looks reasonable enough. I find the settings a bit obtuse :(

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 8, 2017

Alpha has to be near-zero for good reason, the fade to darkness has to be very smooth in an exponential-like way, a high alpha value causes a sharp edge to the lighting area at night which looks bad.

@MarkuBu

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2017

Even Minecraft Alpha had shadows under the tree. I know it is not smooth light, but even without smooth light Minetest has not really a noticable shadow under trees.

minecraft-shadows

On the other hand, if the foliage is dense enough it is pitch black, even by day

screenshot_20171108_210637

@Fixer-007

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2017

Changing should not compromise torch light in any way, is that possible? Torch light as usual - sunlight - deeper shadows?

@sofar

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

One way to solve the issue is to make smooth lighting work up to light level 14, and have light level 15 be a distinct light level, in other words, remove smoothing between 14 and 15.

Another way to solve this is to lower the light levels of 14, 13 and perhaps 12 a bit to make a "gap" in between those levels.

This tuning will get done a few more times at least. We shouldn't be too worried about making adustments, the really important thing is to annotate the decisions in code for future reference.

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

Fixer that's not possible.

sofar both those ideas will cause weird looking results. 15 needs to smoothly and gently blend into 14 and below.

I think it's best to add to the curve generation maths to enable recreating the more complex shape it had before, the blue lines here:

1

Keep the parameters alpha and beta for gradient at bottom and top, but add extra parameters that add a smooth 'hump' in the middle at position delta and height epsilon. This will bring back the subtle shadows while keeping the 'brighter lighting' effect.

@numberZero any ideas for a new function? Perhaps add a gaussian type curve that has zero effect near the top and bottom.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2017

The current light curve at least is not great, and there's not enough leeway for tweaking.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2017

It's possible in fact to create a "saddle" in the middle. By setting alpha = beta = 2, or even alpha = beta = 3.
2 looks pretty good at night and shows some shadow.

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 12, 2017

Yes, but alpha has to be near-zero as explained above, a sharp cutoff at light edge was a problem we had to fix before.

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2017

Perhaps add a gaussian type curve that has zero effect near the top and bottom.

The gaussian has nearly zero effect far enough from the peak. But that should be acceptable. So you can experiment with #6620 (it adds 3 new parameters).

@paramat

This comment has been minimized.

Copy link
Member

commented Nov 12, 2017

Yes near-zero effect far from the peak is fine.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2017

Even after all the changes I find the shadows under trees too weak.
Can somebody point me to the code where the shadows are created in the first place - it's a part of the code I know little about.

@paramat

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 9, 2017

Me too, but try #6696 this will make shadows darker because then 14 becomes visually darker than 15 instead of equal brightness, while using the same curve.
Otherwise, you need to increase the 'beta' value, that is the light curve gradient at max light level, so is how fast brightness falls off with decreasing light level. However it's all a careful balance.
I'd rather merge #6696 than alter the curve again. At least you can now set the curve to your own taste in settings.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2017

I tried #6696 but the change was hardly noticeable. I'll try again.
I was wondering whether we could change light-level of the shadow (at generation time, I guess) rather than (or in addition to) the color the level represent.

@lhofhansl

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2017

I find beta = 3 and alpha = 2 acceptable, even better with boost = 0
But the boost is bad with just artificial light. So b = 3, a = 2 seems best to me.

Or even more extreme, b = 4, a =3. That makes for nice shadows and still works Ok for artificial light.

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 11, 2017

Alpha being significantly larger than 0 causes a very sharp unnatural cutoff at the edges of a lit area in darkness, that was a problem we fixed earlier. The fade to black needs to be as gentle and subtle as possible, alpha = 0 is ideal for that.
Making shadows darker also makes lit areas darker at night, and MT lighting being dark was a long running complaint we addressed with 0.4.16 stable, so it's a balance.

@numberZero

This comment has been minimized.

Copy link
Contributor Author

commented Dec 12, 2017

@paramat @lhofhansl There is also gamma; it does affect things significantly (esp. if its old value remained in the config; try default value in that case, it differs from the old default)

osjc added a commit to osjc/minetest that referenced this pull request Jan 11, 2019

New lighting curve (minetest#5279)
* New lighting curve

* Make polynomial lighting curve

* Update default lighting settings

osjc added a commit to osjc/minetest that referenced this pull request Jan 23, 2019

New lighting curve (minetest#5279)
* New lighting curve

* Make polynomial lighting curve

* Update default lighting settings
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.