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 #14489

Merged
merged 25 commits into from Apr 3, 2024
Merged

Batched rendering of particles #14489

merged 25 commits into from Apr 3, 2024

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Mar 24, 2024

rebased and partially reworked #13834

compared to the original PR this has no spawn distance or global particle limit. this is better done in another PR if desired.

This PR breaks transparency sorting of semi-transparent particles (if it even worked before).

To do

This PR is Ready for Review.

note: squash when merging. Make sure the co-authored-by line shows up for @x2048 and @Desour

How to test

  1. try the code from here Batched rendering of particles #13834
  2. check that dig particles work
  3. try the code from https://gist.github.com/sfan5/f3b8c427c5873474c947df20ff689e22

@sfan5 sfan5 added @ Client / Audiovisuals Roadmap The change matches an item on the current roadmap. Performance labels Mar 24, 2024
@sfan5 sfan5 added this to the 5.9.0 milestone Mar 24, 2024
@ancientmarinerdev
Copy link

Glad to see this stuff is being picked up. Particles have been found to be quite heavy on some servers. I'm not sure if it was strictly our usage of them, or an engine thing, so any work in this area is much appreciated.

One thing I'm curious about is the following:

"this has no spawn distance or global particle limit"

I may be misreading this, but does this mean that all clients connected to a server will pick up the particles regardless of distance? For example, if particles are use for a block like a smoker, does that mean all clients connected to the server will know exactly where these are even if they are in locations 16k blocks away?

If someone has cheat client, would this give people the means to find out where people's base are which could be used for griefing?

I'm not sure if that is exactly a new thing and may have been feasible before. As someone who understands the logic around this, could you please confirm if my understanding is correct or incorrect. If it is correct, do you have any ideas of sensible mitigations for this? Probably could be outside of the scope of the PR, but it's worth consideration now.

Thanks.

@SmallJoker SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Mar 27, 2024
@sfan5 sfan5 removed the Rebase needed The PR needs to be rebased by its author. label Mar 27, 2024
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Code review only so far.

