window: fix overlapping detection of 50% title width

[rbuj]

Integers have no decimal part. Calculating 50% is equivalent to divide an integer by 2 (aka halving = value >> 1).

Test: showing the "Move TitleBar Onscreen" on window menu.

[vkareh]

@rbuj - how do you test this? what's the before/after and what issue is this fixing? it's not clear from the description :/

[rbuj]

Test: you have to move a window to the right, so the window almost comes out of the right margin of the desktop. Then click on window menu for displaying "Move Titlebar Onscreen" entry, as long as the overlap satisfies the constraint set in the if statement:

const int min_height_needed  = 8;
const int min_width_percent  = 0.5;
const int min_width_absolute = 50;

if (overlap.height > MIN (titlebar_rect.height, min_height_needed) &&
    overlap.width  > MIN (titlebar_rect.width * min_width_percent,
                          min_width_absolute))

min_width_percent is equal to 0, without this PR the if statement above can be rewritten as:

if (overlap.height > MIN (titlebar_rect.height, 8)) && (overlap.width  > 0))

However the overlay width must be greater than the minimum width (50px), or at least the half of the window if the width is lower than 100px.

MetaRectangle: src/include/boxes.h

typedef struct _MetaRectangle MetaRectangle;
struct _MetaRectangle
{
  int x;
  int y;
  int width;
  int height;
};

[raveit65]

Which window displays the "Move Titlebar Onscreen" entry? I don't get to display this entry :/

[vkareh]

I cannot get this thing either... maybe a better screenshot or a screencast can help?

[rbuj]

launch marco-window-demo, click on Windows menu, click on Utility menu item, drag Utility window to the right, click on apple icon.

[vkareh]

Oh man, this is a stretch - most windows cannot possibly get this, as the window needs to be really narrow, and then more than half of it needs to be outside of the viewport. So, now I can see the menu entry thanks to this PR, however, clicking it just shifts the window by a few pixels to the right, so hiding it even more - isn't this supposed to move it to the left? Also, can the threshold be increased? I expect something like this to work with any window that has more than half of it and fewer than, say, 100px, beyond the edge. As it stands, moving a window to show less than 50px is impossible unless the window itself is smaller than that...

[rbuj]

RELATED: metacity merged https://github.com/GNOME/metacity/commit/c920a889b71359a576c0d33b8df160b4a980b370 3 hours ago

[muktupavels]

That logic seems broken to me...

MIN (titlebar_rect.width * min_width_percent, min_width_absolute) This just does not make any sense to me! Window titlebar is probably always wider then min_width_absolute (50). Same with height check, I doubt there is cases when titlebar is less then 8px. So check basically can be simplified to (overlap.height > min_height_needed && overlap.width > min_width_absolute) meaning that titlebar is onscreen basically always unless it is moved almost completely out of screen.

Perhaps it was meant that at least half of titlebar width must be on screen to be considered on screen with requirement that at least 50px of titlebar is on screen in some weird cases?

[rbuj]

It can also be tested with the gimp toolbox on not single-window mode.

[raveit65]

Sorry, i don't get to mange to display this menu entry. I tried with dialog window from marco-window-demo or with gimp. Maybe with HIDPI it is different? So this PR needs to be reviewed by another person.