Paths

Table of Contentst

Local View refactor (split off from layers)
AbandonedPublic
Actions

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

Details

Reviewers

Bastien Montagne (mont29)
Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

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) updated this revision to Diff 7184.Aug 6 2016, 5:06 AM

Julian Eisel (Severin) retitled this revision from to Local View refactor (split off from layers).

Julian Eisel (Severin) added reviewers: Brecht Van Lommel (brecht), Sergey Sharybin (sergey), Bastien Montagne (mont29).

Julian Eisel (Severin) updated this object.

Julian Eisel (Severin) updated this object.Aug 6 2016, 5:56 AM

Julian Eisel (Severin) updated this object.Aug 6 2016, 6:14 AM

Julian Eisel (Severin) updated this object.Aug 6 2016, 6:35 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.

Cleanup: Leftover from macro->function conversion
Merge branch 'master' into temp_localview_split
Cleanup: Replace macro with function
Make compatibility work properly

In D2138#49723, @Julian Eisel (Severin) wrote:

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?

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?

In D2138#49759, @Sergey Sharybin (sergey) wrote:

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.

Julian Eisel (Severin) updated this object.Nov 22 2016, 11:12 AM

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

Revision Contents
Changeset List

Path

Size

intern/

cycles/

blender/

18 lines

3 lines

7 lines

28 lines

source/

blender/

blenkernel/

49 lines

30 lines

8 lines

2 lines

intern/

localview.c

67 lines

screen.c

32 lines

sequencer.c

2 lines

blenloader/

intern/

readfile.c

43 lines

versioning_270.c

65 lines

writefile.c

8 lines

editors/

gpencil/

gpencil_convert.c

5 lines

gpencil_utils.c

1 line

mesh/

editmesh_tools.c

2 lines

object/

18 lines

4 lines

18 lines

38 lines

1 line

render/

render_internal.c

33 lines

render_opengl.c

7 lines

screen/

screen_context.c

40 lines

screen_ops.c

8 lines

sculpt_paint/

paint_image_proj.c

3 lines

space_view3d/

4 lines

2 lines

167 lines

24 lines

2 lines

6 lines

238 lines

transform/

transform_manipulator.c

7 lines

transform_orientations.c

1 line

gpu/

GPU_draw.h

3 lines

intern/

gpu_draw.c

15 lines

makesdna/

5 lines

28 lines

40 lines

13 lines

makesrna/

intern/

rna_object.c

12 lines

rna_screen.c

15 lines

rna_space.c

22 lines

render/

extern/

include/

RE_pipeline.h

14 lines

intern/

include/

render_types.h

5 lines

renderdatabase.h

2 lines

source/

convertblender.c

74 lines

envmap.c

5 lines

pipeline.c

44 lines

blenderplayer/

bad_level_call_stubs/

stubs.c

2 lines

creator/

creator_args.c

4 lines

