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 Cascaded Shadows #13956

Closed
wants to merge 3 commits into from

Conversation

lhofhansl
Copy link
Contributor

@lhofhansl lhofhansl commented Nov 5, 2023

From #13833:

Upgrades the current shadow mapping implementation to cascaded shadow maps.

Benefits:

  • Affine transformation for the shadow = simpler math, faster shader, less maintenance
  • Fixes shadows for large objects/meshes such as e.g. in Little Lady or shadows for clouds in the future
  • Smoother updates of shadows (every cascade updates at its own pace, cascade 0 updates every frame)
  • Much less peter-panning
  • Less VRAM consumption for similar shadow quality/scale
  • Visible area covered with shadows in most cases even with fast camera movement

Other fixes:

  • Removed flicker when mapblock is updated (e.g. flowing water)
  • Removed flicker when camera offset changes (every 200 nodes)
  • Added bilinear filter for SHADOW_FILTER == 1 for playable shadow quality with more performance

Notes:

  • 3 cascades
  • Cascade 0 is fixed small size in world and updates every frame, giving reasonable shadow quality close to the player
  • Cascade 2 is updated every shadow_update_frames frame (up to 32) and cascade 1 somewhere in-between
  • Entity shadows on cascades 0 and 1
  • Colored shadows work
  • Soft shadows work (may need some tuning)
  • Shadow performance with client_mesh_chunk_size = 1 may be lower because of increased number of drawcalls covering larger area. Increasing shadow_update_frames should help spread out the updates.
  • shadow_map_texture_size now controls resolution of each of 3 cascades (2 for entities) and should be divided by 2 when migrating

To do

This PR is Ready for Review.

  • Testing needed
  • Tuning/defaults feedback needed
  • Performance testing

Changes from #13833:

  • rebased
  • fixed z bias
  • fixes size of cascades relative to viewing_range

How to test

  1. Turn on the shadows
  2. Enter a game that supports shadows
  3. Check the shadows both at close and long distance, when flying etc.

@superfloh247
Copy link
Contributor

I tried to test on MacOS, but minetest crashes on startup

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libGLProgrammability.dylib 0x1f4b1957c glpLLVMGetFunctionGlobalVariableUse + 116
1 libGLProgrammability.dylib 0x1f4b19794 glpLLVMGetFunctionGlobalVariableUse + 652
2 libGLProgrammability.dylib 0x1f4b19794 glpLLVMGetFunctionGlobalVariableUse + 652
3 libGLProgrammability.dylib 0x1f4b19794 glpLLVMGetFunctionGlobalVariableUse + 652
4 libGLProgrammability.dylib 0x1f4b19794 glpLLVMGetFunctionGlobalVariableUse + 652
5 libGLProgrammability.dylib 0x1f4b17fd8 glpLLVMCGFindSamplersAndBuffers + 1184
6 libGLProgrammability.dylib 0x1f4b16e48 glpLLVMCGTopLevel + 1416
7 libGLProgrammability.dylib 0x1f4b3e814 glpLinkProgram + 9992
8 libGLProgrammability.dylib 0x1f4b586bc ShLink + 208
9 GLEngine 0x1f4cfdb4c gleLinkProgram + 1128
10 GLEngine 0x1f4c7701c glLinkProgramARB_Exec + 180
11 minetest 0x100c13b38 irr::video::COpenGLExtensionHandler::extGlLinkProgram(unsigned int) + 4 (COpenGLExtensionHandler.h:1667) [inlined]
12 minetest 0x100c13b38 irr::video::COpenGLSLMaterialRenderer::linkProgram() + 44 (COpenGLSLMaterialRenderer.cpp:394)
13 minetest 0x100c12fc8 irr::video::COpenGLSLMaterialRenderer::init(int&, char const*, char const*, char const*, irr::scene::E_PRIMITIVE_TYPE, irr::scene::E_PRIMITIVE_TYPE, unsigned int) + 316 (COpenGLSLMaterialRenderer.cpp:195)
14 minetest 0x100c13100 irr::video::COpenGLSLMaterialRenderer::COpenGLSLMaterialRenderer(irr::video::COpenGLDriver*, int&, char const*, char const*, irr::video::E_VERTEX_SHADER_TYPE, char const*, char const*, irr::video::E_PIXEL_SHADER_TYPE, char const*, char const*, irr::video::E_GEOMETRY_SHADER_TYPE, irr::scene::E_PRIMITIVE_TYPE, irr::scene::E_PRIMITIVE_TYPE, unsigned int, irr::video::IShaderConstantSetCallBack*, irr::video::E_MATERIAL_TYPE, int) + 256 (COpenGLSLMaterialRenderer.cpp:79)
15 minetest 0x100c0e034 irr::video::COpenGLDriver::addHighLevelShaderMaterial(char const*, char const*, irr::video::E_VERTEX_SHADER_TYPE, char const*, char const*, irr::video::E_PIXEL_SHADER_TYPE, char const*, char const*, irr::video::E_GEOMETRY_SHADER_TYPE, irr::scene::E_PRIMITIVE_TYPE, irr::scene::E_PRIMITIVE_TYPE, unsigned int, irr::video::IShaderConstantSetCallBack*, irr::video::E_MATERIAL_TYPE, int) + 64 (COpenGLDriver.cpp:3497) [inlined]
16 minetest 0x100c0e034 non-virtual thunk to irr::video::COpenGLDriver::addHighLevelShaderMaterial(char const*, char const*, irr::video::E_VERTEX_SHADER_TYPE, char const*, char const*, irr::video::E_PIXEL_SHADER_TYPE, char const*, char const*, irr::video::E_GEOMETRY_SHADER_TYPE, irr::scene::E_PRIMITIVE_TYPE, irr::scene::E_PRIMITIVE_TYPE, unsigned int, irr::video::IShaderConstantSetCallBack*, irr::video::E_MATERIAL_TYPE, int) + 148
17 minetest 0x1008498bc ShaderSource::generateShader(std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator> const&, MaterialType, NodeDrawType) + 6056 (shader.cpp:800)

