Skip to content

Commit 87855e0

Browse files
akien-mgaakien-mga
committed
Merge pull request #100643 from ze2j/follow_up_of_array_mesh_surface_remove
Follow-up of `ArrayMesh::surface_remove` addition
2 parents 1327484 + 97b0936 commit 87855e0

File tree

6 files changed

+73
-52
lines changed

6 files changed

+73
-52
lines changed

‎doc/classes/ArrayMesh.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@
169169
<return type="void" />
170170
<param index="0" name="surf_idx" type="int" />
171171
<description>
172-
Removes a surface at position surf_idx, shifting greater surfaces one surf_idx slot down.
172+
Removes the surface at the given index from the Mesh, shifting surfaces with higher index down by one.
173173
</description>
174174
</method>
175175
<method name="surface_set_name">

‎doc/classes/RenderingServer.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2513,7 +2513,7 @@
25132513
<param index="0" name="mesh" type="RID" />
25142514
<param index="1" name="surface" type="int" />
25152515
<description>
2516-
Removes a mesh's surface at position surf_idx, shifting greater surfaces one surf_idx slot down.
2516+
Removes the surface at the given index from the Mesh, shifting surfaces with higher index down by one.
25172517
</description>
25182518
</method>
25192519
<method name="mesh_surface_set_material">

‎drivers/gles3/storage/mesh_storage.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
479479
}
480480

481481
if (s.versions) {
482-
memfree(s.versions); //reallocs, so free with memfree.
482+
memfree(s.versions); // reallocs, so free with memfree.
483483
}
484484

485485
if (s.wireframe) {

‎servers/rendering/renderer_rd/storage_rd/mesh_storage.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -505,11 +505,11 @@ void MeshStorage::mesh_add_surface(RID p_mesh, const RS::SurfaceData &p_surface)
505505
mesh->material_cache.clear();
506506
}
507507

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

511511
if (s.vertex_buffer.is_valid()) {
512-
RD::get_singleton()->free(s.vertex_buffer); //clears arrays as dependency automatically, including all versions
512+
RD::get_singleton()->free(s.vertex_buffer); // Clears arrays as dependency automatically, including all versions.
513513
}
514514
if (s.attribute_buffer.is_valid()) {
515515
RD::get_singleton()->free(s.attribute_buffer);
@@ -518,7 +518,7 @@ void MeshStorage::_mesh_surface_clear(Mesh *mesh, int p_surface) {
518518
RD::get_singleton()->free(s.skin_buffer);
519519
}
520520
if (s.versions) {
521-
memfree(s.versions); //reallocs, so free with memfree.
521+
memfree(s.versions); // reallocs, so free with memfree.
522522
}
523523

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

539-
memdelete(mesh->surfaces[p_surface]);
539+
memdelete(p_mesh->surfaces[p_surface]);
540540
}
541541

542542
int MeshStorage::mesh_get_blend_shape_count(RID p_mesh) const {

‎servers/rendering/renderer_rd/storage_rd/mesh_storage.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ class MeshStorage : public RendererMeshStorage {
201201

202202
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);
203203
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);
204-
void _mesh_surface_clear(Mesh *mesh, int p_surface);
204+
void _mesh_surface_clear(Mesh *p_mesh, int p_surface);
205205

206206
void _mesh_instance_clear(MeshInstance *mi);
207207
void _mesh_instance_add_surface(MeshInstance *mi, Mesh *mesh, uint32_t p_surface);

‎tests/scene/test_arraymesh.h

