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

Multipass shadow mapping #11470

Closed

Conversation

Andrey2470T
Copy link
Contributor

@Andrey2470T Andrey2470T commented Jul 19, 2021

This PR is an extension for #11244 and adds a support for multipass (all-directional) shadow rendering. This means the shadows now are casted not only from the directional light, but also from point light sources as torches, mese post lights, various glow blocks, furniture lamps, street lanterns and etc. Also, there will be a support for casting shadows from glow entities.

At the moment, this PR is very raw and it is even impossible to compile it for testing, so this is a draft.

How it works:

The principles of the work are the same, but with some addings (depending on circumstances, they may be much changed):

  1. Render client map, dynamic objects and colored map textures for each directional light (currently, the only one).
  2. Render client map, dynamic objects and colored map textures for each point light.
    2.1. Loop through each frustum directed from the corresponding face to the outside of point light.
    2.2. Render map blocks from a POV of that frustum to the correponding depth map.
  3. Mix all those three textures into one.
  4. Call drawAll() with the attached shadow texture.

To do

This PR is a Work in Progress

  • Concept
  • Separate class for point light representation.
  • Render to depth maps from each point light.
  • Some a bit refactoring in the current ShadowRenderer and DirectionalLight code.
  • Add cube map texture class container saving all 6 video::ITexture`s.
  • Write separate depth shaders for point lights or use the current ones (?).
  • Limit shadow casting from point light based on its maximum light extension range (currently I don`t know how to).
  • Add higher-level rendering calls (in Game::update()).
  • Some still additional code (?).
  • Test the resulted code.

How to test

Currently testing is impossible.

@lhofhansl
Copy link
Contributor

Wouldn't that be exceedingly slow?

@Andrey2470T
Copy link
Contributor Author

Wouldn't that be exceedingly slow?

Well, it is necessary to render to clientmap, dynamic objects and colored map textures 6 times from each direction (+X, -X, +Y, -Y, +Z, -Z) per a light + rendering to those textures for each directional light. I don't know, are there any ways to reduce count of those rendering calls?

@NathanSalapat
Copy link
Contributor

This is an interesting idea, but I'd have to agree with Ihofhansl it'd probably be much too slow to feasibly work. Of course if you can make it not that would be amazing.

@lhofhansl
Copy link
Contributor

Perhaps there could be some (configurable) limit as to how many light sources it would render.
I'd certainly be very interested in how this looks.

@HybridDog
Copy link
Contributor

The problem could be related to the Many-Light Problem.

@hecktest
Copy link
Contributor

What good is a shadow without a light? You can't just keep shadowing ambient GI, it looks wrong and if you add more shadows on top of it, it'll look even more wrong. This whole thing is being done backwards.

@Andrey2470T
Copy link
Contributor Author

Andrey2470T commented Jul 22, 2021

@lhofhansl I have an only such idea: it is necessary to check if any light source is currently locating within a player's camera frustum and if yes, then add to the point light sources list. However, I suppose a bug could be when that light source is clipped out fully (it is invisible by a viewer), but although it should render shadows casted from a closest its face to sideways of a screen, when it wouldn't in such case.

Also, other optimisation could be e.g. making a check if some faces of some point lights are clipped out by a screen (but the remain are visible), then omit rendering of them to the shadowmaps.

@HybridDog oh, thanks for that documentation! I hope does it not involve an overly hard math?

@hecktest What do you mean by "shadow without a light" ? In this PR the shadows would be casted from point lights that emitting a light in all directions.

return false;

