Page MenuHome
Differential D315

Use tabs for image editor.
ClosedPublic
Actions

Authored by Antonis Ryakiotakis (psy-fi) on Feb 13 2014, 6:54 PM.

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

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 13 2014, 7:29 PM
  • Do not update Scopes unless the Scopes category is active.
Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 13 2014, 7:38 PM
  • 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
Brecht Van Lommel (brecht) requested changes to this revision.Feb 16 2014, 5:25 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
829–830

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.

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 9:07 AM
  • 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
Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 10:09 AM
  • 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

Sergey Sharybin (sergey) requested changes to this revision.Feb 19 2014, 10:31 AM

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.

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 11:09 AM
  • Arrange some mask panels similarly to clip editor.
  • Use different version patch (not sure 100% which of the two is correct)
Antonis Ryakiotakis (psy-fi) added a comment.Feb 19 2014, 11:18 AM

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.

Sergey Sharybin (sergey) requested changes to this revision.Feb 19 2014, 11:20 AM

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.

Thomas Dinges (dingto) requested changes to this revision.Feb 19 2014, 12:30 PM

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.

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 12:43 PM
  • Change naming of functions.
Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 1:18 PM

Mask and grease pencil tidy-up

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 1:25 PM
  • Fix versioning and better alignment, by Sergey. Thanks!
Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 1:37 PM
  • Port all of sergey's fix for previous commit, looks like my version only
Antonis Ryakiotakis (psy-fi) added a comment.Feb 19 2014, 1:42 PM

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

Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 2:08 PM
  • Add Tab for UV tools that only contains transform panel for now.
Antonis Ryakiotakis (psy-fi) updated this revision to Unknown Object (????).Feb 19 2014, 2:29 PM
  • Separate common grease pencil panel to own reusable file
Antonis Ryakiotakis (psy-fi) added a comment.Feb 19 2014, 2:32 PM

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?

Brecht Van Lommel (brecht) accepted this revision.Feb 19 2014, 3:51 PM

Looks good to me and works well in my tests.

Adding UV tools can indeed be done separately.

Antonis Ryakiotakis (psy-fi) closed this revision.Feb 19 2014, 4:22 PM

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

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
30 lines
source/
blender/
editors/
screen/
3 lines
space_image/
2 lines
25 lines

Diff 998

View Options

release/scripts/startup/bl_ui/space_image.py

Loading...
View Options

source/blender/editors/screen/area.c

Loading...
View Options

source/blender/editors/space_image/image_buttons.c

Loading...
View Options

source/blender/editors/space_image/space_image.c

Loading...