Page MenuHome

Local View refactor (split off from layers)
AbandonedPublic

Authored by Julian Eisel (Severin) on Aug 6 2016, 5:06 AM.

Details

Summary

Local View refactor (split off from layers)

Goal of this patch was to split local view storage off from layer bit-field, preparing for the layer manager changes of my GSoC project. It also includes some general refactoring for local view code.

Up to 32 local views are now supported, otherwise no changes should be visible for users.

Design Overview

Local view is now mostly handled through an API located in BKE_localview.h (inline functions). Just like the old one, this new system also uses bitfields, wrapped by a struct LocalViewInfo to make possible future changes easier. Ideally, I'd like to have the LocalViewInfo struct local, so devs are forced to use the API, but it needs to be on DNA level for file read/write.

Still need to check on RNA, Cycles and dependency graph.

Diff Detail

Repository
rB Blender
Branch
temp_localview_split
Build Status
Buildable 223
Build 223: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) retitled this revision from to Local View refactor (split off from layers).Aug 6 2016, 5:06 AM
Julian Eisel (Severin) updated this object.
Julian Eisel (Severin) updated this revision to Diff 7184.
Brecht Van Lommel (brecht) edited edge metadata.EditedAug 6 2016, 6:45 AM

What is your plan for merging this, somewhere after the 2.78 release I guess?

For linked objects to work and not lose their bits, shouldn't the local view bits be stored in the Base rather than Object? Though you don't have Base everywhere like the python API or ob->parent. So you'd have to duplicate the data again .. not sure what the solution is or if you had some plan for that for layers in general?

There's also a subtle change in the visibility logic I believe, was that intentional?

// new
if ((base->lay & v3d->lay) &&
    BKE_localview_is_object_visible(v3d, base->object))

// equivalent to old
if ((!v3d->localviewd  && (base->lay & v3d->lay)) ||
    (v3d->localviewd && BKE_localview_is_object_visible(v3d, base->object)))

While v3d->lay may not change while in local view, base->lay might?

source/blender/blenkernel/BKE_localview.h
63

Remove \ here and in following comments

75

Remove \

source/blender/editors/object/object_edit.c
146–147

I suggest to wrap this in a function (didn't check what the consistent function name would be):
if (view3d_is_object_visible(v3d, base)) {

For rendering you'd have something like:
if (render_is_object_visible(re, base)) {

And then use that consistently throughout the code.

For python addons, external renderers, etc, such utility functions may be the right thing to add in the RNA API as well, instead of wrapping any of the local view data structures.

source/blender/editors/object/object_relations.c
1369–1371

Why does this require deselecting? Is that an issue in the old system that still applies to the new one?

source/blender/editors/screen/screen_context.c
119–121

I see no reason for this to be a macro, can be a static function.

source/blender/editors/sculpt_paint/paint_image_proj.c
5209–5210

Convention is to not put { on a new line as far as I know, multiple places in the patch do not do this.

Just some quick answers re styling - patch itself I’ll try to find some time later this week.

source/blender/blenkernel/BKE_localview.h
63

Actually, \a is the 'emphasis' code in doxygen, we use it quite a bit everywhere in Blender code now to mark parameters and such in comments. ;)

source/blender/editors/sculpt_paint/paint_image_proj.c
5209–5210

This is actually correct style, we put { on next line in case of multi-line if/for/etc. statement (https://wiki.blender.org/index.php/Dev:Doc/Code_Style#Exceptions_to_Braces_on_Same_Line).

What is your plan for merging this, somewhere after the 2.78 release I guess?

Think this can be a goal for right after 2.78, yep.

For linked objects to work and not lose their bits, shouldn't the local view bits be stored in the Base rather than Object? Though you don't have Base everywhere like the python API or ob->parent. So you'd have to duplicate the data again .. not sure what the solution is or if you had some plan for that for layers in general?

Don't think we'd have an issue here? Only the Object is linked, and the Base is recreated (give_base_to_objects), so the local view bits are kept (worked in quick test).

There's also a subtle change in the visibility logic I believe, was that intentional?

// new
if ((base->lay & v3d->lay) &&
    BKE_localview_is_object_visible(v3d, base->object))
// equivalent to old
if ((!v3d->localviewd  && (base->lay & v3d->lay)) ||
    (v3d->localviewd && BKE_localview_is_object_visible(v3d, base->object)))

While v3d->lay may not change while in local view, base->lay might?

Hmm, right, it's possible that base->lay changes, but only from a 3D View that's not in local view (noticed a bug in master here, see rBe635f0413d85). Not sure how to solve this best, we could:

  • Show a popup to exit all local views if any local view is found (currently only done for current 3D View).
  • Temporarily change bit-fields of all local views to include the layer the object has been moved to.
  • Use proper visibility-checks, similar to what you suggested - Would prefer to have some utility macros/inline-functions for this then.
source/blender/editors/object/object_relations.c
1369–1371

Just copied what old code did, I actually have no idea why it's deselecting.

Julian Eisel (Severin) updated this revision to Diff 7200.
  • Cleanup: Leftover from macro->function conversion
  • Merge branch 'master' into temp_localview_split
  • Cleanup: Replace macro with function
  • Make compatibility work properly

Don't think we'd have an issue here? Only the Object is linked, and the Base is recreated (give_base_to_objects), so the local view bits are kept (worked in quick test).

What I'm thinking of is:

  • Link object from another .blend
  • Go into local view with this object
  • Save and reload
  • Linked object is no longer in local view (at least that's what I expect, did not test the patch)

Effectively you're editing a linked datablock and such edits will be lost. With layers this does not happen because Base.lay is the authoritative data source and part of the scene, not the object.

Hmm, right, it's possible that base->lay changes, but only from a 3D View that's not in local view (noticed a bug in master here, see rBe635f0413d85). Not sure how to solve this best, we could:

  • Show a popup to exit all local views if any local view is found (currently only done for current 3D View).
  • Temporarily change bit-fields of all local views to include the layer the object has been moved to.
  • Use proper visibility-checks, similar to what you suggested - Would prefer to have some utility macros/inline-functions for this then.

The last option seems like the right solution to me.

How would we deal with multiple viewports entering a local view?

Julian Eisel (Severin) updated this revision to Diff 7577.
  • Expose new local view in RNA
  • Expose object local view data in RNA too
  • Make Cycles work with new local view
  • Add/use BKE object visibility checks using correct logic
  • Move inline function defines into own source file

(Removed merge commits)

Re reloading linked objects: Eeek, this indeed doesn't work correctly. The only way to solve this right now would be doing the same syncing as done for Base.lay and Object.lay. AFAIK the planned override system is supposed to support save-able overrides for linked data-blocks though (right @Sergey Sharybin (sergey)?). This would be a proper solution so maybe it's not worth bothering with this (minor) issue now?

How would we deal with multiple viewports entering a local view?

What do you mean exactly? It's one of the core features of local view to have it work in multiple viewports, that's why it uses bit-flags.

Would like to work on deprecating layer bit-fields in layer manager branch, but I'm a bit afraid of hitting big merge conflicts once this is in. So was hoping we could get this merged soonish. Not sure if it should be applied on master or blender2.8 though. Guess if I rebase & apply on blender2.8, I don't have to care for old DepsGraph...

I think this could go into the blender2.8 branch? I don't think merging will be more of a problem than other big changes that have and will be done there.

Abandoning since a) lots of touched code has changed dramatically, and b) a new implementation of local view is worked on in D3973 .