Page MenuHome
Differential D14365 Diff 59272 source/blender/blenkernel/intern/mesh.cc

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/mesh.cc

Show First 20 Lines • Show All 282 Lines • ▼ Show 20 Lines if (!BLO_write_is_undo(writer)) {
mesh->mloop = const_cast<MLoop *>(mesh->loops().data()); mesh->mloop = const_cast<MLoop *>(mesh->loops().data());
mesh->dvert = const_cast<MDeformVert *>(mesh->deform_verts().data()); mesh->dvert = const_cast<MDeformVert *>(mesh->deform_verts().data());
} }
CustomData_blend_write_prepare(mesh->vdata, vert_layers, names_to_skip); CustomData_blend_write_prepare(mesh->vdata, vert_layers, names_to_skip);
CustomData_blend_write_prepare(mesh->edata, edge_layers, names_to_skip); CustomData_blend_write_prepare(mesh->edata, edge_layers, names_to_skip);
CustomData_blend_write_prepare(mesh->ldata, loop_layers, names_to_skip); CustomData_blend_write_prepare(mesh->ldata, loop_layers, names_to_skip);
CustomData_blend_write_prepare(mesh->pdata, poly_layers, names_to_skip); CustomData_blend_write_prepare(mesh->pdata, poly_layers, names_to_skip);
if (!BLO_write_is_undo(writer)) {
BKE_mesh_legacy_convert_uvs_to_struct(mesh, temp_arrays_for_legacy_format, loop_layers);
brechtUnsubmitted
Done
Inline Actions

Is the plan to do this always without any users setting?

I'm not really sure what the right choice is, just wondering if there was some decision to keep things forward compatible for now?

brecht: Is the plan to do this always without any users setting? I'm not really sure what the right…
HooglyBooglyUnsubmitted
Done
Inline Actions

Yep, the plan we arrived at (D14583, T95965) eventually was keeping forward compatibility until 4.0, without an option at all. I'm pretty happy with that compromise so far.

HooglyBoogly: Yep, the plan we arrived at (D14583, T95965) eventually was keeping forward compatibility until…
}
} }
mesh->runtime = nullptr; mesh->runtime = nullptr;
BLO_write_id_struct(writer, Mesh, id_address, &mesh->id); BLO_write_id_struct(writer, Mesh, id_address, &mesh->id);
BKE_id_blend_write(writer, &mesh->id); BKE_id_blend_write(writer, &mesh->id);
/* direct data */ /* direct data */
Show All 22 Lines
static void mesh_blend_read_data(BlendDataReader *reader, ID *id) static void mesh_blend_read_data(BlendDataReader *reader, ID *id)
{ {
Mesh *mesh = (Mesh *)id; Mesh *mesh = (Mesh *)id;
BLO_read_pointer_array(reader, (void **)&mesh->mat); BLO_read_pointer_array(reader, (void **)&mesh->mat);
/* Deprecated pointers to custom data layers are read here for backward compatibility /* Deprecated pointers to custom data layers are read here for backward compatibility
* with files where these were owning pointers rather than a view into custom data. */ * with files where these were owning pointers rather than a view into custom data. */
BLO_read_data_address(reader, &mesh->mvert); BLO_read_data_address(reader, &mesh->mvert);
HooglyBooglyUnsubmitted
Done
Inline Actions

Looks like the patch needs a clang format

HooglyBoogly: Looks like the patch needs a clang format
BLO_read_data_address(reader, &mesh->medge); BLO_read_data_address(reader, &mesh->medge);
BLO_read_data_address(reader, &mesh->mface); BLO_read_data_address(reader, &mesh->mface);
BLO_read_data_address(reader, &mesh->mtface); BLO_read_data_address(reader, &mesh->mtface);
BLO_read_data_address(reader, &mesh->dvert); BLO_read_data_address(reader, &mesh->dvert);
BLO_read_data_address(reader, &mesh->tface); BLO_read_data_address(reader, &mesh->tface);
BLO_read_data_address(reader, &mesh->mcol); BLO_read_data_address(reader, &mesh->mcol);
HooglyBooglyUnsubmitted
Done
Inline Actions

Yes, I think that's a correct assumption. Actually It's a bit weird these pointers are read at all, since they all point to layers owned by CustomData.
I'd expect that BKE_mesh_update_customdata_pointers would be called instead of reading these values directly.

So I'd guess that this line could just be removed.

HooglyBoogly: Yes, I think that's a correct assumption. Actually It's a bit weird these pointers are read at…
BaardaapAuthorUnsubmitted
Done
Inline Actions

So far any blendfile which I've tested this on had lecacy_mloopuv being read as NULL here so I think it can probably just go away indeed.

Baardaap: So far any blendfile which I've tested this on had lecacy_mloopuv being read as NULL here so I…
BaardaapAuthorUnsubmitted
Done
Inline Actions

I put it back in to ease comparison with older versions during debugging.

Baardaap: I put it back in to ease comparison with older versions during debugging.
BLO_read_data_address(reader, &mesh->mselect); BLO_read_data_address(reader, &mesh->mselect);
/* animdata */ /* animdata */
BLO_read_data_address(reader, &mesh->adt); BLO_read_data_address(reader, &mesh->adt);
BKE_animdata_blend_read_data(reader, mesh->adt); BKE_animdata_blend_read_data(reader, mesh->adt);
BLO_read_list(reader, &mesh->vertex_group_names); BLO_read_list(reader, &mesh->vertex_group_names);
Show All 22 Linesstatic void mesh_blend_read_data(BlendDataReader *reader, ID *id)
if (BLO_read_requires_endian_switch(reader) && mesh->tface) { if (BLO_read_requires_endian_switch(reader) && mesh->tface) {
TFace *tf = mesh->tface; TFace *tf = mesh->tface;
for (int i = 0; i < mesh->totface; i++, tf++) { for (int i = 0; i < mesh->totface; i++, tf++) {
BLI_endian_switch_uint32_array(tf->col, 4); BLI_endian_switch_uint32_array(tf->col, 4);
} }
} }
} }
HooglyBooglyUnsubmitted
Done
Inline Actions

