Fix error in AABB calculation for particles with USERDATA

[permelin]

Selecting Generate AABB on a 3D particle node in the editor would not work and printed an error about incorrect buffer size if the particle shader used one or more of the USERDATA build-ins.

[clayjohn]

Looks good!

Did you test the compatibility renderer as well? I took a quick look and I think the compatibility renderer has the opposite problem where its reading from a buffer that only has Transform, color, and custom, but it uses the stride including userdata

[permelin]

Did you test the compatibility renderer as well?

No, it didn't even cross my mind that it would have separate code for this. I'm still very new to Godot (and C++, GLSL and gamedev in general) and I have much to learn about how the engine is structured.

I took a quick look and I think the compatibility renderer has the opposite problem where its reading from a buffer that only has Transform, color, and custom, but it uses the stride including userdata

I'm almost certainly misunderstanding you here but even while the userdata fields are not in the C++ struct, space is still reserved on the CPU side for userdata that will only exist on the GPU side. I've assumed it is done this way because the buffer seems to be memory mapped to the GPU. (I really have no idea how these pipelines works, I'm just guessing from the clues I've picked up in the code.)

But from a quick look at the particle code for GLES3 it seems like it sometimes includes userdata in the stride and sometimes not. The corresponding GLSL code expects space to be reserved in the buffer for userdata, but I don't think it actually is. And when I ran a quick test, the particle shader completely glitched out if there was any mention of userdata in it. Which seems to confirm that it is not correctly implemented for the compatibility renderer.

[clayjohn]

Did you test the compatibility renderer as well?

No, it didn't even cross my mind that it would have separate code for this. I'm still very new to Godot (and C++, GLSL and gamedev in general) and I have much to learn about how the engine is structured.

I took a quick look and I think the compatibility renderer has the opposite problem where its reading from a buffer that only has Transform, color, and custom, but it uses the stride including userdata

I'm almost certainly misunderstanding you here but even while the userdata fields are not in the C++ struct, space is still reserved on the CPU side for userdata that will only exist on the GPU side. I've assumed it is done this way because the buffer seems to be memory mapped to the GPU. (I really have no idea how these pipelines works, I'm just guessing from the clues I've picked up in the code.)

But from a quick look at the particle code for GLES3 it seems like it sometimes includes userdata in the stride and sometimes not. The corresponding GLSL code expects space to be reserved in the buffer for userdata, but I don't think it actually is. And when I ran a quick test, the particle shader completely glitched out if there was any mention of userdata in it. Which seems to confirm that it is not correctly implemented for the compatibility renderer.

In that case, perhaps it is best just to move forward with the fix for RD right now.

[permelin]

In that case, perhaps it is best just to move forward with the fix for RD right now.

Yes. GLES3 does seem to have a similar bug to the one that this PR fixes, but since the C++ and GLSL disagrees about userdata allocation for GLES3 as it stands, it's impossible to fix the AABB part on its own.

[permelin]

I figured out why userdata in the compatibility particles caused different strides on the CPU and GPU. I'll do a separate PR for that.

@clayjohn You were of course right and I'm sorry that I mansplained things to you. The particular buffers that particles_get_current_aabb() reads from should never have space allocated for userdata. I've pushed an amend that also includes a fix for the compatibility renderer.

[akien-mga]

Thanks!