blender
/
blender
136
411
Fork 595
Code Issues 6.3k Pull Requests 774 Projects 19 Wiki Activity

GPv3: Fix Remove Material Slot and Unused Slots #114850

Merged
Falk David merged 8 commits from antoniov/blender:GPv3_remove_unused_mat into main 2023-11-17 10:38:06 +01:00
Conversation 0 Commits 8 Files Changed 3 +60 -3
3 changed files with 60 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/blenkernel/BKE_grease_pencil.hh
View File

@ -837,6 +837,8 @@ Material *BKE_grease_pencil_object_material_ensure_from_active_input_brush(Main
Material *BKE_grease_pencil_object_material_ensure_from_active_input_material(Object *ob);
Material *BKE_grease_pencil_object_material_ensure_active(Object *ob);
void BKE_grease_pencil_material_remap(GreasePencil *grease_pencil, const uint *remap, int totcol);
void BKE_grease_pencil_material_index_remove(GreasePencil *grease_pencil, int index);
bool BKE_grease_pencil_references_cyclic_check(const GreasePencil *id_reference,
const GreasePencil *grease_pencil);
bool BKE_grease_pencil_material_index_used(GreasePencil *grease_pencil, int index);

49
source/blender/blenkernel/intern/grease_pencil.cc
View File

@ -1390,6 +1390,55 @@ void BKE_grease_pencil_material_remap(GreasePencil *grease_pencil, const uint *r
}
}
void BKE_grease_pencil_material_index_remove(GreasePencil *grease_pencil, int index)
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-16 13:59:35 +01:00
Outdated
Review
Copy Link

The naming convention to copy should be from meshes: BKE_mesh_material_index_remove

The naming convention to copy should be from meshes: `BKE_mesh_material_index_remove`
{
using namespace blender;
using namespace blender::bke;
for (GreasePencilDrawingBase *base : grease_pencil->drawings()) {
if (base->type != GP_DRAWING) {
continue;
}
greasepencil::Drawing &drawing = reinterpret_cast<GreasePencilDrawing *>(base)->wrap();
MutableAttributeAccessor attributes = drawing.strokes_for_write().attributes_for_write();
antoniov marked this conversation as resolved
Hans Goudey commented 2023-11-15 14:33:58 +01:00
Review
Copy Link

Call lookup_for_write instead of lookup_or_add_for_write_span. Removing a material shouldn't add the material_index attribute. Check BKE_mesh_material_index_remove for an example.

Call `lookup_for_write` instead of `lookup_or_add_for_write_span`. Removing a material shouldn't add the `material_index` attribute. Check `BKE_mesh_material_index_remove` for an example.
AttributeWriter<int> material_indices = attributes.lookup_for_write<int>("material_index");
if (!material_indices) {
return;
}
MutableVArraySpan<int> indices_span(material_indices.varray);
for (const int i : indices_span.index_range()) {
if (indices_span[i] > 0 && indices_span[i] >= index) {
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-16 14:00:39 +01:00
Outdated
Review
Copy Link

These parentheses are unnecessary, std::max should be used too. Better to just copy the logic from BKE_mesh_material_index_remove directly.

These parentheses are unnecessary, `std::max` should be used too. Better to just copy the logic from `BKE_mesh_material_index_remove` directly.
indices_span[i]--;
}
antoniov marked this conversation as resolved Outdated
Falk David commented 2023-11-16 10:30:20 +01:00
Outdated
Review
Copy Link

Writing this in a single line as indices_span[i] = std::max(indices_span[i] - 1, 0); seems a bit better.

Writing this in a single line as `indices_span[i] = std::max(indices_span[i] - 1, 0);` seems a bit better.
}
indices_span.save();
material_indices.finish();
}
}
bool BKE_grease_pencil_material_index_used(GreasePencil *grease_pencil, int index)
{
using namespace blender;
using namespace blender::bke;
for (GreasePencilDrawingBase *base : grease_pencil->drawings()) {
if (base->type != GP_DRAWING) {
continue;
}
greasepencil::Drawing &drawing = reinterpret_cast<GreasePencilDrawing *>(base)->wrap();
AttributeAccessor attributes = drawing.strokes().attributes();
const VArraySpan<int> material_indices = *attributes.lookup_or_default<int>(
"material_index", ATTR_DOMAIN_CURVE, 0);
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-15 14:34:22 +01:00
Outdated
Review
Copy Link

Read material indices with the default of 0, not -1, which is an invalid value.

Read material indices with the default of 0, not -1, which is an invalid value.
if (material_indices.contains(index)) {
return true;
}
}
return false;
}
/** \} */
/* ------------------------------------------------------------------- */

12
source/blender/blenkernel/intern/material.cc
View File

@ -463,6 +463,9 @@ static void material_data_index_remove_id(ID *id, short index)
case ID_CU_LEGACY:
BKE_curve_material_index_remove((Curve *)id, index);
break;
case ID_GP:
BKE_grease_pencil_material_index_remove(reinterpret_cast<GreasePencil *>(id), index);
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-16 15:41:54 +01:00
Outdated
Review
Copy Link

reinterpret_cast<GreasePencil *>(id)

New code should use C++ casts for pointers instead of C-style casts which can remove const.

`reinterpret_cast<GreasePencil *>(id)` New code should use C++ casts for pointers instead of C-style casts which can remove const.
break;
case ID_MB:
case ID_CV:
case ID_PT:
@ -501,6 +504,9 @@ bool BKE_object_material_slot_used(Object *object, short actcol)
return false;
case ID_GD_LEGACY:
return BKE_gpencil_material_index_used((bGPdata *)ob_data, actcol - 1);
case ID_GP:
return BKE_grease_pencil_material_index_used(reinterpret_cast<GreasePencil *>(ob_data),
actcol - 1);
default:
return false;
}
@ -1358,14 +1364,14 @@ bool BKE_object_material_slot_remove(Main *bmain, Object *ob)
}
}
/* check indices from mesh */
if (ELEM(ob->type, OB_MESH, OB_CURVES_LEGACY, OB_SURF, OB_FONT)) {
/* check indices from mesh and grease pencil. */
if (ELEM(ob->type, OB_MESH, OB_CURVES_LEGACY, OB_SURF, OB_FONT, OB_GREASE_PENCIL)) {
material_data_index_remove_id((ID *)ob->data, actcol - 1);
if (ob->runtime.curve_cache) {
BKE_displist_free(&ob->runtime.curve_cache->disp);
}
}
/* check indices from gpencil */
/* check indices from gpencil legacy. */
else if (ob->type == OB_GPENCIL_LEGACY) {
antoniov marked this conversation as resolved Outdated
Hans Goudey commented 2023-11-16 13:57:43 +01:00
Outdated
Review
Copy Link

Please change material_data_index_remove_id instead of adding this here. We should be consistent with the way other IDs are handled.

Please change `material_data_index_remove_id` instead of adding this here. We should be consistent with the way other IDs are handled.
BKE_gpencil_material_index_reassign((bGPdata *)ob->data, ob->totcol, actcol - 1);
}