blender
/
blender
135
420
Fork 603
Code Issues 6.3k Pull Requests 756 Projects 19 Wiki Activity

BMesh: Respect data alignment for attributes #109237

Open
Joseph Eagar wants to merge 8 commits from JosephEagar/blender:temp-customdata-aligned-offsets into main
pull from: JosephEagar/blender:temp-customdata-aligned-offsets
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 0 Commits 8 Files Changed 1 +13 -11
1 changed files with 13 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1e3de4c7c7 - Show all commits

24
source/blender/blenkernel/intern/customdata.cc
View File

@ -62,6 +62,8 @@
/* only for customdata_data_transfer_interp_normal_normals */
#include "data_transfer_intern.h"
#include <array>
JosephEagar marked this conversation as resolved Outdated
Hans Goudey commented 2023-06-22 13:19:10 +02:00
Outdated
Review
Copy Link

Unnecessary change

Unnecessary change
using blender::Array;
using blender::float2;
using blender::ImplicitSharingInfo;
@ -2544,6 +2546,8 @@ void CustomData_free_typemask(CustomData *data, const int totelem, eCustomDataMa
static int customData_get_alignment(eCustomDataType type)
JosephEagar marked this conversation as resolved
Hans Goudey commented 2023-06-22 13:16:19 +02:00
Review
Copy Link

This comment isn't quite right, since CD_PROP_BYTE_COLOR is also an array type. Maybe "Handle float array types"?

This comment isn't quite right, since `CD_PROP_BYTE_COLOR` is also an array type. Maybe "Handle float array types"?
Joseph Eagar commented 2023-06-22 13:39:20 +02:00
Review
Copy Link

I has an if statement for that. I admit the comment isn't clear, it applies to the three if statements (for 4,2,1 alignments).

I has an if statement for that. I admit the comment isn't clear, it applies to the three if statements (for 4,2,1 alignments).
Hans Goudey commented 2023-06-22 13:47:51 +02:00
Review
Copy Link

Ah I see. I don't think the comment is necessary honestly

Ah I see. I don't think the comment is necessary honestly
{
/* Handle array types. */
/* Float and integer arrays. */
if (ELEM(type,
CD_PROP_FLOAT2,
CD_PROP_FLOAT3,
@ -2558,10 +2562,12 @@ static int customData_get_alignment(eCustomDataType type)
return 4;
}
/* Short arrays. */
if (ELEM(type, CD_TESSLOOPNORMAL)) {
return 2;
}
/* Byte arrays. */
if (type == CD_PROP_BYTE_COLOR) {
return 1;
}
@ -2585,26 +2591,21 @@ static int customData_get_alignment(eCustomDataType type)
/* Update BMesh block offsets, respects alignment. */
static void customData_update_offsets(CustomData *data)
{
const LayerTypeInfo *typeInfo;
if (data->totlayer == 0) {
data->totsize = 0;
return;
}
JosephEagar marked this conversation as resolved Outdated
Hans Goudey commented 2023-06-22 13:19:01 +02:00
Outdated
Review
Copy Link

std::array should work here, a bit nicer IMO

`std::array` should work here, a bit nicer IMO
const std::array<int, 4> aligns = {8, 4, 2, 1};
int max_alignment = 1;
int aligns[] = {8, 4, 2, 1};
int offset = 0;
JosephEagar marked this conversation as resolved Outdated
Hans Goudey commented 2023-06-22 13:15:30 +02:00
Outdated
Review
Copy Link

Declare variables const

Declare variables const
for (int align : aligns) {
for (int i = 0; i < data->totlayer; i++) {
CustomDataLayer *layer = data->layers + i;
for (int i = 0; i < ARRAY_SIZE(aligns) + 1; i++) {
for (int j = 0; j < data->totlayer; j++) {
CustomDataLayer *layer = data->layers + j;
int align = customData_get_alignment(eCustomDataType(layer->type));
max_alignment = max_ii(max_alignment, align);
if (align != aligns[i]) {
int layer_align = customData_get_alignment(eCustomDataType(layer->type));
if (layer_align != align) {
continue;
}
@ -2616,6 +2617,7 @@ static void customData_update_offsets(CustomData *data)
}
offset += size;
max_alignment = max_ii(max_alignment, align);
}
}