src/client/particles.h Outdated Show resolved Hide resolved
src/client/particles.h Outdated Show resolved Hide resolved
if (!found) {
// search right buffer
for (auto &buffer : m_particle_buffers) {
if (buffer->getMaterial(0) == material) {
Copy link
Member

Choose a reason for hiding this comment

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

Here and above: add a getter for the remaining capacity of the buffer (16000 - (m_mesh_buffer->getVertexCount() / 4 - m_free_list.size())) to skip the buffer and allocate a new one.

Copy link
Member Author

@sfan5 sfan5 Mar 27, 2024

Choose a reason for hiding this comment

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

How important do you think the ability to render more than 16k of the same particle is?
I mean it's very easy but I'd rather skip it.

Copy link
Member

Choose a reason for hiding this comment

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

Not very important. I just thought it would be rather easy to implement to be future-proof in case modders wanted to spawn a dense cloud-like shape made from particles.

src/client/particles.cpp Outdated Show resolved Hide resolved
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.

As I said, this can't properly deal with semitransparent particles, because it can only do the depth sorting per particle "type". To an extent this could be alleviated by using multiple texture slots.

Test code:
minetest.register_chatcommand("spawn_particles", {
	func = function(name)
		local player = assert(minetest.get_player_by_name(name))
		local pos = player:get_pos()
		local colors = {"red", "green", "blue"}
		for dx = -5, 5 do for dz = -5, 5 do for dy = -5, 5 do
			minetest.add_particle({
				pos = pos:offset(dx, dy, dz),
				size = 10,
				expirationtime = 10,
				texture = ("blank.png^[noalpha^[colorize:%s^[opacity:127")
						:format(colors[math.random(#colors)]),
			})
		end end end
	end
})

Expected result (5.8.0):

Screenshot from 2024-03-27 20-58-02

(As you can see, this is a dense cube of semitransparent RGB particles)

Actual result (5.9-dev):

Screenshot from 2024-03-27 20-57-34

(This is not the same bunch of particles, but the second render is obviously incorrect:
Particle-particle semitransparency is wrong (regression) due to depth sorting being broken, since different batches can't be depth-sorted properly.)

For particles that do not make use of semitransparency (clip blend mode), this optimization is entirely fine, however.

@sfan5
Copy link
Member Author

sfan5 commented Mar 27, 2024

As I said, this can't properly deal with semitransparent particles,

I wanted to mention that in the description but forgot: Yes, this totally breaks semitransparent particles.
I don't plan to fix that. Consider it an incremental (but not regression-free) step to a proper particle implementation.

For particles that do not make use of semitransparency (#14353), this optimization is entirely fine, however.

If we were to only apply this optimization to clip blended particles then the case I warned about would occur: modders don't give you this information, so you go and check the texture contents for the sake of being able to apply the optimization.

I may be misreading this, but does this mean that all clients connected to a server will pick up the particles regardless of distance?

No, and this PR does not change anything about that. The server has a distance limit for sending.

@SmallJoker
Copy link
Member

This PR breaks transparency sorting of semi-transparent particles

Do you mean at the alpha blending does not work, or is that a separate issue? For example:
hearts
Can be reproduced with https://github.com/SmallJoker/minetest_lab/blob/master/init_71_particles.lua
and one of the following texture definitions. Both show the same issue.

			--texture = { name = "heart.png", blend = "alpha"},
			texpool = {
				{ name = "heart.png", blend = "alpha", scale = 2 },
			},

Unrelated: I also noticed that there seems to be a memory hog in master. About 0.6 MB every 10 seconds on my end. The PR does not seem to have a negative impact on that.

@sfan5
Copy link
Member Author

sfan5 commented Mar 29, 2024

Do you mean at the alpha blending does not work, or is that a separate issue?

I thought this was a problem on master too and it is, but only with the selection box:
grafik

With this PR this happens for every particle (with each other).
Probably because they're no longer drawn sorted by Z.

Edit: Fixed. Just the alpha bug of course, I didn't add transparency sorting.

@appgurueu
Copy link
Contributor

appgurueu commented Mar 30, 2024

Worth noting that IIRC it should be possible to fix the alpha = 0 special case by discarding the relevant fragments.

Irrelevant to this PR A special particle shader could also further improve performance because we really only need to send one position per particle to the GPU (the shader could then turn this into a quad). In principle we could also consider doing the depth-sorting on the GPU, but I don't know if that's possible to do well with OGL 3.

Something that could be considered to fix the depth sorting:

  1. Depth-sort the particles.
  2. Greedily collect batches of up to (how many texture slots we have, 16 I think?) many particles.
  3. Render the batches collected this way.

Besides an efficient order-independent transparency implementation (which would probably require OGL 4 and is hence far away), I think something like this would be about the only way to restore particle-particle depth sorting while keeping the batching.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Looks good. Works.
Thank you.

@sfan5 sfan5 merged commit f8bff34 into minetest:master Apr 3, 2024
13 checks passed
@sfan5 sfan5 deleted the particlebatch branch April 3, 2024 11:56
@ancientmarinerdev
Copy link

ancientmarinerdev commented Apr 14, 2024

This PR breaks transparency sorting of semi-transparent particles

Do you mean at the alpha blending does not work, or is that a separate issue? For example: hearts Can be reproduced with https://github.com/SmallJoker/minetest_lab/blob/master/init_71_particles.lua and one of the following texture definitions. Both show the same issue.

			--texture = { name = "heart.png", blend = "alpha"},
			texpool = {
				{ name = "heart.png", blend = "alpha", scale = 2 },
			},

Unrelated: I also noticed that there seems to be a memory hog in master. About 0.6 MB every 10 seconds on my end. The PR does not seem to have a negative impact on that.

Apologies for the delayed response. When we added smoke particles into VoxeLibre (fka. MineClone2), we had an issue with transparency making particles behind partially cut out by a transparent area. So it was likely an existing issue from even before last release. It was a reason I cut down the amount of particles I was going to use in campfires

@appgurueu
Copy link
Contributor

appgurueu commented Apr 14, 2024

we had an issue with transparency making particles behind partially cut out by a transparent area

Yes, semitransparency was already partially broken before: Particle - node and particle - entity semitransparency did not work before, and particle - particle depth sorting wasn't perfect either.

But there was some particle - particle depth sorting - so a cloud of semitransparent gas particles would look decently in an otherwise opaque world - which is now basically "fully" broken. This is what I was pointing out.

How high the priority of this is, considering the existing issues, is a different question.

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

Successfully merging this pull request may close these issues.

None yet

6 participants