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

[No squash] Corrections to entity and node light calculation #12741

Merged
merged 4 commits into from Oct 30, 2022

Conversation

x2048
Copy link
Contributor

@x2048 x2048 commented Sep 4, 2022

This PR fixes two things:

  1. When smooth light is disabled, makes visible brightness of nodebox and other draw types the same as NDT_SOLID at the same light level:
    image
  2. Corrects computation of brightness for entity glow property to work on the light levels (0..LIGHT_MAX):
    a. Glow is added to the map light level at the node position, before converting to the color space
    b. Additional brightness is added at the final stage in the same was as for light sources

The reason for a single PR is that light calculation for both scenarios is based on getInteriorLight function

Fixes #12686 in a formally correct way.

To do

This PR is Ready for Review.

How to test

In devtest:
Test 1:

  1. Throw various light sources on the ground in a dark place.
  2. Brightness of the item entities must reflect the light source level

Note, depending on your hardware, dim light sources (1-3) may not be visible in full darkness.

Test 2:

  1. Place arrays of sandstone, sandstone slab and sandstone stairs in sunlight and check that the brightness level is the same
  2. Place arrays of sandstone, sandstone slab and sandstone stairs near an array of light sources as show below:
    image
  3. Brightness of stairs and slabs must be exactly one level higher than on solid nodes. This is because solid nodes use light levels of the air nodes above them, which are 1 manhattan unit further away from the light source (as displayed above).

@@ -881,7 +881,7 @@ void GenericCAO::updateLight(u32 day_night_ratio)
if (!pos_ok)
light_at_pos = LIGHT_SUN;

video::SColor light = encode_light(light_at_pos, decode_light(m_prop.glow));
video::SColor light = encode_light(light_at_pos, m_prop.glow);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glow is a value between [0, LIGHT_SUN] but encode_light treats the increment parameter as a value between [0, 255] for the effective brightness, hence the decode_light call here was more correct than the current code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added comments explaining how glow is used now:

  • Glow now impacts light level of the entity in the getInteriorLight call
  • A small boost is added to the final color to make entities brighter at the same light level

@sfan5 sfan5 added the Rebase needed The PR needs to be rebased by its author. label Oct 20, 2022
@Zughy
Copy link
Member

Zughy commented Oct 20, 2022

@x2048 rebase needed

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SmallJoker SmallJoker added One approval ✅ ◻️ and removed Rebase needed The PR needs to be rebased by its author. labels Oct 28, 2022
@SmallJoker SmallJoker merged commit 485b3b1 into minetest:master Oct 30, 2022
@x2048 x2048 deleted the entity_glow branch November 4, 2022 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.6 changed object glow drastically
4 participants