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

ArrayMesh with flag USES_EMPTY_VERTEX_ARRAY still needs vertex array for rendering work #83446

Closed
erickweil opened this issue Oct 16, 2023 · 7 comments

Comments

@erickweil
Copy link

erickweil commented Oct 16, 2023

Godot version

v4.2.beta1.mono.official [b137180]

System information

Godot v4.2.beta1.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated GeForce GTX 1650 () - AMD Ryzen 7 4800H with Radeon Graphics (16 Threads)

Issue description

I was experimenting with reading geometry from a Texture when I come across #62046 and found some weird behaviours when creating a ArrayMesh with the MESH_ARRAY_FLAG_USES_EMPTY_VERTEX_ARRAY flag:

  1. If no vertex array if provided, no rendering happen (buffer allocates memory but 0 primitives drawn)
  2. If a 0-sized vertex array if provided, error is produced
  3. If a vertex array with small size (3 for example) is provided, the rendering work as expected and all primitives are drawn from the indice array, even with thousands of indices with no problem, but fails with weird glitches with more than 65535 indices
  4. If in the case of more than 65535 indices a vertex array with size 65537 (1 more) is provided, everything works as intended, even with millions of indices.

From this analisis, I conclude the most probable cause is that somewhere in the mesh creation phase there is a check that looks at vertice count to decide if should use or not 16-bit indices. So when no
The main issue here is the requirement in providing a dummy 65k vertice array just to signal that a 16-bit index should not be used.

Conclusion

Vertex-less meshes should work out of the box without needing any vertex array to be created, basically the way it should work is that when the mesh has the empty vertex array flag the logic to decide if a 16 or 32 bit index should be used needs to be different, maybe the user could pass a flag overriding the underlying logic and specifying that 32 bit indices are needed

Steps to reproduce

Look at the Minimal reproduction project.

The general steps are:

  1. Have a custom gdshader that doesn't use VERTEX, instead using only VERTEX_ID and some math outputs a vertex position
  2. Have a ArrayMesh with indices and with the flag MESH_ARRAY_FLAG_USES_EMPTY_VERTEX_ARRAY set
  3. Observe the following behaviour:

If no vertex array is give to ArrayMesh, nothing is drawn
If a vertex array with size 1 is given, primitives are drawn but VERTEX_ID will be 16 bits, causing issues with big meshes, solving by giving a vertex array with size 65536+1

Minimal reproduction project

IssueVertexLessMesh.zip

Edit: Added GDScript version
IssueVertexLessMeshGDScript.zip

@erickweil erickweil changed the title ArrayMesh with flah USES_EMPTY_VERTEX_ARRAY still needs vertex array for rendering work ArrayMesh with flag USES_EMPTY_VERTEX_ARRAY still needs vertex array for rendering work Oct 16, 2023
@AThousandShips
Copy link
Member

Please provide a GDScript version of your code, unless this is unique to C# this is preferred and otherwise few contributors will be able to help debugging this

@clayjohn
Copy link
Member

Indeed, the size of the vertex array is used to determine whether 16 bit or 32 bit indices are used.

I guess we have a few options:

  1. Iterate over the index array and use the max value to determine which is needed (this is simple and foolproof, but wastes some cycles)
  2. Use another attribute array (users may still provide UVs etc)
  3. Let the user hint how many bits to use with additional flags

@erickweil
Copy link
Author

Please provide a GDScript version of your code, unless this is unique to C# this is preferred and otherwise few contributors will be able to help debugging this

Ok I updated the issue with a GDScript version. It's not unique to C#.

@TokisanGames
Copy link
Contributor

As of 4.2.1, the GDScript MRP with the no vertex array option works fine. The custom_aabb or extra cull margin do need to be non-zero so it doesn't get culled by the renderer.

Here's an updated, working MRP.

test_vertex_less.zip

image

@Calinou
Copy link
Member

Calinou commented Apr 9, 2024

The custom_aabb or extra cull margin do need to be non-zero so it doesn't get culled by the renderer.

In this case, is this just a documentation issue?

@TokisanGames
Copy link
Contributor

It appears to me that as of 4.2.1, this ticket is satisfied and can be closed, unless you want to add documentation.

@erickweil
Copy link
Author

Looks good to me. It could be closed.

@akien-mga akien-mga added this to the 4.2 milestone Apr 13, 2024
Adrian-Samoticha added a commit to Adrian-Samoticha/godot that referenced this issue Jun 24, 2024
While the `RenderingServer::mesh_create_surface_data_from_arrays`
method does support vertexless meshes (see godotengine#62046 and godotengine#83446),
it enforces that the size of custom arrays is dependent on the size of
the vertex array. This effectively means  that custom arrays cannot be
used in vertexless meshes.

This commit changes the way the array length is computed so that if no
vertex array is provided, its length will be inferred from the custom
arrays, if provided. It therefore adds support for custom arrays in
vertexless meshes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants