blender
/
blender
136
406
Fork 590
Code Issues 6.3k Pull Requests 763 Projects 19 Wiki Activity

Fix #114428: clamp setting object active_material_index #120434

Merged
Philipp Oeser merged 4 commits from lichtwerk/blender:114428 into main 2024-04-12 17:35:33 +02:00
Conversation 1 Commits 4 Files Changed 1 +2
1 changed files with 2 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/blender/makesrna/intern/rna_object.cc
View File

@ -1120,6 +1120,8 @@ static int rna_Object_active_material_index_get(PointerRNA *ptr)
static void rna_Object_active_material_index_set(PointerRNA *ptr, int value)
{
Object *ob = reinterpret_cast<Object *>(ptr->owner_id);
Campbell Barton commented 2024-04-10 03:20:20 +02:00
Review
Copy Link

There is an error in the code introduced as mat_nr is zero-based and actcol is 1 based, so the assignment should be mat_nr = ob->actcol - 1 however I think it's better to clamp value once at the beginning of the function.

There is an error in the code introduced as `mat_nr` is zero-based and `actcol` is 1 based, so the assignment should be `mat_nr = ob->actcol - 1` however I think it's better to clamp value once at the beginning of the function.
value = std::max(std::min(value, ob->totcol - 1), 0);
Campbell Barton commented 2024-04-10 10:23:40 +02:00
Outdated
Review
Copy Link

There may be no materials which will assert, suggest

value = (ob->totcol > 1) ? std::clamp(value, 0, ob->totcol - 1) : 0;

There may be no materials which will assert, suggest `value = (ob->totcol > 1) ? std::clamp(value, 0, ob->totcol - 1) : 0;`
👍 1
Hans Goudey commented 2024-04-10 14:08:50 +02:00
Outdated
Review
Copy Link

std::max(std::min(value, ob->totcol - 1), 0); might be even simpler

`std::max(std::min(value, ob->totcol - 1), 0);` might be even simpler
ob->actcol = value + 1;
Hans Goudey commented 2024-04-10 02:54:51 +02:00
Outdated
Review
Copy Link

std::clamp(value, 0, ob->totcol) seems simpler here

`std::clamp(value, 0, ob->totcol)` seems simpler here
👍 1
Philipp Oeser commented 2024-04-10 10:21:12 +02:00
Outdated
Review
Copy Link

std::clamp(value, 0, ob->totcol) seems simpler here

we should really clamp to ob->totcol -1 here then, no?

> `std::clamp(value, 0, ob->totcol)` seems simpler here we should really clamp to `ob->totcol -1` here then, no?
if (ob->type == OB_MESH) {