+64-43
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@
3939
namespace TestArrayMesh {
4040

4141
TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
42-
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
42+
Ref<ArrayMesh> mesh;
43+
mesh.instantiate();
4344
StringName name_a{ "ShapeA" };
4445
StringName name_b{ "ShapeB" };
4546

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

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

9193
SUBCASE("Adding blend shapes once all surfaces have been removed is allowed") {
92-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
93-
Array cylinder_array{};
94+
Ref<CylinderMesh> cylinder;
95+
cylinder.instantiate();
96+
Array cylinder_array;
9497
cylinder_array.resize(Mesh::ARRAY_MAX);
9598
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
9699
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
@@ -136,16 +139,17 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
136139
SUBCASE("Clearing all blend shapes once all surfaces have been removed is allowed") {
137140
mesh->add_blend_shape(name_a);
138141
mesh->add_blend_shape(name_b);
139-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
140-
Array cylinder_array{};
142+
Ref<CylinderMesh> cylinder;
143+
cylinder.instantiate();
144+
Array cylinder_array;
141145
cylinder_array.resize(Mesh::ARRAY_MAX);
142146
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
143-
Array blend_shape{};
147+
Array blend_shape;
144148
blend_shape.resize(Mesh::ARRAY_MAX);
145149
blend_shape[Mesh::ARRAY_VERTEX] = cylinder_array[Mesh::ARRAY_VERTEX];
146150
blend_shape[Mesh::ARRAY_NORMAL] = cylinder_array[Mesh::ARRAY_NORMAL];
147151
blend_shape[Mesh::ARRAY_TANGENT] = cylinder_array[Mesh::ARRAY_TANGENT];
148-
Array blend_shapes{};
152+
Array blend_shapes;
149153
blend_shapes.push_back(blend_shape);
150154
blend_shapes.push_back(blend_shape);
151155
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend_shapes);
@@ -165,8 +169,7 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
165169
SUBCASE("Can't add surface with incorrect number of blend shapes.") {
166170
mesh->add_blend_shape(name_a);
167171
mesh->add_blend_shape(name_b);
168-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
169-
Array cylinder_array{};
172+
Array cylinder_array;
170173
ERR_PRINT_OFF
171174
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
172175
ERR_PRINT_ON
@@ -176,16 +179,17 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
176179
SUBCASE("Can't clear blend shapes after surface had been added.") {
177180
mesh->add_blend_shape(name_a);
178181
mesh->add_blend_shape(name_b);
179-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
180-
Array cylinder_array{};
182+
Ref<CylinderMesh> cylinder;
183+
cylinder.instantiate();
184+
Array cylinder_array;
181185
cylinder_array.resize(Mesh::ARRAY_MAX);
182186
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
183-
Array blend_shape{};
187+
Array blend_shape;
184188
blend_shape.resize(Mesh::ARRAY_MAX);
185189
blend_shape[Mesh::ARRAY_VERTEX] = cylinder_array[Mesh::ARRAY_VERTEX];
186190
blend_shape[Mesh::ARRAY_NORMAL] = cylinder_array[Mesh::ARRAY_NORMAL];
187191
blend_shape[Mesh::ARRAY_TANGENT] = cylinder_array[Mesh::ARRAY_TANGENT];
188-
Array blend_shapes{};
192+
Array blend_shapes;
189193
blend_shapes.push_back(blend_shape);
190194
blend_shapes.push_back(blend_shape);
191195
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend_shapes);
@@ -203,15 +207,18 @@ TEST_CASE("[SceneTree][ArrayMesh] Adding and modifying blendshapes.") {
203207
}
204208

205209
TEST_CASE("[SceneTree][ArrayMesh] Surface metadata tests.") {
206-
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
207-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
208-
Array cylinder_array{};
210+
Ref<ArrayMesh> mesh;
211+
mesh.instantiate();
212+
Ref<CylinderMesh> cylinder;
213+
cylinder.instantiate();
214+
Array cylinder_array;
209215
cylinder_array.resize(Mesh::ARRAY_MAX);
210216
cylinder->create_mesh_array(cylinder_array, 3.f, 3.f, 5.f);
211217
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
212218

213-
Ref<BoxMesh> box = memnew(BoxMesh);
214-
Array box_array{};
219+
Ref<BoxMesh> box;
220+
box.instantiate();
221+
Array box_array;
215222
box_array.resize(Mesh::ARRAY_MAX);
216223
box->create_mesh_array(box_array, Vector3(2.f, 1.2f, 1.6f));
217224
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, box_array);
@@ -255,68 +262,77 @@ TEST_CASE("[SceneTree][ArrayMesh] Surface metadata tests.") {
255262
}
256263

257264
SUBCASE("Set material to two different surfaces.") {
258-
Ref<Material> mat = memnew(Material);
265+
Ref<Material> mat;
266+
mat.instantiate();
259267
mesh->surface_set_material(0, mat);
260268
CHECK(mesh->surface_get_material(0) == mat);
261269
mesh->surface_set_material(1, mat);
262270
CHECK(mesh->surface_get_material(1) == mat);
263271
}
264272

265273
SUBCASE("Set same material multiple times doesn't change material of surface.") {
266-
Ref<Material> mat = memnew(Material);
274+
Ref<Material> mat;
275+
mat.instantiate();
267276
mesh->surface_set_material(0, mat);
268277
mesh->surface_set_material(0, mat);
269278
mesh->surface_set_material(0, mat);
270279
CHECK(mesh->surface_get_material(0) == mat);
271280
}
272281

273282
SUBCASE("Set material of surface then change to different material.") {
274-
Ref<Material> mat1 = memnew(Material);
275-
Ref<Material> mat2 = memnew(Material);
283+
Ref<Material> mat1;
284+
mat1.instantiate();
285+
Ref<Material> mat2;
286+
mat2.instantiate();
276287
mesh->surface_set_material(1, mat1);
277288
CHECK(mesh->surface_get_material(1) == mat1);
278289
mesh->surface_set_material(1, mat2);
279290
CHECK(mesh->surface_get_material(1) == mat2);
280291
}
281292

282293
SUBCASE("Get the LOD of the mesh.") {
283-
Dictionary lod{};
284-
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, TypedArray<Array>{}, lod);
294+
Dictionary lod;
295+
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, TypedArray<Array>(), lod);
285296
CHECK(mesh->surface_get_lods(2) == lod);
286297
}
287298

288299
SUBCASE("Get the blend shape arrays from the mesh.") {
289-
TypedArray<Array> blend{};
300+
TypedArray<Array> blend;
290301
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array, blend);
291302
CHECK(mesh->surface_get_blend_shape_arrays(2) == blend);
292303
}
293304
}
294305

295306
TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
296-
Ref<ArrayMesh> mesh = memnew(ArrayMesh);
297-
Ref<CylinderMesh> cylinder = memnew(CylinderMesh);
298-
Array cylinder_array{};
307+
Ref<ArrayMesh> mesh;
308+
mesh.instantiate();
309+
Ref<CylinderMesh> cylinder;
310+
cylinder.instantiate();
311+
Array cylinder_array;
299312
cylinder_array.resize(Mesh::ARRAY_MAX);
300313
constexpr float cylinder_radius = 3.f;
301314
constexpr float cylinder_height = 5.f;
302315
cylinder->create_mesh_array(cylinder_array, cylinder_radius, cylinder_radius, cylinder_height);
303316
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, cylinder_array);
304317

305-
Ref<BoxMesh> box = memnew(BoxMesh);
306-
Array box_array{};
318+
Ref<BoxMesh> box;
319+
box.instantiate();
320+
Array box_array;
307321
box_array.resize(Mesh::ARRAY_MAX);
308322
const Vector3 box_size = Vector3(2.f, 1.2f, 1.6f);
309323
box->create_mesh_array(box_array, box_size);
310324
mesh->add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, box_array);
311325

312326
SUBCASE("Set the shadow mesh.") {
313-
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
327+
Ref<ArrayMesh> shadow;
328+
shadow.instantiate();
314329
mesh->set_shadow_mesh(shadow);
315330
CHECK(mesh->get_shadow_mesh() == shadow);
316331
}
317332

318333
SUBCASE("Set the shadow mesh multiple times.") {
319-
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
334+
Ref<ArrayMesh> shadow;
335+
shadow.instantiate();
320336
mesh->set_shadow_mesh(shadow);
321337
mesh->set_shadow_mesh(shadow);
322338
mesh->set_shadow_mesh(shadow);
@@ -325,8 +341,10 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
325341
}
326342

327343
SUBCASE("Set the same shadow mesh on multiple meshes.") {
328-
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
329-
Ref<ArrayMesh> mesh2 = memnew(ArrayMesh);
344+
Ref<ArrayMesh> shadow;
345+
shadow.instantiate();
346+
Ref<ArrayMesh> mesh2;
347+
mesh2.instantiate();
330348
mesh->set_shadow_mesh(shadow);
331349
mesh2->set_shadow_mesh(shadow);
332350

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

337355
SUBCASE("Set the shadow mesh and then change it.") {
338-
Ref<ArrayMesh> shadow = memnew(ArrayMesh);
356+
Ref<ArrayMesh> shadow;
357+
shadow.instantiate();
339358
mesh->set_shadow_mesh(shadow);
340359
CHECK(mesh->get_shadow_mesh() == shadow);
341-
Ref<ArrayMesh> shadow2 = memnew(ArrayMesh);
360+
Ref<ArrayMesh> shadow2;
361+
shadow2.instantiate();
342362
mesh->set_shadow_mesh(shadow2);
343363
CHECK(mesh->get_shadow_mesh() == shadow2);
344364
}
345365

346366
SUBCASE("Set custom AABB.") {
347-
AABB bound{};
367+
AABB bound;
348368
mesh->set_custom_aabb(bound);
349369
CHECK(mesh->get_custom_aabb() == bound);
350370
}
351371

352372
SUBCASE("Set custom AABB multiple times.") {
353-
AABB bound{};
373+
AABB bound;
354374
mesh->set_custom_aabb(bound);
355375
mesh->set_custom_aabb(bound);
356376
mesh->set_custom_aabb(bound);
@@ -359,8 +379,8 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
359379
}
360380

361381
SUBCASE("Set custom AABB then change to another AABB.") {
362-
AABB bound{};
363-
AABB bound2{};
382+
AABB bound;
383+
AABB bound2;
364384
mesh->set_custom_aabb(bound);
365385
CHECK(mesh->get_custom_aabb() == bound);
366386
mesh->set_custom_aabb(bound2);
@@ -380,7 +400,8 @@ TEST_CASE("[SceneTree][ArrayMesh] Get/Set mesh metadata and actions") {
380400
SUBCASE("Create surface from raw SurfaceData data.") {
381401
RID mesh_rid = mesh->get_rid();
382402
RS::SurfaceData surface_data = RS::get_singleton()->mesh_get_surface(mesh_rid, 0);
383-
Ref<ArrayMesh> mesh2 = memnew(ArrayMesh);
403+
Ref<ArrayMesh> mesh2;
404+
mesh2.instantiate();
384405
mesh2->add_surface(surface_data.format, Mesh::PRIMITIVE_TRIANGLES, surface_data.vertex_data, surface_data.attribute_data,
385406
surface_data.skin_data, surface_data.vertex_count, surface_data.index_data, surface_data.index_count, surface_data.aabb);
386407
CHECK(mesh2->get_surface_count() == 1);

0 commit comments

Comments
 (0)