Page MenuHome

DNA renaming support
AbandonedPublic

Authored by Campbell Barton (campbellbarton) on Feb 10 2019, 3:36 PM.

Details

Summary

This patch supports renaming structs and struct members in DNA.

Motivation

Over time Blender's internal struct names and their members get out of sync with whats used in the interface, some of the examples include SpaceOops (for the outliner), SpaceButs (properties editor).

In some cases its just a bit confusing, other times its misleading and makes the code hard to follow. It also means developers have to remember two names for the same thing.

Other examples:

Struct members:

- Object.size -> scale
- Object.dupli_ofs -> instance_offset
- Camera.YF_dofdist -> dof_dist;

Structs:

- Lamp -> Light
- SpaceOops -> SpaceOutliner
- SpaceButs -> SpaceProperties

Implementation

This patch supports reading old SDNA with renamed fields, by patching them on load.

Unlike do_versions in readfile.c, version information is stored in an array, with the first two members being the version, subversion used to check if updates are needed.

/* Struct rename. */
DNA_sdna_patch_struct("StructNameOld", "StructNameNew");
/* Struct member rename. */
DNA_sdna_patch_struct_member("Struct", "member_old", "member_new");

Leading/trailing characters are accounted for, so you don't need to include pointer or array sizes, just the name, so PreviewImage's unsigned int *rect[2] can be renamed to unsigned int *image_data[2] with:

DNA_sdna_patch_struct_member("PreviewImage", "rect", "image_data")

It does, of course - mean loading these files in older Blender versions wont work (the values will be cleared, in some cases this will fail gracefully, in others not, so this will influence what we choose to rename).

Testing

Some things tested to work.

  • Rename UserDef to bPreferences.
  • Rename Object.size to Object.scale
  • Rename PreviewImage.rect to PreviewImage.image_data (not a full check, just to see the renaming arrays and pointers properly converts the name).
  • Updating old 2.8x files (already in this patch, try this old 2.8x file which runs the 2.8x view layer changes ).

Open Topics

Accessing Sub-Version from DNA

Getting the subversion from the Blend file isn't so simple because this is stored in FileGlobal currently this is read from the structure without decoding since it has been the first member since 2.43 (added a static assert to ensure it's always the case). This means we can't read the subversion from older files but I don't think we'll need this anyway.

When to Rename?

Suggest only major releases, if there are breakages in forward compatibility.

Support for opening 2.8x files in 2.7x is already limited, so while it shouldn't crash, loosing some settings is acceptable (Camera.YF_dofdist for eg).

Note that this isn't that extreme given that objects currently don't show up at all.

For future 2.8x releases we'll limit renaming since to cases that wont break forward compatibility since its useful to be able to test on older point releases.

We could be less struct with user preferences since users generally don't load new preferences in older versions of blender.

Diff Detail

Repository
rB Blender
Branch
TEMP-DNA-RENAME (branched from master)
Build Status
Buildable 2884
Build 2884: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
  • Comments.
  • Correct version check.
  • Ensure versions are increasing.
Harbormaster completed remote builds in B2868: Diff 13582.

Rename variables, avoid right shift.

Harbormaster completed remote builds in B2869: Diff 13583.
Harbormaster completed remote builds in B2870: Diff 13584.

Use version patching for 2.8x SceneLayer -> ViewLayer renaming.

Harbormaster completed remote builds in B2873: Diff 13587.

Write the version into the SDNA instead of using a 'TEST' BCode

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Read the subversion from Global, it's always the first member and we don't need to DNA-decode it.

Avoid reading in junk subversion values for old blend files.

Move DNA versioning into it's own function versioning_dna.c.

Harbormaster completed remote builds in B2877: Diff 13592.
source/blender/makesdna/intern/dna_genfile.c
1424–1446

While this works it can be simplified (will do soon).

Campbell Barton (campbellbarton) marked an inline comment as not done.Feb 11 2019, 10:27 AM
Brecht Van Lommel (brecht) requested changes to this revision.Feb 11 2019, 11:11 AM

I think we should keep file compatibility and keep the old names in the file.

This way we can do renaming / refactoring at any time, as you're working in some area, without having to do it at specific releases.

This revision now requires changes to proceed.Feb 11 2019, 11:11 AM

I would favour a solution that allows for forward compatibility. Even if the old DNA struct is filled with a default value, and not the data from the equivalent new struct. That should happen for both old files that are converted, and new files created after the rename.

Discussed in IRC, I'll work on a solution that does forward compatibility, however we already do non-forward-compatible renaming (as a hack), so this patch can still be used in the rare cases we need to rename structs or struct members.

  • Simplify logic for str_member_match a little.
Brecht Van Lommel (brecht) requested changes to this revision.Feb 11 2019, 7:46 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/blenloader/intern/readfile.c
956

It's not clear to me why changing to data_alloc = false here is safe. It may be but it's not obvious to me from looking at this code that bhead[1] is never freed before DNA. In the other case we know it's a static string embedded in the executable.

Any specific reason for that change?

source/blender/blenloader/intern/versioning_dna.c
31–32

Please add a comment about only doing this in rare cases where we agree to break forward compatibility.

source/blender/makesdna/DNA_genfile.h
106–107

Why is this not a static function?

For argument naming, maybe old and new are more clear than src and dst.

111–112

Same question.

This revision now requires changes to proceed.Feb 11 2019, 7:46 PM

One thing I’d like to add here, and which also pushes towards not breaking forward compat as much as possible, is the ability to link/append new data into old Blender. Even if 2.8 .blend files won't open in 2.7x, it is still possible to append from them most of the useful data-blocks with relatively little loss of data. Changes like Object.size to Object.scale is a typical case that should be forbidden to be performed with that patch, imho. Even in such big breaking steps as the 2.8 one. ;)

Campbell Barton (campbellbarton) added inline comments.
source/blender/blenloader/intern/readfile.c
956

Removed because it was used for versioning check, became unused argument - and worked without.
But your right, the lifetime of this data isn't so clear compared to the BHead so better leave.

source/blender/makesdna/DNA_genfile.h
106–107

At the time was expecting these would be used for a lot more versioning - where there would be more advantage in doing a single struct lookup then operating on it.

Made static, can always make public again to avoid lots of lookups - if that ever happens.