@lhofhansl
Copy link
Contributor Author

Looks like this is coming from Irrlicht.
That happens only with this PR applied? Never without?
I do not have a Mac to verify.

@superfloh247
Copy link
Contributor

Only with this PR applied.

cd ~/VSCode
rm -rf minetest
git clone --depth=1 https://github.com/minetest/minetest.git
cd minetest
git fetch origin pull/13956/head:pr13956
git checkout pr13956
git clone --depth=1 --branch cat misc/irrlichtmt_tag.txt https://github.com/minetest/irrlicht.git lib/irrlichtmt
git clone --depth 1 https://github.com/minetest/minetest_game.git games/minetest_game
mkdir build && cd build
cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_OSX_DEPLOYMENT_TARGET=10.14 -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_INSTALL_PREFIX=../build/macos/ -DRUN_IN_PLACE=FALSE -DENABLE_GETTEXT=TRUE -DENABLE_POSTGRESQL=0 -DENABLE_LUAJIT=1 -DENABLE_SPATIAL=0
make -j$(sysctl -n hw.logicalcpu)
make install
codesign --remove-signature macos/minetest.app && codesign --force --deep -s - macos/minetest.app

irrlicht_mt_tag.txt expands to 1.9.0mt13

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

Can someone else test on a Mac and confirm?

@lhofhansl
Copy link
Contributor Author

Rebased.

@lhofhansl
Copy link
Contributor Author

BTW. I tried the artifact generated by the build CI and ran it on MacOS, and it worked fine. No crash and no strange behavior.

So @superfloh247 that looks like a problem in your environment only.

@lhofhansl lhofhansl removed Help needed WIP The PR is still being worked on by its author and not ready yet. labels Jan 11, 2024
@lhofhansl
Copy link
Contributor Author

And... Rebased again. This one was tedious.

@lhofhansl
Copy link
Contributor Author

Actually, I would love to separate all the other improvements from the Cascade logic, but I won't have enough time, and there's a lot of "black-magic" in that code - lots of math with hard-coded parameters and no comments.

@lhofhansl
Copy link
Contributor Author

Rebased... Again. This is bit-rotting fast.

@rubenwardy rubenwardy added the Roadmap The change matches an item on the current roadmap. label Jan 14, 2024
@lhofhansl
Copy link
Contributor Author

Rebased...

@Zughy Zughy added the Feature ✨ PRs that add or enhance a feature label Jan 22, 2024
@lhofhansl lhofhansl force-pushed the shadow_cascades branch 2 times, most recently from 4c1d26a to 62b9b72 Compare January 26, 2024 22:29
@lhofhansl
Copy link
Contributor Author

Anybody else interested in this.
If not I am going to close and someone else can pick it up later.

@lhofhansl
Copy link
Contributor Author

The perspective factor is still not right. With very short viewing_range and low sun (morning/evening) there are incorrect shadow stripes.

@lhofhansl lhofhansl added the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Feb 27, 2024
@lhofhansl
Copy link
Contributor Author

I'm signing off from this one.

@Zughy Zughy closed this Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Adoption needed The pull request needs someone to adopt it. Adoption welcomed! @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Performance Roadmap The change matches an item on the current roadmap. Testing needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants