Skip to content

Commit

Permalink
Follow-up of ArrayMesh::surface_remove addition
Browse files Browse the repository at this point in the history
  • Loading branch information
ze2j authored and tGautot committed Feb 5, 2025
1 parent 0de46df commit d95dd8a
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 52 deletions.
2 changes: 1 addition & 1 deletion doc/classes/ArrayMesh.xml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
<return type="void" />
<param index="0" name="surf_idx" type="int" />
<description>
Removes a surface at position surf_idx, shifting greater surfaces one surf_idx slot down.
Removes the surface at the given index from the Mesh, shifting surfaces with higher index down by one.
</description>
</method>
<method name="surface_set_name">
Expand Down
2 changes: 1 addition & 1 deletion doc/classes/RenderingServer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2513,7 +2513,7 @@
<param index="0" name="mesh" type="RID" />
<param index="1" name="surface" type="int" />
<description>
Removes a mesh's surface at position surf_idx, shifting greater surfaces one surf_idx slot down.
Removes the surface at the given index from the Mesh, shifting surfaces with higher index down by one.
</description>
</method>
<method name="mesh_surface_set_material">
Expand Down
2 changes: 1 addition & 1 deletion drivers/gles3/storage/mesh_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
}

if (s.versions) {
memfree(s.versions); //reallocs, so free with memfree.
memfree(s.versions); // reallocs, so free with memfree.
}

if (s.wireframe) {
Expand Down
10 changes: 5 additions & 5 deletions servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,11 +505,11 @@ void MeshStorage::mesh_add_surface(RID p_mesh, const RS::SurfaceData &p_surface)
mesh->material_cache.clear();
}

void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
Mesh::Surface &s = *mesh->surfaces[p_surface];
void MeshStorage::_mesh_surface_clear(Mesh *p_mesh, int p_surface) {
Mesh::Surface &s = *p_mesh->surfaces[p_surface];

if (s.vertex_buffer.is_valid()) {
RD::get_singleton()->free(s.vertex_buffer); //clears arrays as dependency automatically, including all versions
RD::get_singleton()->free(s.vertex_buffer); // Clears arrays as dependency automatically, including all versions.
}
if (s.attribute_buffer.is_valid()) {
RD::get_singleton()->free(s.attribute_buffer);
Expand All @@ -518,7 +518,7 @@ void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
RD::get_singleton()->free(s.skin_buffer);
}
if (s.versions) {
memfree(s.versions); //reallocs, so free with memfree.
memfree(s.versions); // reallocs, so free with memfree.
}

if (s.index_buffer.is_valid()) {
Expand All @@ -536,7 +536,7 @@ void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
RD::get_singleton()->free(s.blend_shape_buffer);
}

memdelete(mesh->surfaces[p_surface]);
memdelete(p_mesh->surfaces[p_surface]);
}

int MeshStorage::mesh_get_blend_shape_count(RID p_mesh) const {
Expand Down
2 changes: 1 addition & 1 deletion servers/rendering/renderer_rd/storage_rd/mesh_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ class MeshStorage : public RendererMeshStorage {

RD::VertexFormatID _mesh_surface_generate_vertex_format(uint64_t p_surface_format, uint64_t p_input_mask, bool p_instanced_surface, bool p_input_motion_vectors, uint32_t &r_position_stride);
void _mesh_surface_generate_version_for_input_mask(Mesh::Surface::Version &v, Mesh::Surface *s, uint64_t p_input_mask, bool p_input_motion_vectors, MeshInstance::Surface *mis = nullptr, uint32_t p_current_buffer = 0, uint32_t p_previous_buffer = 0);
void _mesh_surface_clear(Mesh *mesh, int p_surface);
void _mesh_surface_clear(Mesh *p_mesh, int p_surface);

void _mesh_instance_clear(MeshInstance *mi);
void _mesh_instance_add_surface(MeshInstance *mi, Mesh *mesh, uint32_t p_surface);
Expand Down
107 changes: 64 additions & 43 deletions tests/scene/test_arraymesh.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
namespace TestArrayMesh {

TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
Ref<ArrayMesh> mesh;
mesh.instantiate();
StringName name_a{ "ShapeA" };
StringName name_b{ "ShapeB" };

Expand Down Expand Up @@ -76,8 +77,9 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
}

SUBCASE("Adding blend shape after surface is added causes error") {
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
Expand All @@ -89,8 +91,9 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
}

SUBCASE("Adding blend shapes once all surfaces have been removed is allowed") {
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
Expand Down Expand Up @@ -136,16 +139,17 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
SUBCASE("Clearing all blend shapes once all surfaces have been removed is allowed") {
mesh->add_blend_shape(name_a);
mesh->add_blend_shape(name_b);
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
Array blend_shape{};
Array blend_shape;
blend_shape.resize(Mesh::ARRAY_MAX);
blend_shape[Mesh::ARRAY_VERTEX] = cylinder_array[Mesh::ARRAY_VERTEX];
blend_shape[Mesh::ARRAY_NORMAL] = cylinder_array[Mesh::ARRAY_NORMAL];
blend_shape[Mesh::ARRAY_TANGENT] = cylinder_array[Mesh::ARRAY_TANGENT];
Array blend_shapes{};
Array blend_shapes;
blend_shapes.push_back(blend_shape);
blend_shapes.push_back(blend_shape);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend_shapes);
Expand All @@ -165,8 +169,7 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
SUBCASE("Can't add surface with incorrect number of blend shapes.") {
mesh->add_blend_shape(name_a);
mesh->add_blend_shape(name_b);
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Array cylinder_array;
ERR_PRINT_OFF
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
ERR_PRINT_ON
Expand All @@ -176,16 +179,17 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
SUBCASE("Can't clear blend shapes after surface had been added.") {
mesh->add_blend_shape(name_a);
mesh->add_blend_shape(name_b);
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
Array blend_shape{};
Array blend_shape;
blend_shape.resize(Mesh::ARRAY_MAX);
blend_shape[Mesh::ARRAY_VERTEX] = cylinder_array[Mesh::ARRAY_VERTEX];
blend_shape[Mesh::ARRAY_NORMAL] = cylinder_array[Mesh::ARRAY_NORMAL];
blend_shape[Mesh::ARRAY_TANGENT] = cylinder_array[Mesh::ARRAY_TANGENT];
Array blend_shapes{};
Array blend_shapes;
blend_shapes.push_back(blend_shape);
blend_shapes.push_back(blend_shape);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend_shapes);
Expand All @@ -203,15 +207,18 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
}

TEST_CASE("[SceneTree][ArrayMesh] Surface metadata tests.") {
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<ArrayMesh> mesh;
mesh.instantiate();
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);

Ref<BoxMesh> box = memnew(BoxMesh);
Array box_array{};
Ref<BoxMesh> box;
box.instantiate();
Array box_array;
box_array.resize(Mesh::ARRAY_MAX);
box->create_mesh_array(box_array, Vector3(2.f, 1.2f, 1.6f));
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, box_array);
Expand Down Expand Up @@ -255,68 +262,77 @@ TEST_CASE("[SceneTree][ArrayMesh] Surface metadata tests.") {
}

SUBCASE("Set material to two different surfaces.") {
Ref<Material> mat = memnew(Material);
Ref<Material> mat;
mat.instantiate();
mesh->surface_set_material(0, mat);
CHECK(mesh->surface_get_material(0) == mat);
mesh->surface_set_material(1, mat);
CHECK(mesh->surface_get_material(1) == mat);
}

SUBCASE("Set same material multiple times doesn't change material of surface.") {
Ref<Material> mat = memnew(Material);
Ref<Material> mat;
mat.instantiate();
mesh->surface_set_material(0, mat);
mesh->surface_set_material(0, mat);
mesh->surface_set_material(0, mat);
CHECK(mesh->surface_get_material(0) == mat);
}

SUBCASE("Set material of surface then change to different material.") {
Ref<Material> mat1 = memnew(Material);
Ref<Material> mat2 = memnew(Material);
Ref<Material> mat1;
mat1.instantiate();
Ref<Material> mat2;
mat2.instantiate();
mesh->surface_set_material(1, mat1);
CHECK(mesh->surface_get_material(1) == mat1);
mesh->surface_set_material(1, mat2);
CHECK(mesh->surface_get_material(1) == mat2);
}

SUBCASE("Get the LOD of the mesh.") {
Dictionary lod{};
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, TypedArray<Array>{}, lod);
Dictionary lod;
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, TypedArray<Array>(), lod);
CHECK(mesh->surface_get_lods(2) == lod);
}

