Page MenuHome

Store custom transform orientations per workspace
ClosedPublic

Authored by Julian Eisel (Severin) on May 24 2017, 10:39 PM.

Details

Summary
  • Move list of custom transform orientations from Scene to WorkSpace struct.
  • Store active transform orientation as pointer separate from View3D.twmode (twmode can only be set to preprocessor defined values now).
  • Display custom transform orientation name in header when transforming in it (used to show "global" which isn't really correct).

Had to do some uglyish things for file reading again, but it's not too
bad IMHO (certainly not worse than previously in workspace branch).

Patch based on workspaces branch. Note that I plan to undo some changes
regarding transform orientations in workspace branch.

Diff Detail

Repository
rB Blender
Branch
temp-workspace-transform-orientation
Build Status
Buildable 683
Build 683: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) planned changes to this revision.May 24 2017, 11:17 PM

Planning changes. Not much of the actual implementation should change however, so it's fine to do a first review already.
Also just found a crash when reading transform orientations from old files, think it's easy to fix, JFYI.

  • Fix various crashes/glitches when reading transform orientations of old files
  • Undo changes in workspaces branch and merge into this patch
  • Minor cleanup and fix wrong argument list caused by merge
Bastien Montagne (mont29) requested changes to this revision.May 28 2017, 2:48 PM

Only did very quick review so far, think this is going in the right direction, but… You are still trying to save & reload a 'foreign' data pointer!

fd->globmap & co might be nice trick, but this will not survive linking libraries. Further more, this will make remapping etc. of IDs more complicated, since it will require a special case handling.

Once again: a given datablock shall not be allowed to store a pointer to data from another datablock. Ideally, not at all, ever. In practice, this might be needed for performances reasons - in that case, pointer should be considered as purely runtime, temp cache data. It means that:

  1. It should merely be NULL'ed in readfile.c.
  2. A way to retrieve pointer should be stored in a robust, failsafe way (here, probably name of custom transform).
  3. A way to sanitize/validate/restore proper pointer from that 'failsafe' storage should be provided, as well as a way to tag pointer as dirty (a new flag in twflag? ).

Point 3 is crucial for any serious datablock management! It also allows for lazy loading (you only search for valid pointer when you need it).

This revision now requires changes to proceed.May 28 2017, 2:48 PM

@Julian Eisel (Severin) do you think you can tackle this next day or so? Otherwise I’ll try to it, would really, really like to merge the whole workspace in blender2.8 this week. ;)

What I'll probably do now is storing active active transform orientation as index internally, WorkSpace API can return the pointer then.

@Julian Eisel (Severin) do you think you can tackle this next day or so? Otherwise I’ll try to it, would really, really like to merge the whole workspace in blender2.8 this week. ;)

I'm on it, can hopefully get it done by tomorrow noon or Thursday latest. (Sorry for being so unreliable lately...)

What I'll probably do now is storing active active transform orientation as index internally, WorkSpace API can return the pointer then.

@Julian Eisel (Severin) do you think you can tackle this next day or so? Otherwise I’ll try to it, would really, really like to merge the whole workspace in blender2.8 this week. ;)

I'm on it, can hopefully get it done by tomorrow noon or Thursday latest. (Sorry for being so unreliable lately...)

Sounds good (and no problem, know what real life can be ;) ).

Julian Eisel (Severin) edited edge metadata.
  • Store active 3D-View transform orientation as index not as pointer
  • Sync with blender2.8/workspaces

LGTM (besides picky naming note below). :)

source/blender/makesdna/DNA_view3d_types.h
229

Maybe rather custom_orientation_index? Two letters more won't change much here, and iirc we try to avoid condensed naming in wide-spread code nowadays, hence especially in DNA area. ;)

This revision is now accepted and ready to land.May 31 2017, 9:42 PM

Committed rB46fc0bb87ebda, thanks a lot for the review!