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

Continuation of x2048's implementation of 'Godrays' #13881

Merged
merged 2 commits into from Dec 22, 2023

Conversation

lhofhansl
Copy link
Contributor

@lhofhansl lhofhansl commented Oct 9, 2023

This PR adds x2048's implementation of "Godrays".

Currently this is added to the Bloom stage, should probably get its own stage.
This also only shows godrays when the sun is inside the frustum.

To do

This PR is Ready for Review

  • Add a separate setting
  • Add a separate rendering stage...?
  • Make effect server-controllable (like shadow, saturation, etc)

How to test

Enable bloom, volumetric lighting, set the volumetric light strength on the server, and look at the sun/moon through or next to some object.

player:set_lighting({volumetric_light={strength = 0.3}})

@lhofhansl lhofhansl added @ Client / Audiovisuals WIP The PR is still being worked on by its author and not ready yet. Testing needed labels Oct 9, 2023
@lhofhansl
Copy link
Contributor Author

Did anyone get a chance to test this as is?
I do not want to spend a lot of time on this if we're not happy with how it looks/behaves.

@Calinou
Copy link
Member

Calinou commented Oct 22, 2023

Tested locally, it works as expected. Looks great in action 🙂

I suggest changing the color to fade towards orange during sunrise/sunset, as godrays are always white right now.

simplescreenrecorder-2023-10-23_01.11.13.mp4
screenshot_20231023_011021 webp screenshot_20231023_011035 webp screenshot_20231023_012523 webp

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Oct 24, 2023

I moved the volumetric light to a separate rendering stage.

@Calinou When I try I see that the rays are color adjusted already. Do you have tone-mapping or maybe directional fog enabled? Tinting currently only works when dynamic shadows are enabled, otherwise the light direction is not available.

@lhofhansl lhofhansl force-pushed the godrays branch 2 times, most recently from bc9aa91 to 5f312e5 Compare October 24, 2023 00:36
@lhofhansl
Copy link
Contributor Author

lhofhansl commented Oct 24, 2023

I think the separate stage only works accidentally. When I separate the stages and have individual settings and then turn bloom off the rays do not work.

Perhaps I'll do what x2048 did for dynamic exposure and keep it all in stage after all.
(Sorry I have so little time to look at this)

Edit: I see... The actual mixing of the colors happens in the "second_stage" stage. I would need to mix-in the "god-ray" texture separately (and then all do the down-sampling separately). I somehow expected the staging would be "cleaner" of hard-coded dependencies between them, instead the final stage is hard-coded and needs to know about all the other stages to work.

With all that it might be best to keep god-rays in the same stage as bloom + auto-exposure, so that it benefits from the existing down-sampling.

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Oct 25, 2023

OK... Added a (clientside) setting. And really put volumetric lighting in its own stage.
Note that now it is no longer downsampled and hence it looks "sharper" now.

Please have a look - the rays are now sharper and more "grainy". Update: I softened that effect now with H/W filtering.

If we need to downsample this aswell, I'd have to think about how to do that. I could add another 4 textures to downsample the godrays as well... Or go back and write it all into the BLOOM texture (and then volumetric lighting would not work without bloom enabled).

@lhofhansl lhofhansl changed the title x2048's Initial implementation of 'Godrays' Continuation of x2048's implementation of 'Godrays' Oct 25, 2023
@lhofhansl
Copy link
Contributor Author

The strength of the effect is now configurable.
I think in the end this should be server controllable, just like shadows, saturation, etc.

@lhofhansl lhofhansl removed the WIP The PR is still being worked on by its author and not ready yet. label Oct 25, 2023
@lhofhansl
Copy link
Contributor Author

NM. Made it server controlled.
Please have a look.

@lhofhansl
Copy link
Contributor Author

@Calinou The light direction is only available when dynamic shadows are enabled - which is needed for tint.

@oong819
Copy link
Contributor

oong819 commented Oct 28, 2023

Can OpenGL ES device have this?

@lhofhansl
Copy link
Contributor Author

What do folks think about the API?
In set_sky this could be part of the fog table instead, or it could be part of the set_lighting API.

@Calinou
Copy link
Member

Calinou commented Oct 31, 2023

Can OpenGL ES device have this?

I believe Minetest only supports OpenGL ES 2.0 when it comes to OpenGL ES, and it's too limited to support advanced shaders like this one. Most mobile devices would struggle with Minetest's shaders anyway.

Copy link
Member

@grorp grorp left a comment

Choose a reason for hiding this comment

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

I like the godrays. They look very good, especially when they are tinted / affected by light direction. Some things:

  • I think the downsampled godrays looked a lot better. The current non-downsampled version still looks grainy, even with filtering.

  • Would it be possible to extract the light direction code from the dynamic shadows code, so that tinting the godrays works even if dynamic shadows are unavailable (Android) or disabled? It doesn't make sense to me that volumetric lighting partially depends on dynamic shadows.

  • I'd prefer this API:

    -- reasons for set_lighting:
    --   - volumetric light is a light thing, not a sky thing
    --   - consistency: dynamic shadows and automatic exposure are also in set_lighting
    player:set_lighting({
       -- reasons for a separate table:
        --   - maybe we want to expose more parameters of the algorithm in the future
        --   - consistency: dynamic shadows and automatic exposure also have separate tables
        volumetric_light = {
            strength = 0.5,
        },
    })
  • While you call the effect "volumetric light" in most places, the setting is named "Enable volumetric lighting". I think it would be good to settle on one spelling. Never mind, it doesn't matter.