m_pointlight_list.emplace_back(m_shadow_map_texture_size,
// farPlane -- I don`t know how to calculate this parameter.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does anybody know how to calculate a maximum distance on which a point light can extend depending on its light_source intensivity parameter? Somebody suggested to go me to light.cpp and use set_light_data(float gamma), but I don't understand what it calculates and how to use those returned parameters for that.

@hecktest
Copy link
Contributor

Please describe exactly how you plan on accumulating the lights and how they interact with existing node lighting.

Also, do not enable lights without an API to control them. Attaching point lights to every glowing node and object will be excessively slow and not everyone wants or needs direct lighting.

@lhofhansl
Copy link
Contributor

A config option is good enough as a start, an API (presumably you mean a Lua API for mods), of course is better.

@HybridDog
Copy link
Contributor

oh, thanks for that documentation! I hope does it not involve an overly hard math?

It's probably not a good documentation because the main purpose of the paper is to explain a solution algorithm and not the problem. Unfortunately I cannot find good explanations of the many lights problem.

As far as I understand it, the idea which leads to the many light problem is that instead of doing slow ray-tracing for the whole image, it is also possible to calculate a huge number of light sources and then use phong shading (or another simple shading method) with these light sources. If the positions and colours of the light sources are calculated well, the resulting image is a good approximation for the realistic ray-traced image.

In Minetest the many lights are placed by players and not calculated by an algorithm for realistic rendering; nonetheless, I think this is also the many-light problem for which the existing algorithms, e.g. LightSlice, could be helpful. Some of these algorithms reduce the number of light sources so that the rendering becomes significantly faster.
Here are presentation slides for LightSlice, which could be helpful to understand the problem and algorithms: https://www.csie.ntu.edu.tw/~cyy/courses/rendering/13fall/lectures/handouts/ManyLight-II.pdf
It may be possible to adjust the algorithms to calculate shadows and realistic lighting in Minetest. I'm not an expert in this topic, so I don't know if it's difficult to implement.

Some more links:
http://pellacini.di.uniroma1.it/publications/lightslice11/lightslice11-paper.pdf
http://www.aconty.com/pdf/many-lights-hpg2018.pdf
https://www.graphics.cornell.edu/~bjw/lightcuts.pdf
https://www.cs.cornell.edu/~kb/publications/MatrixSampling_SIG07.pdf

@sfan5 sfan5 added the Discussion Issues meant for discussion of one or more proposals label Jul 23, 2021
@hecktest
Copy link
Contributor

A config option is good enough as a start

No it's not, do not change the look and degrade the performance of existing content without a server side opt-out.

@lhofhansl
Copy link
Contributor

@hecktest We're talking about eye-candy here.

I want to be able to control the eye-candy that I want to see on my client without the server interfering with that. Maybe I just like shadows, or reflective water, or smooth lighting, or connected glass, or fancy leaves, etc, etc, etc.

And I most definitely do not want the server to turn on or off graphics feature on my client, without my control over it.

This is eye candy, not a game feature.
At least then we need 3 options for these features: "off", "server controlled", "on". With "off" or "server controlled" as the default setting.

…owmap textures, update m_pointlight_list according to ClientMaps m_drawlist_shadow)
@Andrey2470T Andrey2470T force-pushed the multipass_shadowmap branch from 90cb875 to 013a582 Compare July 25, 2021 09:53
@hecktest
Copy link
Contributor

@lhofhansl
We've already discussed this on many occasions and came to an agreement that we will no longer break any game's visuals. We still need to actually write it down but it's pretty much settled. This PR is not getting merged without a proper API.

@Andrey2470T
Copy link
Contributor Author

Please describe exactly how you plan on accumulating the lights and how they interact with existing node lighting.

Also, do not enable lights without an API to control them. Attaching point lights to every glowing node and object will be excessively slow and not everyone wants or needs direct lighting.

If I understand you correctly, in this PR I am not going to implement any new lighting API in the engine, here I just follow the similar implementation steps which the Liso's PR does, but aim with point lights. I don't attach poing lights to glowing nodes, PointLight class just saves an info about that (shadow frustums, position, map resolution and pointer to the light's MapNode) and that is necessary for rendering its environing mapblocks (within light emitting range radius) to the shadow maps.

We've already discussed this on many occasions and came to an agreement that we will no longer break any game's visuals. We still need to actually write it down but it's pretty much settled. This PR is not getting merged without a proper API.

Why do you think my PR will break the visuals? It is just necessary to optimize the rendering algorithm (about what I talked in the previous post above) to avoid heavy FPS dropping. Also, this feature is marked as "experimental" and any user is in a right not to enable it. :)

