Page MenuHome

Synchronizing editors between workspaces
Needs RevisionPublic

Authored by Michael Soluyanov (crantisz) on Mar 29 2020, 12:45 PM.
Tokens
"Like" token, awarded by julperado."Like" token, awarded by BulatKR."Like" token, awarded by Biaru."Love" token, awarded by RedMser."Love" token, awarded by brilliant_ape."Like" token, awarded by MJunk."Love" token, awarded by kioku."Love" token, awarded by Tetone."Love" token, awarded by pablovazquez."Love" token, awarded by Zuorion.

Details

Summary

This is my second attempt to synchronize editors between workspaces. For start synchronizing, two (or more) editors with same type on different workspaces should be marked with command Synchronize area.

To make this possible, the desired areas are marked with slots that determine which area in one workspace corresponds to the area in another workspace. Slots selected automatically, based on editor type and order. This design works well for 3D-view, 2D-editors. Only Outliner not supported

Demo, how it works:

Diff Detail

Event Timeline

Nice implementation! I like the simple way you set this, and the fact it works for multiple editor types.

This revision is now accepted and ready to land.Mar 29 2020, 1:11 PM
dupoxy added a subscriber: dupoxy.Mar 29 2020, 1:49 PM
Brecht Van Lommel (brecht) requested changes to this revision.Mar 29 2020, 1:58 PM

This is not a full review, just quick notes from browsing the code.

release/scripts/startup/bl_ui/space_info.py
96

Use elif.

96–99

