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

Batched rendering of particles #13834

Closed
wants to merge 5 commits into from
Closed

Conversation

x2048
Copy link
Contributor

@x2048 x2048 commented Sep 23, 2023

Improves performance by rendering particles with the same texture as a single mesh buffer. Possibly resolves #1414

Puts limits on how many particles can be drawn:

  • 16K particles per texture.
  • 64K particles in total.
  • Within 200 nodes from the local player.

Example results:
AMD Renoir
34K particles on the screen
13 FPS on master
45 FPS on the branch.

To do

This PR is Ready for Review.

How to test

Minimal test:

  1. Launch dev test
  2. Get particle spawner
  3. Spawn particles around and watch them animate and disappear. No glitches.

Example code for performance test:

local nodes = {}
local players = {}

local torch_colors = {
	"#0F0000",
	"#0F0800",
	"#080F00",
	"#080F00",
	"#000808"
}


minetest.register_abm({
	nodenames = { "default:torch" },
	interval = 1,
	chance = 1,
	catch_up = false,
	action = function(pos, node)
		local meta = minetest.get_meta(pos)
		local color = meta:get("colored_torch:color")
		if color == nil then
			color = torch_colors[math.floor(math.random() * #torch_colors) + 1]
			meta:set_string("colored_torch:color", color)
		end
		minetest.add_particlespawner({
			pos = { min = vector.add(pos, vector.new(-0.2, 0.35, -0.2)), max = vector.add(pos, vector.new(0.2, 0.45, 0.2)) },
			vel = { min = vector.new(-2, 7, -2), max = vector.new( 2, 5, 2) },
			acc = { min = vector.new(0, -6, 0), max = vector.new(0, -5, 0) },
			time = 1,
			amount = 160,
			exptime = 6,
			collisiondetection = false,
			collision_removal = false,
			glow = 14,
			texpool = {
				{ name = "flame_spark.png^[multiply:"..color, blend = "screen", scale = 3 },
			}
		})
	end
})

@x2048 x2048 self-assigned this Sep 23, 2023
@x2048 x2048 added @ Client / Audiovisuals Testing needed Roadmap The change matches an item on the current roadmap. labels Sep 23, 2023
@rubenwardy
Copy link
Member

Thanks so much!

Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Minor concern: Won't this mess with particle-particle blending? (Since this can at best sort particle vertices per-texture).

(Only a minor concern because (1) particle-nonparticle blending doesn't work anyways and (2) particle-particle blending has only been fixed recently-ish (5.6 IIRC) (3) acceptable performance has priority (4) particle-particle blending will still work among particles that use the same texture or that use order-independent blend modes)

* Initialize default_particle_texture explicitly
* Add override keyword where necessary
@lhofhansl
Copy link
Contributor

I've had this enabled for a few days. Observed nothing undue. Works.

@rubenwardy rubenwardy added the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Oct 2, 2023
@Zughy
Copy link
Member

Zughy commented Oct 2, 2023

Note for devs: if there are no major reviews, this PR can still get approved and merged

@appgurueu
Copy link
Contributor

appgurueu commented Oct 2, 2023

I've had this enabled for a few days. Observed nothing undue. Works.

Have you tested particle-particle blending among semitransparent particles with different textures?

(This can be safely enabled for particles that don't use order-dependent blending. There could also be an option for particles that do need blending - but only among particles of the same kind - to turn this on, sacrificing visual correctness for performance. But as said this can probably be considered minor anyways...)

@lhofhansl
Copy link
Contributor

Have you tested particle-particle blending among semitransparent particles with different textures?

No I have not.

@rubenwardy rubenwardy mentioned this pull request Oct 7, 2023
@Desour
Copy link
Member

Desour commented Oct 11, 2023

Here's a rebased version, plus some small code quality improvements, like irr_ptr and code style: https://github.com/Desour/minetest/tree/x2048_particle-performance_rebased

@Zughy Zughy removed the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Oct 11, 2023
@Zughy
Copy link
Member

Zughy commented Oct 11, 2023

@Desour if you confirm you want to take care of this PR, feel free to close this one

@Desour
Copy link
Member

Desour commented Oct 12, 2023

I'd prefer if someone else adopts it.

@Desour Desour added the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Oct 12, 2023
@jordan4ibanez
Copy link
Contributor

Hello there core dev teams, this is my child now. I have adopted it. Please give me the rights or whatever adoption requires

@rubenwardy
Copy link
Member

@jordan4ibanez you just need to open a new pr for it

@rubenwardy rubenwardy closed this Dec 24, 2023
@rubenwardy rubenwardy removed the Adoption needed The pull request needs someone to adopt it. Adoption welcomed! label Dec 24, 2023
@Desour
Copy link
Member

Desour commented Dec 24, 2023

@jordan4ibanez Have you considered taking the rebased branch I've linked up there? It was meant to be used by the adopter.

(As there seems to be quite some confusion on why I didn't adopt this, let me clarify: Much of the work needed for this PR, i.e. testing that everything still works, especially with alpha blending issues and whatnot, is still left to be done. And this PR is not high on my already too long priority list.)

@jordan4ibanez
Copy link
Contributor

Hello desour, please comment any issues on the new pr, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client / Audiovisuals High priority Performance Roadmap The change matches an item on the current roadmap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Particles are very slow
7 participants