Page MenuHome

Fix T88500: Maximize button in the header is broken.
ClosedPublic

Authored by Pratik Borhade (PratikPB2123) on May 24 2021, 9:34 AM.

Details

Summary

Fix T88500.

Caused by rB4fb052f08dfd: Fix T88384: Improved Win32 Window Sizing and Positioning

Title bar disappears on windows after clicking Restore down button.
Adding extra line of code right after clamping the values to monitor border fixes the issue.

Diff Detail

Repository
rB Blender

Event Timeline

Pratik Borhade (PratikPB2123) requested review of this revision.May 24 2021, 9:34 AM
Pratik Borhade (PratikPB2123) created this revision.

Some background on this.

Windows 10, when creating windows, includes a hidden border (of about 7 pixels) to the left, right, and bottom (but not top). So in order to place a window at left or right edges, the params to CreateWindowExW() needs to include positions that are slightly outside of monitor bounds. In the past this caused issues because we clamped those values to monitor bounds.

My referenced commit improved our ability to size and position windows precisely by changing when bounds clamping is done. We now clamp our initial requested values by monitor bounds and then send those values to AdjustWindowRectEx() and have it supply us with the actual values to use with CreateWindowExW(). Basically we just ask the OS for the values to use and it calculates this based on chrome sizes, scaling, borders, etc. We tell it we want a left at 0 and it tells us to use -7 instead.

But... it looks like this is TOO flexible in one way and results in this error. The current code allows us to create and place windows that are too high. We ask for a vertical position of 0 and the OS says "no problem, just use -31" and we do so. There isn't any circumstance where we want this. That is why this patch just adds a single clamp to the top position, ensuring it is not less than the top of the working area of the monitor (zero in simple cases).

@Ray Molenkamp (LazyDodo) - Does this make sense? I'll add a comment before this change explaining why just that one clamp is needed there.

LGTM! Tested it could repro the issue, fix solves it

This revision is now accepted and ready to land.May 24 2021, 7:19 PM

will land with an added comment in there.