I think simple (2), (3) will work better, this is wrong for slots >= 3. (And it's also 3rd rather than 3-rd).

I think this should be a boolean property rather than an operator. With a regular checkbox, since the custom icon does not communicate on/off state as clearly.

source/blender/editors/screen/screen_ops.c
4125 ↗(On Diff #23244)

Please use clang-format.

4162 ↗(On Diff #23244)

Remove debug prints.

4203 ↗(On Diff #23244)

Use title case

4205 ↗(On Diff #23244)

Provide a proper description that explains what this feature does.

source/blender/editors/screen/workspace_edit.c
140–152

Put all area syncing logic together in the same file, not spread out across two C files.

179

Doing it only one workspace changes means it will not work when multiple workspace area opened in different windows at the same time. That's harder to support though.

182

Remove debug prints.

183–191

Deduplicate code with synchronize_area_from_slot.

source/blender/makesrna/intern/rna_screen.c
242

This is only one of multiple places that change butspacetype and butspacetype_subtype. If it's needed here it's needed everywhere.

However, it's not clear to me this is the right place to store the slot. Shouldn't it be in the space, so that the settings is not lost when you temporary change to another space and go back?

385

sinc -> sync, here and in other places.

388

Add a proper description that explains what this feature does.

This revision now requires changes to proceed.Mar 29 2020, 1:58 PM
source/blender/editors/screen/workspace_edit.c
183–191

This one do different job. It copies current area to all workspaces, instead coping state from outer workspaces

source/blender/makesrna/intern/rna_screen.c
242

I think it's correct. Synchronization lost, then you change editor type manually. But If it changes by temporary window (like Render Image Editor, or File Browser) it must be saved, even editor type changes

Michael Soluyanov (crantisz) marked 9 inline comments as done.

Converted operator to prop. Removed debug. Improved descriptions.

Note that part of the purpose of workspaces was specifically to allow, different viewport settings based on the workflow, giving a quick way to switch between them.

I think it would be best if most viewport settings were kept as-is when switching viewports, only syncing the viewport position/orientation (perhaps a few other settings).


There are many more design decisions to be made here, just noting that copying 3D view shading is something I think conflicts with the current design.

Note that part of the purpose of workspaces was specifically to allow, different viewport settings based on the workflow, giving a quick way to switch between them.
I think it would be best if most viewport settings were kept, syncing the viewport position/orientation could be useful though.

While I do understand this conflict, viewport shading is actually one of the settings I was eager to be able to synchronize, given it's complexity and number of options, manually copying between viewports can be quite a chore.
Being able to sync it was actually one of the goals I was hoping for. I can totally see why it goes against the current design principles however, because technically the shading mode is tied to the task, but that is not always the case.

I understand however there are different workflows, and that may not te the case for everyone else. Maybe that us just my personal preference, but I'm genuinely interested to hear what other people feel about this.

There could always be more fine grained options if needed.

Note, that this design covers all types of editors (Image, Node, Dopesheets, ets). I suggest add an option for Shading Mode in Workspace, as it was for Object Mode.
But in reality I'm not like changing Shading between Workspaces, because of computing time.

Julian Eisel (Severin) requested changes to this revision.Mar 31 2020, 2:04 PM

I like how much simpler this version is to use. But I still think this needs more design work. The point raised by @Campbell Barton (campbellbarton) are very valid, just like the one from @Duarte Farrajota Ramos (duarteframos). The current implementations are also still limited to changing workspaces, multi-window setups are still not supported and we have to figure out how that would work.

So if you plan to continue work on this, I'd still suggest to open a design task.
Setting to Changes Requested because these things should be discussed before further review.

This revision now requires changes to proceed.Mar 31 2020, 2:04 PM

I'd still suggest to open a design task.

How to do this?

Couple of ways, maybe try clicking Maniphest on the left bar in the home page, then Create Task in the upper right corner and Create Design:


I'm not sure if this actually creates a design task or just a type-less task. Some recent changes were made to forbid anyone but module owners from setting the type themselves (mainly for the bug tracker) but maybe this specific button still works.

For me the most important feature would be just viewport navigation syncing, which we should enable in the default workspaces if it's available.

Syncing all settings is not something I have a particularly strong opinion about. I can see how it's useful to share an editor between workspaces this way. I don't think it's really against the design in that you can have two workspaces for the same task, or workspaces with some overlap between the tasks.

release/scripts/startup/bl_ui/space_info.py
99

Is the (Synchronized) part of the label necessary, doesn't the checkbox communicate that?

source/blender/editors/screen/workspace_edit.c
157

sinc -> sync

158–171

This could should be in area.c along with the other synchronization functions.

161

Remove debug print.

Thanks much @Michael Soluyanov (crantisz) for keeping improving it. Personally I really like this version, it is much easier to handle than the previous one. Concerning on what should be included in the syncing I am personally rather agreeing with @Brecht Van Lommel (brecht), that syncing the transforms between the viewports is the most important thing.

A nice additional feature I had in mind for this is to be able to limit which components are respected from from the synced transform and which ones are custom/set manually. We could have turntable views by just taking the heading from the rotation, or zoomed out top views that follow the position of the 3dview camera in larger scenes. To keep it simpler these partial sync areas should not influence the synced values themselves.

For all other settings like shading I am also not sure if these have to be included in this sync mode. Having shading sync mandatory also has drawbacks so it would have to be optional to suit all needs.
I can see what @Duarte Farrajota Ramos (duarteframos) means and agree that these should be easier transferable, but is this a thing that needs to be synced permanently like the transform?

To me a copy and paste system for such viewport/area settings would also fulfill all needs. A "Copy Area Settings" and "Paste Area Settings", that collects all relevant info from that area type and stores it for transfer. It could even have an advanced paste option that presents some checkboxes in a dialog to choose which settings should be pasted to the target.

source/blender/makesdna/DNA_screen_types.h
356–357

Would prefer the term "sync", it's a common term (used over 400 times in our source/ dir), the name could also include what it synchronizes.

Examples: area_data_sync, screen_switch_sync, viewport_sync.

Love the concept!

Agree it would be better to split into at least two entries:

  • Area -> Sync View
  • Area -> Sync Settings

Also, call it Sync/Synced instead of Synchronize/Synchronized.