Commit	Tree	Parents	Author	Summary	Date
d37679ad7360	fe6976967c22	6bbdde1fd0fa	Julian Eisel	Move inline function defines into own source file	Oct 7 2016, 11:23 PM
6bbdde1fd0fa	f8dfc35ba2d1	b28b6111b251 0a2a00677554	Julian Eisel	Merge branch 'master' into temp_localview_split (Show More…)	Oct 7 2016, 11:11 PM
b28b6111b251	a76d7600a3a8	9d4501512aa2 e12f5b699d5a	Julian Eisel	Merge branch 'master' into temp_localview_split	Sep 20 2016, 12:48 AM
9d4501512aa2	1ca9adf85e53	1de2f534cf9d 45f833c21dcf	Julian Eisel	Merge branch 'master' into temp_localview_split (Show More…)	Sep 8 2016, 1:17 PM
1de2f534cf9d	851e28f492c1	a796cbf3072f	Julian Eisel	Add/use BKE object visibility checks using correct logic (Show More…)	Aug 10 2016, 2:59 AM
a796cbf3072f	503a770d31fe	50f413fca120	Julian Eisel	Make Cycles work with new local view	Aug 9 2016, 9:12 PM
50f413fca120	ff14ac08e09a	76c29fdd41db b53eca171158	Julian Eisel	Merge branch 'master' into temp_localview_split	Aug 9 2016, 4:38 AM
76c29fdd41db	55f4eea7586a	041ff44d7a0e	Julian Eisel	Expose object local view data in RNA too	Aug 8 2016, 12:52 AM
041ff44d7a0e	47ccf02bc6d4	df24bef79db0	Julian Eisel	Expose new local view in RNA	Aug 8 2016, 12:28 AM
df24bef79db0	5a935dac7d04	e80e9e756873	Julian Eisel	Make compatibility work properly	Aug 7 2016, 10:32 PM
e80e9e756873	8edc1dcbaf37	e31f57d217fa	Julian Eisel	Cleanup: Replace macro with function	Aug 7 2016, 10:32 PM
e31f57d217fa	605280047417	109f06bdf27a e635f0413d85	Julian Eisel	Merge branch 'master' into temp_localview_split (Show More…)	Aug 7 2016, 9:28 PM
109f06bdf27a	6417c025664b	4661cafc2b6d	Julian Eisel	Cleanup: Leftover from macro->function conversion	Aug 7 2016, 9:22 PM
4661cafc2b6d	3abb88f2a8f3	702bdcd9a67b	Julian Eisel	Adjust local view checks in base macros	Aug 6 2016, 5:02 AM
702bdcd9a67b	ad925aa7a4c7	14934299e057 734e0aca3872	Julian Eisel	Merge branch 'master' into temp_localview_split	Aug 6 2016, 4:26 AM
14934299e057	ef0dc67582dc	762e10bab228	Julian Eisel	Update comment	Aug 6 2016, 4:25 AM
762e10bab228	8c3a0877d8f4	0b964ad7f0f8	Julian Eisel	Fix viewport not updating layers when changing out of local view	Aug 6 2016, 3:40 AM
0b964ad7f0f8	38d797d8f451	aa6adab1fc9a	Julian Eisel	Some more tweaks for local view checks	Aug 6 2016, 3:18 AM
aa6adab1fc9a	41cbcae68e65	d522f558024a	Julian Eisel	Add security check for View3D.lay behavior	Aug 6 2016, 2:44 AM
d522f558024a	bfe35c8b4853	103b6aedf981	Julian Eisel	More checks for new local view	Aug 5 2016, 4:23 AM
103b6aedf981	590cf8288f33	eeff1f7fef9d	Julian Eisel	Use inline functions instead of macros for local view utilities	Aug 4 2016, 5:25 PM
eeff1f7fef9d	c4a8b7af4f8c	4b016fdc850c	Julian Eisel	Move local view macros into own file (BKE_localview.h)	Aug 4 2016, 4:52 PM
4b016fdc850c	3deb26ea9c37	cea68dd2a262	Julian Eisel	Mark old View3D.localvd as deprecated	Aug 4 2016, 5:16 AM
cea68dd2a262	207eb22c2712	a80c7c73e385	Julian Eisel	Remove old local view RNA property definitions	Aug 4 2016, 5:14 AM
a80c7c73e385	58bdbbf967b5	523c93159375 14720e2e6265	Julian Eisel	Merge branch 'master' into temp_localview_split (Show More…)	Aug 4 2016, 5:08 AM
523c93159375	951b36f502f8	1be415395053	Julian Eisel	Add more checks for new local view	Aug 4 2016, 4:00 AM
1be415395053	6a0a69a0439b	40c4b952874a	Julian Eisel	Use new local view system for viewport and View3D context iterators	Aug 3 2016, 12:44 AM
40c4b952874a	2b64a6f62c4b	0eb8a374249f	Julian Eisel	Make Blender Internal work with new local view design (Show More…)	Jul 28 2016, 6:20 PM
0eb8a374249f	8adaf27c4efb	2b8705fa001f	Julian Eisel	Increase max num of local views from 8 to 32	Jul 28 2016, 6:11 PM
2b8705fa001f	2af166286d6f	c59444f71bc4	Julian Eisel	Prepare local view OP to work without layer bitfields	Jul 28 2016, 5:18 PM
c59444f71bc4	f318f6a44f43	ba9c9a8c9621	Julian Eisel	Move LocalView structs to DNA_screen_types.h	Jul 28 2016, 3:58 AM
ba9c9a8c9621	adad4e633934	9ece78f6ef32	Julian Eisel	Use LocalLayerInfo struct for area visibility bits	Jul 28 2016, 3:48 AM
9ece78f6ef32	64958e483f66	ad53f65b2172	Julian Eisel	Fix do_versions using wrong region list	Jul 28 2016, 3:25 AM
ad53f65b2172	991a6614c3a5	48074c6522a1	Julian Eisel	Add/use utility macro for removing object from local view (Show More…)	Jul 27 2016, 7:08 PM
48074c6522a1	f8734ba4ff9f	ecee478ef86b	Julian Eisel	Make move to layer OP work with new local view data (Show More…)	Jul 27 2016, 6:55 PM
ecee478ef86b	9c6ca859ec2a	ddc8701c4027	Julian Eisel	Start using separate bitfield for local view visibility (Show More…)	Jul 27 2016, 5:28 PM
ddc8701c4027	d8d8d20bc080	3ae44ffa9a8f 24b8e781f6de	Julian Eisel	Merge branch 'master' into temp_localview_split	Jul 27 2016, 1:34 AM
3ae44ffa9a8f	8a18ba4517f9	57fe880fd23f	Julian Eisel	Compatibility with old local view storage (Show More…)	Jul 27 2016, 1:06 AM
57fe880fd23f	86a1d59e0130	4c6789b2371a	Julian Eisel	Mark RegionView3D.localvd as deprecated	Jul 27 2016, 12:01 AM
4c6789b2371a	9057515504b4	3a5eff95c59f	Julian Eisel	Initial commit for splitting off local-view data from layer visibility (Show More…)	Jul 26 2016, 11:35 PM