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

[manual merge] VBO-related optimizations and improvements #14395

Closed
wants to merge 25 commits into from

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Feb 20, 2024

⚠️ requires minetest/irrlicht#292

goal:

  • use more VBOs (not constantly uploading data to the GPU is supposed to be good for performance)
  • move cloud rendering away from immediate mode and use a shader for further optimization
  • some code cleanup

applicability: dunno, but I had fun and learned stuff while working on the cloud code.

note for second commit:
We would need to wrap every call to setHardwareMappingHint (try ctrl+f) into an if that checks the setting and also cache that etc. All while there is essentially no reason to disable VBOs.
(Irrlicht won't use them if they are not supported for some reason.)

How to merge

Keep all commits that start with upper-case letters
Squash others into one and name it VBO-related optimizations and improvements (#14395)

To do

This PR is Ready for Review.

How to test

  1. check and make sure dynamic shadows still work
  2. open main menu, observe clouds
  3. use test script and enjoy rapidly changing cloud cover (join a mgv6 world or MTG will get in the way)
local passed = 0
core.register_globalstep(function(dtime)
	passed = passed + dtime
	if passed < 0.4 then
		return
	end
	passed = passed - 0.4

	local t = core.get_us_time() / (1000*1000)
	for _, player in ipairs(minetest.get_connected_players()) do
		player:set_clouds({
			density = (1 + math.sin(t)) / 2
		})
	end
end)
  1. watch cloud colors at dusk (/time 19:00)
  2. check if anything is actually faster than before?

@lhofhansl
Copy link
Contributor

I tried. Everything seems to still work. No performance gain, though.

@sfan5
Copy link
Member Author

sfan5 commented Feb 22, 2024

That's largely to be expected, unless you're stress-testing entity or item rendering (maybe).
MapBlocks already use VBOs and I don't expect the cloud changes to make any difference on remotely modern hardware.

@sfan5 sfan5 force-pushed the vbostuff branch 2 times, most recently from 6110307 to 48d4a93 Compare February 24, 2024 00:04
@lhofhansl
Copy link
Contributor

Looks good.
BTW. I am giving up on #13956, now. This would cause another annoying rebase, and nobody except me seems to care ;)

@Desour
Copy link
Member

Desour commented Mar 1, 2024

@sfan5 The assert at client/clientlauncher.cpp:270 is failing when I close the game: assert(g_menuclouds->getReferenceCount() == 1); (Still grabbed by scene node parent I guess.)

@sfan5
Copy link
Member Author

sfan5 commented Mar 1, 2024

Yep, noticed it. PR soon.

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

3 participants