SUBCASE("Get the blend shape arrays from the mesh.") {
TypedArray<Array> blend{};
TypedArray<Array> blend;
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend);
CHECK(mesh->surface_get_blend_shape_arrays(2) == blend);
}
}

TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
Array cylinder_array{};
Ref<ArrayMesh> mesh;
mesh.instantiate();
Ref<CylinderMesh> cylinder;
cylinder.instantiate();
Array cylinder_array;
cylinder_array.resize(Mesh::ARRAY_MAX);
constexpr float cylinder_radius = 3.f;
constexpr float cylinder_height = 5.f;
cylinder->create_mesh_array(cylinder_array, cylinder_radius, cylinder_radius, cylinder_height);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);

Ref<BoxMesh> box = memnew(BoxMesh);
Array box_array{};
Ref<BoxMesh> box;
box.instantiate();
Array box_array;
box_array.resize(Mesh::ARRAY_MAX);
const Vector3 box_size = Vector3(2.f, 1.2f, 1.6f);
box->create_mesh_array(box_array, box_size);
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, box_array);

SUBCASE("Set the shadow mesh.") {
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
Ref<ArrayMesh> shadow;
shadow.instantiate();
mesh->set_shadow_mesh(shadow);
CHECK(mesh->get_shadow_mesh() == shadow);
}

SUBCASE("Set the shadow mesh multiple times.") {
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
Ref<ArrayMesh> shadow;
shadow.instantiate();
mesh->set_shadow_mesh(shadow);
mesh->set_shadow_mesh(shadow);
mesh->set_shadow_mesh(shadow);
Expand All @@ -325,8 +341,10 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
}

SUBCASE("Set the same shadow mesh on multiple meshes.") {
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
Ref<ArrayMesh> mesh2 = memnew(ArrayMesh);
Ref<ArrayMesh> shadow;
shadow.instantiate();
Ref<ArrayMesh> mesh2;
mesh2.instantiate();
mesh->set_shadow_mesh(shadow);
mesh2->set_shadow_mesh(shadow);

Expand All @@ -335,22 +353,24 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
}

SUBCASE("Set the shadow mesh and then change it.") {
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
Ref<ArrayMesh> shadow;
shadow.instantiate();
mesh->set_shadow_mesh(shadow);
CHECK(mesh->get_shadow_mesh() == shadow);
Ref<ArrayMesh> shadow2 = memnew(ArrayMesh);
Ref<ArrayMesh> shadow2;
shadow2.instantiate();
mesh->set_shadow_mesh(shadow2);
CHECK(mesh->get_shadow_mesh() == shadow2);
}

SUBCASE("Set custom AABB.") {
AABB bound{};
AABB bound;
mesh->set_custom_aabb(bound);
CHECK(mesh->get_custom_aabb() == bound);
}

SUBCASE("Set custom AABB multiple times.") {
AABB bound{};
AABB bound;
mesh->set_custom_aabb(bound);
mesh->set_custom_aabb(bound);
mesh->set_custom_aabb(bound);
Expand All @@ -359,8 +379,8 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
}

SUBCASE("Set custom AABB then change to another AABB.") {
AABB bound{};
AABB bound2{};
AABB bound;
AABB bound2;
mesh->set_custom_aabb(bound);
CHECK(mesh->get_custom_aabb() == bound);
mesh->set_custom_aabb(bound2);
Expand All @@ -380,7 +400,8 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
SUBCASE("Create surface from raw SurfaceData data.") {
RID mesh_rid = mesh->get_rid();
RS::SurfaceData surface_data = RS::get_singleton()->mesh_get_surface(mesh_rid, 0);
Ref<ArrayMesh> mesh2 = memnew(ArrayMesh);
Ref<ArrayMesh> mesh2;
mesh2.instantiate();
mesh2->add_surface(surface_data.format, Mesh::PRIMITIVE_TRIANGLES, surface_data.vertex_data, surface_data.attribute_data,
surface_data.skin_data, surface_data.vertex_count, surface_data.index_data, surface_data.index_count, surface_data.aabb);
CHECK(mesh2->get_surface_count() == 1);
Expand Down

0 comments on commit d95dd8a

Please sign in to comment.