Details

Reviewers

Campbell Barton (campbellbarton)
Brecht Van Lommel (brecht)
Jonathan Williamson (carter2422)
Thomas Dinges (dingto)
Sergey Sharybin (sergey)

Maniphest Tasks

T38371: Clean up the image editor

Commits

rBSeb7485389b8a: Use tabs for image editor.
rBACeb7485389b8a: Use tabs for image editor.
rBeb7485389b8a: Use tabs for image editor.

Summary

For initial discussion see T38371

There is one point in the code where I feel uncomfortable. Image editor uses
PREVIEW type sidebars while tabs are only defined for TOOLS type. I did the easy
thing of adding PREVIEW to tabified sidebars but maybe correct solution is to
switch image editor to TOOLS type?

Diff Detail

Branch: image_editor_tabs

Event Timeline

Do not update Scopes unless the Scopes category is active.

Change the call to account for pinning. Thanks to DingTo for catching

Sergey Sharybin (sergey) added a reviewer: Sergey Sharybin (sergey).Feb 16 2014, 4:17 PM

I think we should indeed have a version patch to change it so that we have a TOOLS and PROPERTIES region here instead of a PREVIEW.

Further the tools should be on the left and the properties on the right, they are still flipped here.

I would also change the order of tabs so it's Paint | Scopes | Grease Pencil or Mask Tools | Scopes | Grease Pencil.

In the 3D view the Paint tab is called Tools. Can we make this consistent somehow?

source/blender/editors/space_image/space_image.c
831–832	I guess we'll have to live with that loose coupling, I don't think it's so bad. The other solution would be to compute the scopes just in time before drawing, but I don't like that much either, since ideally the scopes should be computed as part of the dependency graph somehow for better threading.

Do not update Scopes unless the Scopes category is active.
Change the call to account for pinning. Thanks to DingTo for catching
Change PREVIEW region to TOOLS region, similar to 3D viewport

More changes:

Move scopes after paint and mask tools
Move Grease pencil to buttons area
Rename Mask Tools to Mask
Rename *_scope functions to *_tools

My main concern is that both mask tooks and properties are in the tools panel. Would rather split this in the same way as it's done in clip editor.

Arrange some mask panels similarly to clip editor.
Use different version patch (not sure 100% which of the two is correct)

I think most points of review have been addressed, could you recheck please?

Remarks:

I used Tools and UI (not tool -operator?- properties) areas, similar to 3D viewport
Grease Pencil is moved to UI area (left) similar to other spaces. On spaces where it is in the tools area, only the operators reside there.
There is in fact a depsgraph update for scopes which unsets scopes.OK variable. This might do what we want but scopes_update() is called even when scopes.OK is true. I am not familiar with this code but may be possible to include the scopes_update call to only happen on scopes invalidation from depsgraph?

I am getting some memory leak on exit which must be related to scopes which I can't pinpoint. I think it may be old (I have had leaks previously with scopes on) but I am only getting "data from ID_SCR" as a guardedalloc report.

I'm pretty happy with the order now. Probably could be tweaked further based on feedback or so, but consider it's rather ready for testbuild.

One thing tho, versioning is still a bit wrong. Here's the file which demonstrates the issue

image-editor.blend472 KBDownload

Simply change 3d viewport to image editor and hit N,T.

I applied the last patch and I can't see any tabs? I used the Raw Download here, Blender shows "2.69.11" and git diff reports the changes too?

Also it is still called "Scopes" (View menu), IMAGE_OT_scopes and the corresponding functions should be renamed too.

Change naming of functions.

Mask and grease pencil tidy-up

Fix versioning and better alignment, by Sergey. Thanks!

Port all of sergey's fix for previous commit, looks like my version only

bah stupid arc..."looks like my version only accounted for inactive areas" is the correct here.

Add Tab for UV tools that only contains transform panel for now.

Separate common grease pencil panel to own reusable file

Ready here, awaiting review.

Only issue is what uv tools to add to Tools tab (already have nice parent class to inherit from for panels that want to do that). But maybe that should be a new task?

Looks good to me and works well in my tests.

Adding UV tools can indeed be done separately.

Closed by commit rBeb7485389b8a (authored by @Antonis Ryakiotakis (psy-fi)).

Use tabs for image editor.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 996

release/scripts/startup/bl_ui/space_image.py

source/blender/editors/screen/area.c

source/blender/editors/space_image/image_buttons.c

source/blender/editors/space_image/space_image.c

Use tabs for image editor.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 996

release/scripts/startup/bl_ui/space_image.py

source/blender/editors/screen/area.c

source/blender/editors/space_image/image_buttons.c

source/blender/editors/space_image/space_image.c

Use tabs for image editor.
ClosedPublic
Actions

Revision Contents
Changeset List