Page MenuHome

[Scr-ops] Join operator
Needs ReviewPublic

Authored by Ilya Shurupov (Hto-Ya) on Jun 5 2020, 6:32 PM.
Tokens
"Love" token, awarded by phocomelus."Love" token, awarded by pablovazquez."Love" token, awarded by HEYPictures."Love" token, awarded by Alaska."Love" token, awarded by roman13."Love" token, awarded by ofuscado."Love" token, awarded by jc4d."Like" token, awarded by duarteframos.

Details

Reviewers
Hans Goudey (HooglyBoogly)
Group Reviewers
User Interface
Summary

It is all about user interaction with the operator or "How user should tell to join op what to do"
It allows you to quickly join areas.


Original topic: https://devtalk.blender.org/t/proposal-resize-join-area-operators/12695

Diff Detail

Event Timeline

Ilya Shurupov (Hto-Ya) requested review of this revision.Jun 5 2020, 6:32 PM
Ilya Shurupov (Hto-Ya) created this revision.
Ilya Shurupov (Hto-Ya) created this object with edit policy "Administrators".
Ilya Shurupov (Hto-Ya) edited the summary of this revision. (Show Details)Jun 5 2020, 6:39 PM
Sybren A. Stüvel (sybren) changed the edit policy from "Administrators" to "All Users".

Actually I'm happy to review the basics here, and @Harley Acheson (harley) is a great person to look at this since he has been working in this area recently (as far as I know).

I will definitely take a look at this soon. This seems interesting, and so nice to see someone else playing in there.

This is starting with a single area and then finding neighbors. screen_geom_find_area_whith_common_edge() assumes that a joinable neighbor completely shares an edge. But we are currently able to join areas that don't line up exactly (and differ up to AREAJOINTOLERANCE) . So screen_geom_find_area_whith_common_edge() could be altered to do similar to area_getorientation() and find those areas too. If you did so I think you would find even cooler to play with. For example, current master (2.90) can merge the areas LEFT and RIGHT for the following, not just the obvious up and down:

I'm a little curious about the four lines of changes shown to screen_area_join(). What is that doing for you?

I am currently experimenting in that same area, specifically with the joining of areas that are wildly different from each other. For example, joining to either of two equal adjoining neighbors. If I am successful with those experiments it would conflict with this in the assumption that there is only one neighbor in a single direction. But perhaps then position of the pointer could also be used in that evaluation. So drag right and it shows the one neighbor that matches best, drag down a bit and highlight the secondary neighbor below. But again, that would only matter if I have some success and should know more within the week.

Changed the key events that trigger the operator to apply or cancel. Also, general cases when the cursor leaves the area improved.

"So screen_geom_find_area_whith_common_edge() could be altered to do similar to area_getorientation()..." - Yeah, I think it would be awesome. I definitely will try it later.

I'm so not sure about those lines:
In short, the totrct seems to be not updated after a call to area_join(), I don't know if it's normal, but when I want to immediately find the active area after that it is not possible because BKE_screen_find_area_xy() using totrct.
Finding active area after applying it's crucial for cases like "cascade joining"

@Ilya Shurupov (Hto-Ya) - In short, the totrct seems to be not updated after a call to area_join(), I don't know if it's normal, but when I want to immediately find the active area after that it is not possible because BKE_screen_find_area_xy() using totrct. Finding active area after applying it's crucial for cases like "cascade joining"

Cool, thanks! Will keep that in mind. The cascading joins look pretty sweet. Although it seems to makes assumptions on how the two areas are positioned. Not sure, will take a look.

Okay, sorry for jumping in here without looking at your code much, but didn't want you to take my earlier advice and change screen_geom_find_area_whith_common_edge() so that it accepts areas that are out of alignment. I don't think that is necessary. I think that a better strategy would not need screen_geom_find_area_whith_common_edge at all.

If you imagine your mouse in an area when your operator starts. It is near the middle horizontally, but vertically it is closer to the top than the bottom. Then you move right.

At that point you could just examine the rect of the area you are in. You just need to choose a point that is just to the right of the area's right edge at the same y position. Then you get that neighbor by calling BKE_screen_find_area_xy. Now you have a source area and a potential target area. Then use area_getorientation() as if it were actually named "CanTheseAreasJoin?" because it works exactly like that, returning -1 if not possible.

Why I specifically said to use that mouse y position means that if the code is ever improved to allow even more areas to be joined your operator would get the benefit of that too. So in this example, moving your mouse lower might select a different neighbor that can, or cannot, be merged.

Ilya Shurupov (Hto-Ya) updated this revision to Diff 25573.EditedJun 6 2020, 10:03 AM

Now op can merge not obvious cases, thanks to the @Harley Acheson (harley) great idea

There is a refactor, for more info see comments bellow https://developer.blender.org/D7949#192723

Update:
That patch will not be affected by any changes of implementation functions like apply or getorientation (the second one is testing if areas can join ), that patch is not even about that, that's about how the user interacts with the operator.
So I think it's clear then that it should remain as a separate topic/patch.
And then it's ready for review.

Ilya Shurupov (Hto-Ya) edited the summary of this revision. (Show Details)Jun 9 2020, 9:02 AM
Ilya Shurupov (Hto-Ya) updated this revision to Diff 26225.EditedJun 24 2020, 3:27 PM
Ilya Shurupov (Hto-Ya) edited the summary of this revision. (Show Details)

refactor

Ilya Shurupov (Hto-Ya) retitled this revision from Join Area Operator to [Scr-ops] Join operator.Jun 24 2020, 11:51 PM