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

Allow nodes to have their post_effect_color affected by lighting #13637

Merged
merged 6 commits into from Aug 24, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Jun 30, 2023

Closes #12071.

This PR adds a new boolean field called post_effect_color_shaded to node definitions. If this field is true, the post_effect_color of the node will be affected by lighting. The default value is false, so nothing changes for existing mods.
Applying lighting to post_effect_color makes sense because post_effect_color is not applied by color multiplication, but by alpha blending. Lighting is done the same way as for the wielditem.

Here are two videos so you can see the effect:
post_effect_color_shaded = false: https://share.grecloud.de/without%20post_effect_color_shaded%202.mp4
post_effect_color_shaded = true: https://share.grecloud.de/with%20post_effect_color_shaded%202.mp4

The videos were taken with Minetest Game, the Soothing 32 texture pack and post_effect_color = {r = 40, g = 204, b = 223, a = 100} for default:water_source and default:water_flowing.
Notice that in the first video, the scene is much brighter at night when I'm underwater than when I'm not. In contrast, in the second video, the brightness remains relatively constant at night, regardless of whether I'm underwater or not. To me, the second video looks much better.

Does this relate to a goal in the roadmap?
Maybe "2.1 Rendering/Graphics improvements".

To do

This PR is a Ready for Review.

How to test

Verify that the post_effect_color of existing nodes still looks the same.

Try adding post_effect_color_shaded = true to the definition of an existing node. Verify that the post_effect_color of the node is now affected by lighting. This isn't only visible at night, but also when you go into deep, dark water.

@Desour Desour added @ Script API @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Concept approved Approved by a core dev: PRs welcomed! labels Jun 30, 2023
@Desour
Copy link
Member

Desour commented Jun 30, 2023

I support this feature.

Copy link
Member

@Desour Desour left a comment

Choose a reason for hiding this comment

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

Code looks fine.

Please add a test node to devtest. And maybe activate the feature for devtest basenode water.

Tested with devtest basenode water.
I'm a bit worried how the color seems to get more transparent the deeper you go. Maybe this is some issue related to srgb nonlinearity.

doc/lua_api.md Outdated Show resolved Hide resolved
@grorp
Copy link
Member Author

grorp commented Jul 16, 2023

Please add a test node to devtest. And maybe activate the feature for devtest basenode water.

Done.

I'm a bit worried how the color seems to get more transparent the deeper you go. Maybe this is some issue related to srgb nonlinearity.

To me, it doesn't seem like the color is becoming more transparent. And it definitely isn't actually becoming more transparent. But it's very well possible that there are problems with linear / non-linear color everywhere in Minetest.

@Desour
Copy link
Member

Desour commented Jul 16, 2023

Here are two screenshots, to show what I mean with the transparency increasement:
screenshot_20230716_182157
screenshot_20230716_182139
It might also just be an illusion. After all the sky is getting darker.
(Will approve when the devtest nodes are fixed.)

@SmallJoker
Copy link
Member

Is there a particular use-case where it would not be helpful to apply the light multiplication? Comparing the two videos, it appears that this change is much better fit to be applied on all nodes with post_effect_color due to more natural appearance.

@Desour
Copy link
Member

Desour commented Aug 5, 2023

Due to how low-alpha colors look with this features (see screenshots above), imo we should not force this option on content creators.

@grorp
Copy link
Member Author

grorp commented Aug 24, 2023

There's another case where modders can decide whether they want lighting or not: The shaded object property of entities.

@grorp grorp merged commit aea9242 into minetest:master Aug 24, 2023
15 checks passed
@grorp grorp deleted the post-color-shaded branch December 18, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client / Audiovisuals Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature One approval ✅ ◻️ @ Script API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Shaded" post effect color
3 participants