DNA: Cleanup endian switching when loading file.
ClosedPublic
Actions

Authored by Jacques Lucke (JacquesLucke) on Oct 2 2020, 12:11 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Commits

rB51e43f27fa2e: DNA: cleanup endian switching when loading file

Summary

This patch "modernizes" DNA_struct_switch_endian similar to how I updated DNA_struct_reconstruct recently. Furthermore, some special case handling has been moved/removed.
Unfortunately, I don't have any 64 bit big endian file to test this with. Our tests don't contain such a file, it's probably quite a rare combination in general...

The first special case that I moved somewhere else is the blocktype member. It was used by the old animation system and was not supposed to be endian switched. Now it is endian switched in the generic code and then switched back during direct linking. Something similar is done in other cases as well. For example IDP_DirectLinkProperty.

Secondly, the way pointers were endian switched before was confusing. Sometimes they were switched and sometimes not depending on different factors. Now, they are always switched when the endianness is different, and never switched when the endianness is the same.

Diff Detail

Repository: rB Blender

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Oct 2 2020, 12:11 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B10533: Diff 29507.Oct 2 2020, 12:11 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 2 2020, 12:22 PM

Jacques Lucke (JacquesLucke) added inline comments.Oct 2 2020, 12:26 PM

source/blender/blenloader/intern/readfile.c
779	This is not necessary anymore, because the pointer has been endian switched before.
source/blender/makesdna/intern/dna_genfile.c
1033–1038	Here we were switching 8 byte pointers only, which I found confusing.
1044	This special case has been moved to `direct_link_ipo`.

Jacques Lucke (JacquesLucke) added a reviewer: Campbell Barton (campbellbarton).Oct 2 2020, 12:26 PM

Jacques Lucke (JacquesLucke) mentioned this in T81226: OldNewMap handling of pointers in blenloader does not work correctly with FD_FLAGS_SWITCH_ENDIAN.Oct 5 2020, 5:07 PM

Merge branch 'master' into dna-endian-switch
clang tidy cleanup

Harbormaster completed remote builds in B10580: Diff 29628.Oct 6 2020, 1:47 PM

@Campbell Barton (campbellbarton) this just became more important, because the master branch does not work on 64 bit big endian systems currently! I got the file below from T81226. It cannot be opened in master, but can be opened with this patch applied.

big_endian_64bit.blend712 KBDownload

Merge branch 'master' into dna-endian-switch
cleanup

Harbormaster completed remote builds in B10601: Diff 29682.Oct 7 2020, 1:17 PM

NOTE: Committed an alternate fix to T81226, as this is unrelated to this cleanup.

Switching all addresses seems unnecessarily, both because it's extra logic to run and it could cause unforeseen problems.

The old address is just an ID, it's only used for lookups, switching was only needed when reading 64bit files on 32bit system so meaningful bits aren't masked out.

bh4_from_bh8 can be left as is. The STRUCT_MEMBER_CATEGORY_POINTER can be made only to switch when reading 64bit files on 32bit systems (matching current master).

Otherwise this cleanup seems fine.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 8 2020, 6:18 PM

Closed by commit rB51e43f27fa2e: DNA: cleanup endian switching when loading file (authored by Jacques Lucke (JacquesLucke)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Jacques Lucke (JacquesLucke) added a commit: rB51e43f27fa2e: DNA: cleanup endian switching when loading file.