Page MenuHome

UI: Area Joining and Closing
Needs ReviewPublic

Authored by Harley Acheson (harley) on Sat, Jun 20, 7:18 PM.
Tokens
"Love" token, awarded by radi0n."Love" token, awarded by brilliant_ape."Grey Medal" token, awarded by girafic."Love" token, awarded by mswf."Burninate" token, awarded by Hto-Ya."Grey Medal" token, awarded by duarteframos."Love" token, awarded by erickblender."Love" token, awarded by monio."Love" token, awarded by kioku.

Details

Summary

Until fairly recently, joining screen areas required that the edges of those areas line up exactly (as seen in the left of the following image). But that requirement has been relaxed somewhat over the last year and we can now join areas that are bit misaligned (as shown in the second image). This patch allows joining areas that are misaligned as much as the third image:

Basically any areas that can be joined by simply moving existing edges around, without wrecking or dramatically changing neighbors, will be successful. Additionally, some other common joins that require a bit more work under the hood will also work:

An animated GIF showing some of the new possibilities:

Because more neighboring areas can successfully join, it now (finally) means that all areas can be closed. So I added a SCREEN_OT_area_close operator and then fleshed out the "Header" context menu:

Details:

There has been lots of buzz lately about different ways of managing areas, but this patch does not compete or interfere with them but in fact complements them. This patch mainly adds the under-the-hood code to better join and to close areas: shared functions used by the visible UI code. Once we are able to do these things then we can explore different ways to initiate them.

This patch is very small and is designed to be easy to evaluate. You should just see a bunch of one-line changes were I need to get the "screen" to area_getorientation() so that it can use knowledge of other areas in its decisions. The code to allow misaligned areas mostly just adds one new 30-line function. Then a bit more code In order to support fancier joining. But you should find it minimal, logical, and well-commented.

Diff Detail

Repository
rB Blender

Event Timeline

Good work. But seems like it is only for special cases, is that right?

@Ilya Shurupov (Hto-Ya) - Good work. But seems like it is only for special cases, is that right?

Yes, I have made no claims here that this joins in all possible cases, just much more than now and better than now, and enough to allow all areas to close. My goal with this was to create something as simple as possible that was also easy to follow, easy to review and approve, allow for future improvement, and that could act as a baseline for "close" behavior.

However, I did keep your patches in mind while doing this. So you could later try to sell your idea of initiating area maintenance operations with right-clicks (with modifiers), or of improving the joining. But your "join anything" patch is of such complexity that I worry that it would be difficult to approve. So having this in place could make your experiments easier to achieve.

Adding a public function for closing areas - screen_area_close() - for use in the new operator or elsewhere.

Just tried the patch and it works great! Looks like there's no downside, right? Two things that may be outside the scope of this patch but worth mentioning:

  • Undo: Now that's easier to play with the areas, it's also easier to mess up our setup. I know we don't have undo for any layout operations (like switching editors), could merge/split editors be an exception? I'm only talking about operations in a single window (not undo on creating new windows or similar).
  • Preview: It's hard to predict the layout when joining two areas in an L or J shape. Do you think there's a way we could preview this? With an overlay or dimming the editors more than now.
Hans Goudey (HooglyBoogly) requested changes to this revision.EditedMon, Jun 22, 3:48 PM

I sort of agree with Pablo about undo, but I also think think this is probably not so commonly done by mistake. I think a better solution would be to improve the visualization of what will happen after the operation.

For example, it's a bit odd where the join arrow is drawn:

Code-wise it looks good, I appreciate that it's well commented.
Although it may be out of scope, for me a DIR_{UP, RIGHT, DOWN, LEFT} enum would make the code easier to follow. But I'm sure you get used to it after looking at 1,2,3,4 for a while.

source/blender/editors/screen/screen_edit.c
621

Use bool / true and false? (Same for screen_area_3wayjoin)

source/blender/editors/screen/screen_ops.c
1431

Return "true" and "false" here.

4197

I would prefer to consistently use capitals for these actions: "Flip Header to Bottom", etc..

This comment also applies to the operators below in this function.

This revision now requires changes to proceed.Mon, Jun 22, 3:48 PM

Updating to address some of the issues brought up by @Hans Goudey (HooglyBoogly)

  • screen_area_3wayjoin() now returning bool
  • area_close_poll() now properly returning bools
  • changes to capitalization in Header Tools menu

Stronger indication of the two areas involved in the join, as per suggestion by @Pablo Vazquez (pablovazquez)

@Hans Goudey (HooglyBoogly) - For example, it's a bit odd where the join arrow is drawn:

Yes, I have made the indication of the two areas stronger but those arrows could be aligned better.