By the way, I also tested the initial version of this PR on Android (with OpenGL ES 2). It worked nicely, but decreased the framerate from 60 to 40 FPS.

@lhofhansl
Copy link
Contributor Author

I think the downsampled godrays looked a lot better.

I agree. Lemme think about it. The easiest solution would be to require blooms to be enabled on the client (no separate setting), and the server would still have to enable it.
I could also look into drawing into the same texture depending on what is enabled.
In both cases volumetric light strength and bloom strength would not be able to be controlled independently.
Doing down/up scaling twice will likely not perform well at all.

Would it be possible to extract the light direction code from the dynamic shadows code

Yes. I'd prefer to do that in another PR though.

I'd prefer this API:

Yeah. That's better, I'll do that.

So the main part is figuring out how do to down sampling efficiently while remaining independent of bloom.
Open for suggestions! :)

@Calinou
Copy link
Member

Calinou commented Nov 1, 2023

I assume requiring bloom isn't too much of an issue if you want to use godrays. I can't see too many use cases for using godrays without bloom. That said, if you really wanted not to use bloom while using godrays, you could enable it but change its strength to 0.0.

PS: We should probably look into tuning the bloom formula in a separate PR as it has that "dreamy" look out of the box, which tends to not look great in mid-brightness scenes. Bloom should ideally be reserved for the brightest pixels in the scene (it's meant to replicate real world camera/eye behavior).

@AbduSharif
Copy link

AbduSharif commented Nov 1, 2023

Most mobile devices would struggle with Minetest's shaders anyway.

Any mid-range device (and even most low-end ones nowadays) are fine actually.

Minetest have become pretty optimized and it's configurability helps alot.

@lhofhansl
Copy link
Contributor Author

you could enable it but change its strength to 0.0

In the scenario above I cannot control bloom strength separately from "godray" strength, though. For that they have to be rendered to separate textures, which then have to be scaled up/down separately.

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Nov 1, 2023

OK... So specifically I would turn the volumetric strength into a boolean instead and render into the bloom texture (kinda what @x2048 had :), just the server config is new).
In another PR we could then make the bloom effect configurable.

If I render into the bloom texture the advantage is that auto-exposure would also just work. Otherwise I'd have to reason about that too.

Or... I just down/up sample the volumetric light as well and make it client optional.

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Nov 2, 2023

OK... Had some time during lunch...

  • god rays are now scaled together with bloom, and require blooms to enabled
  • godrays strength can be controlled with the API (0 is off)
  • client bloom intensity also affects godray intensity (I cannot avoid that)
  • API is switched to set_lighting

I.e. player:set_lighting({volumetric_light={strength = 0.3}})

I feel that this is good to go :)

@lhofhansl lhofhansl force-pushed the godrays branch 2 times, most recently from 0a303f6 to 4d19ffc Compare November 3, 2023 17:52
@lhofhansl
Copy link
Contributor Author

Oops. Forgot to add the changed volumetric shader to the commit. Done now.

@velartrill
Copy link
Contributor

looks fantastic, but there's a small issue with particles. currently, particles always seem to act as a physical obstruction, even particles that are intended to represent light effects. i'm not sure how best to fix this since MT doesn't have a material system at all, let alone for mere particles; the simplest/laziest solution that comes to mind is picking a cutoff glow value and treating particles with a glow >= the cutoff as invisible for the purposes of godrays.

the color also needs to be controllable. consider the following screenshot, from a game set on a planet orbiting a blue giant:
godray color mismatch

@lhofhansl
Copy link
Contributor Author

Thanks @velartrill
I think these should be follow-up PRs to control the scope of this one.
Volumetric light is optional and a game/server should not enable it if it does not fit into the game.

@lhofhansl
Copy link
Contributor Author

lhofhansl commented Nov 4, 2023

It turns out, I can relative easily allow the tint of the rays to be configurable.
Still not sure I want to do this as part of this PR.

And... Would the color be different for day/night? Probably yes.

Since we're talking about sun-color, I can now also see this in the set_sun/set_moon API.
Maybe we should not overthink this either. (It could be part of the sun, fog, sky, or light definition :) )

In the end I think I'd like merge this one as is, it's ready for review, and tackle any changes in other PRs - as long as we agree on the API.

@lhofhansl
Copy link
Contributor Author

How do folks want to take this forward? I can add custom tints to the API now, or we can add that later in a different PR.

@lhofhansl
Copy link
Contributor Author

Ping.

I think this is ready. Feature is server controllable. Additional API to control tint, etc, can be added later.

@lhofhansl
Copy link
Contributor Author

Rebased. And fixed some nits. Let's get this in so it does not bitrot.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

the effect looks splendid

doc/lua_api.md Outdated Show resolved Hide resolved
src/client/game.cpp Outdated Show resolved Hide resolved
src/client/game.cpp Outdated Show resolved Hide resolved
src/client/render/secondstage.cpp Outdated Show resolved Hide resolved
src/script/lua_api/l_object.cpp Show resolved Hide resolved
@lhofhansl
Copy link
Contributor Author

Addressed comments + rebase.

- Make volumetric light effect strength server controllable
- Separate volumetric and bloom shader pipeline
- Require bloom to be enable, scale godrays with bloom
@lhofhansl
Copy link
Contributor Author

Same changes, squashed to two commit: @x2048's original code, and my changes on top of them.

@lhofhansl lhofhansl merged commit e0d4a9d into minetest:master Dec 22, 2023
13 checks passed
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.

None yet

8 participants