[PATCH] Fix: exclude degenerate case from frame overlap

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix: exclude degenerate case from frame overlap

Antoine Busque-2
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 <[hidden email]>
---
 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
[hidden email]
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel
Reply | Threaded
Open this post in threaded view
|

Re: [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 <[hidden email]> 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 <[hidden email]>
> ---
>  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
> [hidden email]
> https://lists.nongnu.org/mailman/listinfo/ratpoison-devel

_______________________________________________
Ratpoison-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel