Re: [RP] [PATCH] Fix: exclude degenerate case from frame overlap

[Daniel Maturana]

I realize it has been over a year, but I can reproduce this bug and
confirm the patch fixes the issue for me. I also found a different
edge case not handled in these functions, reported in a different
email.
best,
Daniel

On Mon, Feb 5, 2018 at 9:44 PM Antoine Busque <address@hidden> wrote:
>
> The current implementation of the find_frame_* functions considered
> the degenerate case of frame overlap (i.e. a single pixel overlap, or
> in other words, one frame beginning where the other ends) as valid.
>
> This led to issues where certain frames would be impossible to reach
> via focus* commands, since another frame with single pixel overlap
> would be focused onto instead.
>
> An easy way to reproduce such a case is to split one screen into 4
> frames, by doing one vertical split and then two horizontal splits, or
> vice versa. In the current implementation, the bottom-right frame is
> unreachable via directional focus commands, since either the top-right
> (when trying to focusright from the bottom-left) or bottom-left (when
> trying to focusdown from the top-right) frames are focused onto, due
> to a single pixel overlap.
>
> Therefore, this patch excludes the degenerate case from being
> considered as a valid frame overlap.
>
> Signed-off-by: Antoine Busque <address@hidden>
> ---
>  src/split.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/src/split.c b/src/split.c
> index eed0f50..1c7d8b5 100644
> --- a/src/split.c
> +++ b/src/split.c
> @@ -1000,6 +1000,28 @@ show_frame_message (char *msg)
>    sbuf_free (msgbuf);
>  }
>
> +static int
> +frames_overlap_horizontal (rp_frame *f1, rp_frame *f2)
> +{
> +  int f1_right = frame_right_abs (f1);
> +  int f2_right = frame_right_abs (f2);
> +  int f1_left = frame_left_abs (f1);
> +  int f2_left = frame_left_abs (f2);
> +
> +  return f1_left < f2_right && f2_left < f1_right;
> +}
> +
> +static int
> +frames_overlap_vertical (rp_frame *f1, rp_frame *f2)
> +{
> +  int f1_bottom = frame_bottom_abs (f1);
> +  int f2_bottom = frame_bottom_abs (f2);
> +  int f1_top = frame_top_abs (f1);
> +  int f2_top = frame_top_abs (f2);
> +
> +  return f1_top < f2_bottom && f2_top < f1_bottom;
> +}
> +
>  rp_frame *
>  find_frame_up (rp_frame *frame)
>  {
> @@ -1011,7 +1033,7 @@ find_frame_up (rp_frame *frame)
>        list_for_each_entry (cur, &s->frames, node)
>          {
>            if (frame_top_abs (frame) == frame_bottom_abs (cur))
> -            if (frame_right_abs (frame) >= frame_left_abs (cur) && 
> frame_left_abs (frame) <= frame_right_abs (cur))
> +            if (frames_overlap_horizontal (frame, cur))
>                return cur;
>          }
>      }
> @@ -1030,7 +1052,7 @@ find_frame_down (rp_frame *frame)
>        list_for_each_entry (cur, &s->frames, node)
>          {
>            if (frame_bottom_abs (frame) == frame_top_abs (cur))
> -            if (frame_right_abs (frame) >= frame_left_abs (cur) && 
> frame_left_abs (frame) <= frame_right_abs (cur))
> +            if (frames_overlap_horizontal (frame, cur))
>                return cur;
>          }
>      }
> @@ -1049,7 +1071,7 @@ find_frame_left (rp_frame *frame)
>        list_for_each_entry (cur, &s->frames, node)
>          {
>            if (frame_left_abs (frame) == frame_right_abs (cur))
> -            if (frame_bottom_abs (frame) >= frame_top_abs (cur) && 
> frame_top_abs (frame) <= frame_bottom_abs (cur))
> +            if (frames_overlap_vertical (frame, cur))
>                return cur;
>          }
>      }
> @@ -1068,7 +1090,7 @@ find_frame_right (rp_frame *frame)
>        list_for_each_entry (cur, &s->frames, node)
>          {
>            if (frame_right_abs (frame) == frame_left_abs (cur))
> -            if (frame_bottom_abs (frame) >= frame_top_abs (cur) && 
> frame_top_abs (frame) <= frame_bottom_abs (cur))
> +            if (frames_overlap_vertical (frame, cur))
>                return cur;
>          }
>      }
> --
> 2.16.1
>
>
> _______________________________________________
> Ratpoison-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/ratpoison-devel