Page MenuHome

Fix Regions Max Size bugs. And remove old code for Header Size.
ClosedPublic

Authored by George Vogiatzis (Gvgeo) on Mar 28 2019, 7:11 PM.

Details

Summary

FIX:

  1. Region max size is independent from dpi, and is possible to have bigger size that the tools size.

It will minimize when bigger, and will not open from tab click or drag up. To open it need to drag the tab[+] down to scale it down.

  1. When a region have a bigger size from the space it can fit will not appear with clicking on tab (will need to drag). This case can happen when shrinking the size of a space.
  2. File browser's tool's properties now have correct max size(can go up to top).

REMOVED: A piece of code about header size that have no use anymore.
And a part about header edge, that no longer needed.

Diff Detail

Repository
rB Blender

Event Timeline

Can you give specific steps to redo the issue this is fixing?

source/blender/editors/screen/area.c
989

No longer a problem, it is no longer possible to hide the header from edge.

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

Header size is coded in other place, this part makes no difference anymore.

set scale 2.
in a tool regions(eg. file browser), maximize the tools properties region up to top.
shrink the Y size of the file browser. so that the tool properties get minimized.
then try to open the tool properties from clicking the [+} tab or by dragging it up.
Will not appear, can restore only if you drag it down or get screen bigger again.

Also fixes the maximum size Y the region can get when scale is not 1.

George Vogiatzis (Gvgeo) planned changes to this revision.Mar 29 2019, 12:40 PM

While this patch fixes more regions, there is still a bug left, I want to fix together. Maybe I will separate the old code removal part into a different patch.

George Vogiatzis (Gvgeo) updated this revision to Diff 14487.

Added fix 2, 3.
Clean up more code with the this update.

George Vogiatzis (Gvgeo) marked 2 inline comments as not done.Mar 29 2019, 7:31 PM
George Vogiatzis (Gvgeo) added inline comments.
source/blender/editors/screen/area.c
989

region_azone_edge_poll

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

region_rect_recursive


Video with bugs 1 and 2 .

George Vogiatzis (Gvgeo) retitled this revision from Fix Tools Properties dpi bug. And remove old code for Header Size. to Fix Regions Max Size bugs. And remove old code for Header Size..Mar 29 2019, 8:15 PM
George Vogiatzis (Gvgeo) planned changes to this revision.Mar 30 2019, 3:15 PM

improved area_max_regionsize logic

I should mention a related bug, in case it is a 5 minute fix. But it is such a minuscule issue, that I don't believe opening a bug report worth it (you seem to have enough of these).

In file browser when clicking the tab to open the RGN_TYPE_TOOL_PROPS, if the click is not fast enough, in region_scale_modal along with the LEFTMOUSE case fires the MOUSEMOVE too.
It does not appear to do it consistently, but does not happen in other regions.
In a case where the region was hidden from shrinking the file browser size, the MOUSEMOVE will fire before LEFTMOUSE, causing the region taking the full size. The desired function is to leave a header size free.

Jacques Lucke (JacquesLucke) requested changes to this revision.Apr 2 2019, 1:48 PM

Seems to work in my test. I made some comments inline.

In general it would be nicer if you would not try to fix 3 bugs at the same time (that would make it easier to review for me, as I don't know the code either).

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

style

2296

This will break when another flag is added in the future.
Better use a mask to get the lower bits only and then do the ELEM check.

2323

style: use spaces to indent a second line of the condition.

This revision now requires changes to proceed.Apr 2 2019, 1:48 PM

It to me a while to understand the description, I think this would be more clear for the commit.

Fix odd behavior in region sizing, simplify code.

* When resizing sidebars, don't collapse when the region becomes too big but
  instead clamp the region size to the available space.
* Fix clicking the tab to expand sidebars no working if the sidebar is too
  wide to fit. Instead make it less wide so it does fit.
* Fix incorrect limit on tool properties region height, for example in the
  file browser.

Other than that the issues pointed out in code review this seems fine.

Moved dist calculation from rectangle, in the else statement.

Fixed style, added mask.

George Vogiatzis (Gvgeo) marked 3 inline comments as done.Apr 2 2019, 6:59 PM

Will commit in a moment.

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

hmm, this will still break when another flag at a higher bit is added.
I understand that this is a tricky issue due to the mix of flags with non-flags in the same variable.

I'll fix it myself.

This revision is now accepted and ready to land.Apr 3 2019, 2:41 PM
This revision was automatically updated to reflect the committed changes.

I totally forgot about the lower bits part you mention.

I tried to avoid doing this way and include something unknown.
I wanted to make sure it only had RGN_SPLIT_PREV flag.
I would prefer to stop working, than create a possible hidden bug in the future.

But I see that both of you agree on this.
Must be a 'try to include all cases' rule.