Ideally this would go in versioning code (versioning_300.c) rather than file reading, since then it would only run on old files.

HooglyBoogly: Ideally this would go in versioning code (`versioning_300.c`) rather than file reading, since…
BaardaapAuthorUnsubmitted
Done
Inline Actions

Yeah. I looked at the code in versioning_300.c and couldn't quickly find where I would need to put it. I'll need to carefully study that file to understand what happens better.

So I decided to stick it here (where it was easy) first. But it's good you added this comment, because I should have documented this 'temporary' thing before I'd have forgotten about it ;-)

Baardaap: Yeah. I looked at the code in versioning_300.c and couldn't quickly find where I would need to…
static void mesh_blend_read_lib(BlendLibReader *reader, ID *id) static void mesh_blend_read_lib(BlendLibReader *reader, ID *id)
{ {
Mesh *me = (Mesh *)id; Mesh *me = (Mesh *)id;
/* this check added for python created meshes */ /* this check added for python created meshes */
if (me->mat) { if (me->mat) {
for (int i = 0; i < me->totcol; i++) { for (int i = 0; i < me->totcol; i++) {
BLO_read_id_address(reader, me->id.lib, &me->mat[i]); BLO_read_id_address(reader, me->id.lib, &me->mat[i]);
} }
▲ Show 20 Lines • Show All 90 Lines • ▼ Show 20 Lines case MESHCMP_CDLAYERS_MISMATCH:
return "CustomData Layer Count Mismatch"; return "CustomData Layer Count Mismatch";
case MESHCMP_ATTRIBUTE_VALUE_MISMATCH: case MESHCMP_ATTRIBUTE_VALUE_MISMATCH:
return "Attribute Value Mismatch"; return "Attribute Value Mismatch";
default: default:
return "Mesh Comparison Code Unknown"; return "Mesh Comparison Code Unknown";
} }
} }
static bool is_sublayer_name(char const *sublayer_name, char const *name)
{
BLI_assert(strlen(sublayer_name) == 2);
if (name[1] != sublayer_name[0]) {
return false;
}
if (name[2] != sublayer_name[1]) {
return false;
}
if (name[3] != '.') {
return false;
}
return true;
}
static bool is_uv_bool_sublayer(CustomDataLayer const *l)
{
char const *name = l->name;
if (name[0] != '.') {
return false;
}
return is_sublayer_name(UV_VERTSEL_NAME, name) || is_sublayer_name(UV_EDGESEL_NAME, name) ||
is_sublayer_name(UV_PINNED_NAME, name);
}
/** Thresh is threshold for comparing vertices, UV's, vertex colors, weights, etc. */ /** Thresh is threshold for comparing vertices, UV's, vertex colors, weights, etc. */
static int customdata_compare( static int customdata_compare(
CustomData *c1, CustomData *c2, const int total_length, Mesh *m1, Mesh *m2, const float thresh) CustomData *c1, CustomData *c2, const int total_length, Mesh *m1, Mesh *m2, const float thresh)
{ {
const float thresh_sq = thresh * thresh;
CustomDataLayer *l1, *l2; CustomDataLayer *l1, *l2;
int layer_count1 = 0, layer_count2 = 0, j; int layer_count1 = 0, layer_count2 = 0, j;
const uint64_t cd_mask_non_generic = CD_MASK_MEDGE | CD_MASK_MPOLY | CD_MASK_MLOOPUV | const uint64_t cd_mask_non_generic = CD_MASK_MEDGE | CD_MASK_MPOLY | CD_MASK_PROP_BYTE_COLOR |
CD_MASK_PROP_BYTE_COLOR | CD_MASK_MDEFORMVERT; CD_MASK_MDEFORMVERT;
const uint64_t cd_mask_all_attr = CD_MASK_PROP_ALL | cd_mask_non_generic; const uint64_t cd_mask_all_attr = CD_MASK_PROP_ALL | cd_mask_non_generic;
const Span<MLoop> loops_1 = m1->loops(); const Span<MLoop> loops_1 = m1->loops();
const Span<MLoop> loops_2 = m2->loops(); const Span<MLoop> loops_2 = m2->loops();
/* The uv selection / pin layers are ignored in the comparisons because
* the original flags they replace were ignored as well. Because of the
* lazy creation of these layers it would need careful handling of the
* test files to compare these layers. For now it has been decided to
* skip them.
*/
for (int i = 0; i < c1->totlayer; i++) { for (int i = 0; i < c1->totlayer; i++) {
l1 = &c1->layers[i]; l1 = &c1->layers[i];
if ((CD_TYPE_AS_MASK(l1->type) & cd_mask_all_attr) && l1->anonymous_id == nullptr) { if ((CD_TYPE_AS_MASK(l1->type) & cd_mask_all_attr) && l1->anonymous_id == nullptr &&
!is_uv_bool_sublayer(l1)) {
layer_count1++; layer_count1++;
} }
} }
brechtUnsubmitted
Done
Inline Actions

Which test files is this referring to? Something saved with a branch that never ended up in master I guess?

brecht: Which test files is this referring to? Something saved with a branch that never ended up in…
BaardaapAuthorUnsubmitted
Done
Inline Actions

No there is at least one test which relies on the vertsel flags in MLoopUV being ignored while comparing. But we don't want to ignore all CD_PROP_BOOL layers. So the test file should probably be updated. I *think* there were a few other tests where the expected number of layers was hardcoded in the test.

Before landing in master I'll check all tests again with this test removed and make sure the tests are updated where needed.

Baardaap: No there is at least one test which relies on the vertsel flags in MLoopUV being ignored while…
for (int i = 0; i < c2->totlayer; i++) { for (int i = 0; i < c2->totlayer; i++) {
l2 = &c2->layers[i]; l2 = &c2->layers[i];
if ((CD_TYPE_AS_MASK(l2->type) & cd_mask_all_attr) && l2->anonymous_id == nullptr) { if ((CD_TYPE_AS_MASK(l2->type) & cd_mask_all_attr) && l2->anonymous_id == nullptr &&
!is_uv_bool_sublayer(l2)) {
layer_count2++; layer_count2++;
} }
} }
if (layer_count1 != layer_count2) { if (layer_count1 != layer_count2) {
/* TODO(@HooglyBoogly): Re-enable after tests are updated for material index refactor. */ /* TODO(@HooglyBoogly): Re-enable after tests are updated for material index refactor and UV as
* generic attribute refactor. */
// return MESHCMP_CDLAYERS_MISMATCH; // return MESHCMP_CDLAYERS_MISMATCH;
} }
l1 = c1->layers; l1 = c1->layers;
l2 = c2->layers; l2 = c2->layers;
for (int i1 = 0; i1 < c1->totlayer; i1++) { for (int i1 = 0; i1 < c1->totlayer; i1++) {
l1 = c1->layers + i1; l1 = c1->layers + i1;
if (l1->anonymous_id != nullptr) { if (l1->anonymous_id != nullptr || is_uv_bool_sublayer(l1)) {
continue; continue;
} }
bool found_corresponding_layer = false; bool found_corresponding_layer = false;
for (int i2 = 0; i2 < c2->totlayer; i2++) { for (int i2 = 0; i2 < c2->totlayer; i2++) {
l2 = c2->layers + i2; l2 = c2->layers + i2;
if (l1->type != l2->type || !STREQ(l1->name, l2->name) || l2->anonymous_id != nullptr) { if (l1->type != l2->type || !STREQ(l1->name, l2->name) || l2->anonymous_id != nullptr) {
continue; continue;
} }
▲ Show 20 Lines • Show All 51 Lines • ▼ Show 20 Lines for (int i2 = 0; i2 < c2->totlayer; i2++) {
for (j = 0; j < ltot; j++, lp1++, lp2++) { for (j = 0; j < ltot; j++, lp1++, lp2++) {
if (lp1->v != lp2->v) { if (lp1->v != lp2->v) {
return MESHCMP_LOOPMISMATCH; return MESHCMP_LOOPMISMATCH;
} }
} }
break; break;
} }
case CD_MLOOPUV: {
MLoopUV *lp1 = (MLoopUV *)l1->data;
MLoopUV *lp2 = (MLoopUV *)l2->data;
int ltot = m1->totloop;
for (j = 0; j < ltot; j++, lp1++, lp2++) {
if (len_squared_v2v2(lp1->uv, lp2->uv) > thresh_sq) {
return MESHCMP_LOOPUVMISMATCH;
}
}
break;
}
case CD_PROP_BYTE_COLOR: { case CD_PROP_BYTE_COLOR: {
MLoopCol *lp1 = (MLoopCol *)l1->data; MLoopCol *lp1 = (MLoopCol *)l1->data;
MLoopCol *lp2 = (MLoopCol *)l2->data; MLoopCol *lp2 = (MLoopCol *)l2->data;
int ltot = m1->totloop; int ltot = m1->totloop;
for (j = 0; j < ltot; j++, lp1++, lp2++) { for (j = 0; j < ltot; j++, lp1++, lp2++) {
if (lp1->r != lp2->r || lp1->g != lp2->g || lp1->b != lp2->b || lp1->a != lp2->a) { if (lp1->r != lp2->r || lp1->g != lp2->g || lp1->b != lp2->b || lp1->a != lp2->a) {
return MESHCMP_LOOPCOLMISMATCH; return MESHCMP_LOOPCOLMISMATCH;
▲ Show 20 Lines • Show All 1,261 Lines • Show Last 20 Lines