blender
/
blender
136
413
Fork 596
Code Issues 6.3k Pull Requests 771 Projects 19 Wiki Activity

Core: Fix broken CustomData IO with Autosave files #106648

Merged
Joseph Eagar merged 5 commits from temp-autosave-bug into main 2023-04-07 02:02:31 +02:00
Conversation 0 Commits 5 Files Changed 2 +34
3 changed files with 34 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 182d83f281 - Show all commits

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

@ -4217,6 +4217,16 @@ void CustomData_blend_write_prepare(CustomData &data,
}
data.totlayer = layers_to_write.size();
data.maxlayer = data.totlayer;
/* Note: data->layers may be null, this happens when adding
* a legacy MPoly struct to a mesh with no other face attributes.
* This leaves us with no unique ID for DNA to key off of when
Hans Goudey commented 2023-04-07 01:33:48 +02:00
Outdated
Review
Copy Link

I wonder if there's a more specific wording than "key off of". What about something like "identify the old data with"?

I think my lack of understanding here is the key part of the problem.

I wonder if there's a more specific wording than "key off of". What about something like "identify the old data with"? I think my lack of understanding here is the key part of the problem.
* loading the file.
*/
if (!data.layers && layers_to_write.size() > 0) {
/* We just need an address that's unique. */
data.layers = reinterpret_cast<CustomDataLayer *>(&data.layers);
Hans Goudey commented 2023-04-07 01:51:41 +02:00
Review
Copy Link

Would using layers_to_write.data() here be a little less "weird"?

No strong opinion since this is a bit hacky anyway, but it would look less suspect anyway.

Would using `layers_to_write.data()` here be a little less "weird"? No strong opinion since this is a bit hacky anyway, but it would look less suspect anyway.
Joseph Eagar commented 2023-04-07 01:58:39 +02:00
Review
Copy Link

There's no guarantee it will always be unique.

There's no guarantee it will always be unique.
Hans Goudey commented 2023-04-07 02:01:29 +02:00
Review
Copy Link

Ah right, this is the best way to get an actually unique pointer I guess.

Ah right, this is the best way to get an actually unique pointer I guess.
}
}
int CustomData_sizeof(const eCustomDataType type)

1
source/blender/blenkernel/intern/mesh_legacy_convert.cc
View File

@ -1243,6 +1243,7 @@ void BKE_mesh_legacy_sharp_faces_from_flags(Mesh *mesh)
if (attributes.contains("sharp_face")) {
return;
}
Hans Goudey commented 2023-04-07 01:28:53 +02:00
Outdated
Review
Copy Link

Unnecessary white-space change here

Unnecessary white-space change here
const Span<MPoly> polys(static_cast<const MPoly *>(CustomData_get_layer(&mesh->pdata, CD_MPOLY)),
mesh->totpoly);
if (std::any_of(polys.begin(), polys.end(), [](const MPoly &poly) {

23
source/blender/blenloader/intern/versioning_400.cc
View File

@ -8,25 +8,48 @@
#include "CLG_log.h"
#include "DNA_mesh_types.h"
Hans Goudey commented 2023-04-07 01:52:05 +02:00
Outdated
Review
Copy Link

All these includes are unnecessary now :)

All these includes are unnecessary now :)
#include "DNA_movieclip_types.h"
#include "BLI_assert.h"
#include "BLI_listbase.h"
#include "BKE_customdata.h"
#include "BKE_main.h"
#include "BKE_mesh_legacy_convert.h"
#include "BKE_tracking.h"
#include "BLI_index_range.hh"
#include "BLI_offset_indices.hh"
#include "BLO_readfile.h"
#include "readfile.h"
#include "versioning_common.h"
using blender::IndexRange;
using blender::OffsetIndices;
// static CLG_LogRef LOG = {"blo.readfile.doversion"};
static void version_mesh_legacy_to_struct_of_array_format(Mesh &mesh)
{
/* Autosave files don't have CD_MPOLY layers.
*/
if (!CustomData_has_layer(&mesh.pdata, CD_MPOLY) && mesh.poly_offset_indices) {
Hans Goudey commented 2023-04-07 01:31:05 +02:00
Outdated
Review
Copy Link

It shouldn't be necessary to add this at runtime, since we don't use the polys at all. We're meant to be able to read the new (not legacy format) too already. Is this preventing another crash in some of the functions below?

It shouldn't be necessary to add this at runtime, since we don't use the polys at all. We're meant to be able to read the new (not legacy format) too already. Is this preventing another crash in some of the functions below?
Joseph Eagar commented 2023-04-07 01:39:49 +02:00
Outdated
Review
Copy Link

A fair number of the conversion functions are using MPolys (it crashes in the sharp face one).

A fair number of the conversion functions are using MPolys (it crashes in the sharp face one).
MPoly *polys_legacy = static_cast<MPoly *>(
CustomData_add_layer(&mesh.pdata, CD_MPOLY, CD_CONSTRUCT, mesh.totpoly));
/* Getting a link error when using mesh.polys(),
* for now just read poly_offset_indices directly.
*/
for (int poly_i : IndexRange(mesh.totpoly)) {
polys_legacy[poly_i].loopstart = mesh.poly_offset_indices[poly_i];
polys_legacy[poly_i].totloop = mesh.poly_offset_indices[poly_i + 1] -
mesh.poly_offset_indices[poly_i];
}
}
BKE_mesh_legacy_convert_flags_to_selection_layers(&mesh);
BKE_mesh_legacy_convert_flags_to_hide_layers(&mesh);
BKE_mesh_legacy_convert_uvs_to_generic(&mesh);