@hecktest
Copy link
Contributor

"The user can disable it" is not an argument. Minetest is a game engine and it's an upstream for game developers who have already invested their time making content in it. Much of this content has been made with the assumption that this sort of lighting isn't present (and that the game runs relatively well), and this content will look horrible if you suddenly allow the user to force this sort of poorly thought out realtime lighting on.

The user will enable it thinking if it's exposed, it must be good, and it will make the games look and run like trash, which then will be blamed on the wrong person, and will make Minetest look terrible as a whole. Graphics looking like Unity shovelware running at 10 FPS is the opposite of what Minetest needs at the moment. If people can't help implementing and merging these bad ideas, at least don't make the gamedevs bear the fallout of it.

In addition, it's insane to expect content creators to make artwork without knowing in advance how it's gonna be rendered. They would be right to call it lunacy and look for alternatives or create a fork that doesn't do this to them. We've already lost quite a few game projects due to lack of support on our end and we shouldn't continue letting it happen.

In short, we do not have the right to force an artistic and performance decision like this onto our downstreams, unless you want them all to give up and quit.

Liso's PR is something I volunteered to fix myself (API wise), but that doesn't mean it's okay to keep piling these up. If I wasn't absent when it was being merged, I'd probably find a way to block it too.

I suggest you wait with any renderer features until #11396 is done (it'll take a while) because it will make runtime toggling of such features possible without any half-baked hacks. In addition, it'll probably change the low level rendering API significantly. If you still want to make improvements like these, now would be a good time to learn OpenGL (ES 2.0) and read up on how lighting should actually be done.

In any case you will have to provide at minimum a server-side API function to tell the clients to not use these point lights. When disabled, the performance should be as if they were never implemented.
Of course, for the sake of utility, it would be a better idea to actually control what casts shadows on a per-nodedef and per-CAO basis.

@lhofhansl
Copy link
Contributor

lhofhansl commented Jul 26, 2021

I tried shadows on a slower machine. While not related to that, I agree now: Seeing rain in Mineclone2 together with shadows looks fairly awkward. :)

I still like a (non-default) way to override what the server says to turn on certain client side things. Perhaps at some point we are at a place where no client side features need to be added to the actual engine - I fear this is far off currently.
Also, I certainly fear that we dismiss the possible for the perfect and make no progress - as usually I'd love to be proven wrong about this.

Also:

[...] make the games look and run like trash, [...] looking like Unity shovelware running at 10 FPS [...] read up on how lighting should actually be done.

I said it before and I'll say again: Can we PLEASE turn down comment heat? Some folks spent a lot of time implementing these and trashing these is not conducive to further contributions. What do you look to achieve with this?

@hecktest
Copy link
Contributor

hecktest commented Jul 26, 2021

@lhofhansl

Can we PLEASE turn down comment heat?

Look, nothing I said there was untrue and you know it. If we continue going down this road, Minetest will look and run exactly as I've described. This was not an attack on OP.

Also, I certainly fear that we dismiss the possible for the perfect and make no progress - as usually I'd love to be proven wrong about this.

Here's the thing, is somebody holding our pet hamster hostage saying that we need to make progress, that Minetest needs to have realtime lighting by 2021 or else the hamster gets it? No, this isn't even on the roadmap, it's not even on anybody's private roadmap, why are we pretending this is in any way important?
All I see is hype by a lot of people who don't know what makes a video game look good, how to design a 3D engine, how to code a feature without raking up tech debt, and who will probably get bored and move on to the next shiny thing soon enough.

Note that we've already made these mistakes before. Remember normal and parallax mapping? You should because you and me were the ones who deleted it. (#10313, #10487)
This happened after a user made this wonderful issue: #10307
Let me quote him:

When people enable additional graphics features, they expect better graphics, not worse
The original issue was that months / years ago I enabled it, forgot about it, now I open the game again and everything looks like s***
I'm sorry but it looks absolutely atrocious