Although it may be out of scope, for me a DIR_{UP, RIGHT, DOWN, LEFT} enum would make the code easier to follow. But I'm sure you get used to it after looking at 1,2,3,4 for a while.

Yes, how that section deals with directions and orientations is quite a mess. The ints used in place of bools, sometimes using an it for orientations, sometimes 'h' and 'v'. Directions using those ints. I'd love to refactor that, but it would touch a lot of code so I'd rather keep it separate from this. I'd probably approach it something like this?

typedef enum eDirection {
  DIR_INVALID = (1 << 0),
  DIR_N = (1 << 1),
  DIR_S = (1 << 2),
  DIR_E = (1 << 3),
  DIR_W = (1 << 4),
} eDirection;

typedef enum eOrientation {
  DIR_V = (DIR_N | DIR_S),
  DIR_H = (DIR_E | DIR_W),
} eDirection;

#define DIR_ANY = (DIR_N | DIR_S | DIR_E | DIR_W)

That would be a great improvement, but yeah, not as part of this patch.

Personally the more contrasty colors for the join arrow drawing aren't really doing it. It looks more flickery than before.
I would think dimming all other areas would help make it clearer which two are involved in the join. Then they wouldn't have to be dimmed so much.
Alternatively the arrow could move to the border between the two areas involved like I suggested above. I could see how that's not so straightforward in the code though.

source/blender/editors/screen/screen_draw.c
294

I know it's really picky, but I don't see any reason to change these comments from the doxygen style used elsewhere in the file.

306

Oops, It think you dropped UNUSED(dir)

@Hans Goudey (HooglyBoogly)
Personally the more contrasty colors for the join arrow drawing aren't really doing it. It looks more flickery than before.
I would think dimming all other areas would help make it clearer which two are involved in the join. Then they wouldn't have to be dimmed so much.
Alternatively the arrow could move to the border between the two areas involved like I suggested above. I could see how that's not so straightforward in the code though.

It wouldn't work to dim all except for the two areas to be merged because we also have to indicate which is the source and which will be removed. Because we allow the join to be reverse in the middle of the process.

Personally I don't see a solution that involves the current arrows at all, since this is a world where two areas can join at a 45 degree angle. In the following image you could drag from the top-right to the bottom-left. So no matter where you put the arrow in the bottom-left target it will look confusing.

Might be that we can only indicate the source and target, without a direction. So lighter for source, darker for target. Or source could be tinged green, the target red. Or Target could get an "X" in the middle to indicate it gets subsumed.

Harley Acheson (harley) updated this revision to Diff 26210.EditedTue, Jun 23, 4:59 PM

Updated to current state of master. And also, just to see how it feels...

  • Join "arrows" are gone since more than four directions are possible
  • Source area that remains is lightened - and greenish
  • Target area to be removed is darkened - and reddish

Another idea would be to place an arrow overlapping at the intersection center, angled by the area centers. Looks nice in the mockup, but I might be too lazy (and/or dumb) to do such a thing:

@Pablo Vazquez (pablovazquez) - It's hard to predict the layout when joining two areas in an L or J shape.

Part of the problem with that arrangement could be how I am moving the borders. In the following graphic you can see that I am favoring the item that moves. So regardless of where "A" goes the result is something that is wider that the existing split:

But I can instead align to the existing split. While testing it definitely results in something that is immediately readable, as it just feels like the existing split just extends without moving at all. Basically less changes so it is more understandable:

I can do this, even just to try, but requires a bit of changes.

That's a good point with those diagonal merges. It seems there's no great solution.

I wonder if you could somehow tag the join as "diagonal" and not draw the arrows in that case. IMO they worked pretty well for the joins that were possible before.

And about the red and green, it's fun, but I'm not sure it's more descriptive than the brighten / darken you had before and it's a bit "odd".

I think I just thought the white overlay was a bit strong, other than that it was probably fine. I'd be curious to hear other opinions though!

@Hans Goudey (HooglyBoogly) - ...the red and green, it's fun, but I'm not sure it's more descriptive than the brighten / darken you had before and it's a bit "odd".

Yes, I don't like it either, but wanted to experience for a while to make sure. I generally don't things that rely on color anyway - this case would screw up people with deuteranomaly and protanomaly.

That's a good point with those diagonal merges. It seems there's no great solution.

No, I think the perfect solution is the rotatable arrow between the areas. The only disadvantage is that someone else has to make it for me. LOL

3-way join now keeps position of the existing join. More changes to how source and target areas are indicated.

Harley Acheson (harley) edited the summary of this revision. (Show Details)Wed, Jun 24, 2:15 AM

Just some improvement/simplification of screen_areas_align()

Just changes to some comments.