On Wed, Aug 10, 2022 at 11:02:45AM +0200, Stein Gunnar Bakkeby wrote:
> Hi Hijito,
> I wasn't sure how big of a wall of text to put in the commit comment.
> Let me clarify this first.
> We have a block of code adjusting the client's starting position for
> floating windows where:
> Â Â - if the window position and size exceeds the right hand side of
> the monitor, then we move it so that it touches the monitor's right
> edge
> Â Â - if the window position and size exceeds the bottom of the
> monitor, then we move it so that it touches the monitor's bottom edge
> Â Â - if the window position exceeds the left hand side of the
> monitor, then we move it so that it touches the monitor's left edge
> Â Â - then we have the line in question
> Â Â /* only fix client y-offset, if the client center might cover the
> bar */
> Â Â c->y = MAX(c->y, ((c->mon->by == c->mon->my) && (c->x + (c->w /
> 2) >= c->mon->wx)
> Â Â Â Â && (c->x + (c->w / 2) < c->mon->wx + c->mon->ww)) ? bh :
> c->mon->my);
> This line sucks because one can't read it and go "oh, I see, that makes
> sense". The git history doesn't reveal why the line was added or the
> reasoning behind it.
> If we break it down we have:
> ifÂ
> Â Â (c->mon->by == c->mon->my)
> Â Â (the bar is at the top of the monitor and it is not hidden)
> and
> Â Â (c->x + (c->w / 2) >= c->mon->wx) &&
> Â Â (c->x + (c->w / 2) < c->mon->wx + c->mon->ww)
> Â Â (the window center on the x-axis is within the window area of the
> monitor)
> then
> Â Â use bh as the position
> else
> Â Â use my as the position
> Finally we end up with either:
> c->y = MAX(c->y, bh);
> or
> c->y = MAX(c->y, c->mon->my);
> The line comment I presume was added later in an attempt to explain
> what the line of code does to justify the complexity, but it doesn't
> actually represent what is happening. The client's position on the
> x-axis would not have anything to do with covering the bar - this is
> just misleading.
> The check to verify that the client center is within the window area is
> redundant given the two prior adjustments on the x-axis. The only
> exception to this would be if the window is so large that it spans
> three monitors, in which case the center of the window may be outside
> the window area of the client's monitor, and it is unlikely that this
> is the edge case that the line of code was trying to capture.
> In the event that the bar is at the top then we end up using bh as the
> constraint on the y-axis. This is wrong as the variable represents the
> height of the bar, it is not a position and it assumes that the
> monitor's my is 0.
> As-is this should have been c->mon->by + bh (or c->mon->my + bh). In
> practice this rarely causes any issues which is why this line of code
> has survived for this long.
> If need be this can be replicated in dual monitor setups where one
> monitor is placed further down than the other (e.g. one vertical
> monitor and one horizontal). Add a rule to make st windows floating and
> start st - the window will be placed partially or fully out of view
> when started on the lower monitor which will have a larger my value.
> Taking everything into account we can deduce that the intended
> behaviour is simply that if the window's y position is above the top
> edge of the monitor, then we move the window into view so that it
> touches the monitor's top edge - or below the bar if the bar is at the
> top (and is shown).
> To keep the same behaviour as present the statement can be simplified
> to:
> c->y = MAX(c->y, c->mon->wy);
> where the window area's wy position covers all the cases (bar placement
> and whether the bar is hidden).
> If there is agreement regarding that then I would also recommend that
> we use the window area variables (wx, ww, wy, wh) rather than the
> monitor area (mx, mw, my, mh) for the other three constraints for
> consistency:
> Â Â if (c->x + WIDTH(c) > c->mon->wx + c->mon->ww)
> Â Â Â Â c->x = c->mon->wx + c->mon->ww - WIDTH(c);
> Â Â if (c->y + HEIGHT(c) > c->mon->wy + c->mon->wh)
> Â Â Â Â c->y = c->mon->wy + c->mon->wh - HEIGHT(c);
> Â Â c->x = MAX(c->x, c->mon->wx);
> My reasoning for this is that I think the whole block should signal the
> intent that we want to have the window placed within the window
> area, rather than within the monitor.Â
> As-is if using a bottom bar then a window exceeding the bottom edge of
> the monitor is allowed to cover the bar.
> E.g. dwm with bottom bar and a rule to make st windows floating, try to
> run:
> Â Â st -g 50x20+0+9000
> Using window area variables would address that.
> Thanks,
> -Stein
> On Tue, Aug 9, 2022 at 11:38 PM Hiltjo Posthuma
> <[1]hiltjo_AT_codemadness.org> wrote:
>
> On Tue, Aug 09, 2022 at 10:38:08AM +0200, Stein wrote:
> > The reasoning behind the original line may be lost to time as
> > it does not make much sense checking the position on the x-axis
> > to determine how to position the client on the y-axis.
> >
> > In the context of multi-monitor setups the monitor y position
> > (m->my) may be greater than 0 (say 500), in which case the window
> > could be placed out of view if:
> >Â Â - the window attributes have a 0 value for the y position and
> >Â Â - we end up using the y position of bh (e.g. 22)
> >
> > If the aim is to avoid a new floating client covering the bar then
> > restricting y position to be at least that of the window area
> > (m->wy) should cover the two cases of using a top bar and using a
> > bottom bar.
> > ---
> >Â dwm.c | 4 +---
> >Â 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/dwm.c b/dwm.c
> > index 967c9e8..87d0ada 100644
> > --- a/dwm.c
> > +++ b/dwm.c
> > _AT_@ -1049,9 +1049,7 @@ manage(Window w, XWindowAttributes *wa)
> >Â Â Â Â if (c->y + HEIGHT(c) > c->mon->my + c->mon->mh)
> >Â Â Â Â Â Â Â Â c->y = c->mon->my + c->mon->mh - HEIGHT(c);
> >Â Â Â Â c->x = MAX(c->x, c->mon->mx);
> > -Â Â Â /* only fix client y-offset, if the client center might
> cover the bar */
> > -Â Â Â c->y = MAX(c->y, ((c->mon->by == c->mon->my) && (c->x +
> (c->w / 2) >= c->mon->wx)
> > -Â Â Â Â Â Â Â && (c->x + (c->w / 2) < c->mon->wx +
> c->mon->ww)) ? bh : c->mon->my);
> > +Â Â Â c->y = MAX(c->y, c->mon->wy);
> >Â Â Â Â c->bw = borderpx;
> >Â
> >Â Â Â Â wc.border_width = c->bw;
> > --
> > 2.37.1
> >
> >
> Hi,
> Sorry but this commit is much too vague. Doesn't this change the
> behaviour as
> described in the original comment ("client center")?
> Please provide a more clear description of the setup you have and
> the steps to
> reproduce them as it would help to understand the problem and the
> intended
> behaviour better.
> --
> Kind regards,
> Hiltjo
>
> References
>
> 1. mailto:hiltjo_AT_codemadness.org
Hi,
OK thanks for the explanation. I have pushed the patch.
If people find issues in their (multi-monitor) setups please let me know.
--
Kind regards,
Hiltjo
Received on Wed Aug 10 2022 - 15:33:05 CEST