You can fool yourself but you can't fool the user, nor you can stop them from telling you the harsh truth eventually, biting my tongue here would just delay the inevitable. And this is the one in a thousand user who actually took the time to file a bug; you don't know how many others just hit "uninstall" instead.

Also, the amount of effort spent on a PR does not matter if the PR does not fit the roadmap, this is why we have this language in CONTRIBUTING.md:

Before you start coding, consider opening an issue at Github to discuss the suitability and implementation of your intended contribution with the core developers.

All unsolicited work is done at the author's risk.
It's also a bit fallacious to accept contributions just to encourage the contributor - what you don't see is how many contributors you may be discouraging by making poor technical decisions. I've talked to quite a few talented people who just gave up on this engine eventually, and I've been very close to doing it myself a few times.

@Andrey2470T
Please explain this too, if you are not going to render lights but only accumulate shadow in subsequent passes, how would your system render this scene?
fourlights

@Andrey2470T
Copy link
Contributor Author

The user will enable it thinking if it's exposed, it must be good, and it will make the games look and run like trash, which then will be blamed on the wrong person, and will make Minetest look terrible as a whole. Graphics looking like Unity shovelware running at 10 FPS is the opposite of what Minetest needs at the moment. If people can't help implementing and merging these bad ideas, at least don't make the gamedevs bear the fallout of it.

I'm obliged to agree with that.

In addition, it's insane to expect content creators to make artwork without knowing in advance how it's gonna be rendered. They would be right to call it lunacy and look for alternatives or create a fork that doesn't do this to them. We've already lost quite a few game projects due to lack of support on our end and we shouldn't continue letting it happen.

First of all, it is pretty obvious for content creators how it is rendered as the given graphic feature (dynamic shadows) simulates the real ones. Also, why would they need to know about that at all? You are saying about mod developers creating mods, games, texture packs, not core ones. Therefore they don't need to know details of implementation of the shadows that is engine-side.

I suggest you wait with any renderer features until #11396 is done (it'll take a while) because it will make runtime toggling of such features possible without any half-baked hacks. In addition, it'll probably change the low level rendering API significantly. If you still want to make improvements like these, now would be a good time to learn OpenGL (ES 2.0) and read up on how lighting should actually be done.

Ok, I'll wait. I have already learned the OpenGL basics recently and experimented with simple Blinn-Phong lighting, the results were pretty impressive. But actually I'm at a position of a noob currently in this area because I have only surficial acknowledges (also in the Minetest Engine itself), not as deep as lots core devs may have and I may not know many nuances. So, I have nothing to object against and I agree naively with you.

In any case you will have to provide at minimum a server-side API function to tell the clients to not use these point lights. When disabled, the performance should be as if they were never implemented.

I don't think it is necessary, furthermore server-side. If any user wants to disable those shadows, then he should disable it fully (casted from directional and point lights). You'd suggest to disable still per an each point light.

@Andrey2470T
Copy link
Contributor Author

Please explain this too, if you are not going to render lights but only accumulate shadow in subsequent passes, how would your system render this scene?

I thought about that, too. As seen in that screen, visible areas of mapblocks from a POV of each that light node would intersect between themselves. Therefore, on a pass of each point light, it would override the same cluster of nodes. As an order of passes is in advance undefined, in some moment when it would achieve a last light, I suppose it would leave an artifact where the shadow shouldn't be (it is under direct lighting from surrounding light sources).

I can't say anything exactly because I have not tested my code yet. I need to write up it, i'll do it in the closest days as I was busy with other things.

@Andrey2470T
Copy link
Contributor Author

Why close? I will just need to rewrite the code with using GL API after #11396 is implemented as we agreed. Probably it will take a pretty long time because I am not enough competent at this and busy with studying.

@sfan5
Copy link
Collaborator

sfan5 commented Sep 28, 2021

WIP pull requests without progress or need for discussion are typically closed after some time. I'm just marking it so we don't forget to close it if nothing new happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues meant for discussion of one or more proposals Possible close
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants