[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Double Window Borders



Thorsten WiÃmann:

> On Sun, Jul 22, 2012 at 11:06:19PM +0200, Bastien Dejean wrote:
> > Thorsten WiÃmann:
> > 
> > > There are some formal things I don't like about the current patch, the
> > > code itself works as expected.
> > 
> > I fixed the issues you mentioned and I also fixed a bug in
> > window_focus:
> > 
> > The window_unfocus function was called two times:
> > - First on the previous window (that's fine) .
> > - Then on the window to be focus (not good!).
> > Hence the border blinking.
> 
> That produced other problems:
> 
>   - When cycling through the windows using the cycle command the focus
>     cycles but no window border is marked as focused. It is caused by
>     that, because your version only resets the border color on the
>     second call of window_focus, and frame_current_cycle_selection()
>     only calls it once:

My bad: I misread you, this is not the bugfix (moving window_unfocus in
the 'if') that produced those effects, but the fact of putting the
window_update_border call into the 'else'.

I did that when I was trying to identify the bug and forgot to revert it
afterwards.

The fixed patch is attached.

-- 
 b.d
(| |)
 ^ ^
>From 527d0c1c12eb6522759626889038a2cf159c3156 Mon Sep 17 00:00:00 2001
From: Bastien Dejean <nihilhill _at_ gmail _dot_ com>
Date: Mon, 23 Jul 2012 14:38:50 +0200
Subject: [PATCH] New settings: double_window_border and window_border_inner_color

---
 BUGS                 |  2 --
 NEWS                 |  2 ++
 doc/herbstluftwm.txt |  9 ++++++++
 src/clientlist.c     | 59 +++++++++++++++++++++++++++++++++++++++++++---------
 src/clientlist.h     |  2 ++
 src/settings.c       |  4 +++-
 src/utils.c          | 38 +++++++++++++++++++++++++++++++++
 src/utils.h          |  5 +++++
 8 files changed, 108 insertions(+), 13 deletions(-)

diff --git a/BUGS b/BUGS
index 3c91bb1..4f54572 100644
--- a/BUGS
+++ b/BUGS
@@ -49,8 +49,6 @@ Planned features:
     - or: chain commands like: cmdchain 4 cmd1 arg1a arg1b arg1c cmd2 arg2a
       arg2b
     - alias commands: alias shortcmd = cmd default arg, alias shortcmd --delete
-    - double window border to give a good contrast to dark backgrounded and
-      light backgrounded windows.
     - provide multiple mechanisms to split clients to child frames:
          * 100:0 (and vice versa)
          * 50:50
diff --git a/NEWS b/NEWS
index b3f003e..dac2d5d 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,8 @@ Changes:
     * only one Makefile for herbstluftwm and herbstclient. The herbstclient
       binary now is put in the main directory.
     * new settings: smart_frame_surroundings and smart_window_surroundings
+    * new setting: double_window_border
+    * new setting: window_border_inner_color
     * new option --skip-invisible for cycle_all
 
 Release 0.3 on 2012-04-12
diff --git a/doc/herbstluftwm.txt b/doc/herbstluftwm.txt
index d7da1fb..9f59dd8 100644
--- a/doc/herbstluftwm.txt
+++ b/doc/herbstluftwm.txt
@@ -589,6 +589,9 @@ window_border_normal_color (String/Color)::
 window_border_urgent_color (String/Color)::
     Border color of an unfocused but urgent window.
 
+window_border_inner_color (String/Color)::
+    Color of the inner border of a window.
+
 always_show_frame (Integer)::
     If set, all frames are displayed. If unset, only frames with focus or with
     windows in it are displayed.
@@ -627,6 +630,12 @@ smart_window_surroundings (Integer)::
     If set, window borders and gaps will be removed when there's no ambiguity
     regarding the focused window.
 
+double_window_border (Integer)::
+    If set, one pixel inner window borders will be drawn in addition to the
+    regular window borders. It will only activate if window_border_width is
+    greater than one. The inner border's color can be set via
+    window_border_inner_color.
+
 focus_follows_shift (Integer)::
     If set, focus stays in the window, if window is shifted to another frame.
     If unset, focus stays in the frame.
diff --git a/src/clientlist.c b/src/clientlist.c
index b67be6f..c6abd81 100644
--- a/src/clientlist.c
+++ b/src/clientlist.c
@@ -34,9 +34,11 @@ int* g_window_border_width;
 int* g_raise_on_focus;
 int* g_snap_gap;
 int* g_smart_window_surroundings;
+int* g_double_window_border;
 unsigned long g_window_border_active_color;
-unsigned long g_window_border_urgent_color;
 unsigned long g_window_border_normal_color;
+unsigned long g_window_border_urgent_color;
+unsigned long g_window_border_inner_color;
 
 GHashTable* g_clients; // container of all clients
 
@@ -61,6 +63,7 @@ static void fetch_colors() {
     g_window_gap = &(settings_find("window_gap")->value.i);
     g_snap_gap = &(settings_find("snap_gap")->value.i);
     g_smart_window_surroundings = &(settings_find("smart_window_surroundings")->value.i);
+    g_double_window_border = &(settings_find("double_window_border")->value.i);
     g_raise_on_focus = &(settings_find("raise_on_focus")->value.i);
     char* str = settings_find("window_border_normal_color")->value.s;
     g_window_border_normal_color = getcolor(str);
@@ -68,6 +71,8 @@ static void fetch_colors() {
     g_window_border_active_color = getcolor(str);
     str = settings_find("window_border_urgent_color")->value.s;
     g_window_border_urgent_color = getcolor(str);
+    str = settings_find("window_border_inner_color")->value.s;
+    g_window_border_inner_color = getcolor(str);
 }
 
 void clientlist_init() {
@@ -217,11 +222,13 @@ void client_destroy(HSClient* client) {
 }
 
 void window_unfocus(Window window) {
-    XSetWindowBorder(g_display, window, g_window_border_normal_color);
+    HSDebug("window_unfocus NORMAL\n");
+    window_update_border(window, g_window_border_normal_color);
     HSClient* c = get_client_from_window(window);
     if (c) {
         if (c->urgent) {
-            XSetWindowBorder(g_display, window, g_window_border_urgent_color);
+            HSDebug("window_unfocus URGENT\n");
+            window_update_border(window, g_window_border_urgent_color);
         }
         grab_client_buttons(c, false);
     }
@@ -243,17 +250,17 @@ void window_unfocus_last() {
 }
 
 void window_focus(Window window) {
-    // unfocus last one
-    window_unfocus(lastfocus);
-    // change window-colors
-    XSetWindowBorder(g_display, window, g_window_border_active_color);
     // set keyboardfocus
     XSetInputFocus(g_display, window, RevertToPointerRoot, CurrentTime);
+
     if (window != lastfocus) {
         /* FIXME: this is a workaround because window_focus always is called
          * twice.  see BUGS for more information
          *
          * only emit the hook if the focus *really* changes */
+
+        // unfocus last one
+        window_unfocus(lastfocus);
         ewmh_update_active_window(window);
         HSClient* client = get_client_from_window(window);
         char* title = client ? client->title->str : "?";
@@ -261,6 +268,11 @@ void window_focus(Window window) {
         snprintf(winid_str, STRING_BUF_SIZE, "0x%x", (unsigned int)window);
         hook_emit_list("focus_changed", winid_str, title, NULL);
     }
+
+    // change window-colors
+    HSDebug("window_focus ACTIVE\n");
+    window_update_border(window, g_window_border_active_color);
+
     lastfocus = window;
     /* do some specials for the max layout */
     bool is_max_layout = frame_focused_window(g_cur_frame) == window
@@ -278,10 +290,11 @@ void client_setup_border(HSClient* client, bool focused) {
         g_window_border_active_color,
     };
     if (client->urgent) {
-        XSetWindowBorder(g_display, client->window,
-                         g_window_border_urgent_color);
+        HSDebug("client_setup_border URGENT\n");
+        window_update_border(client->window, g_window_border_urgent_color);
     } else {
-        XSetWindowBorder(g_display, client->window, colors[focused ? 1 : 0]);
+        HSDebug("client_setup_border %s\n", (focused ? "ACTIVE" : "NORMAL"));
+        window_update_border(client->window, colors[focused ? 1 : 0]);
     }
 }
 
@@ -353,6 +366,11 @@ void client_resize(HSClient* client, XRectangle rect, HSFrame* frame) {
 
     XSetWindowBorderWidth(g_display, win, border_width);
     XMoveResizeWindow(g_display, win, rect.x, rect.y, rect.width, rect.height);
+    if (*g_double_window_border) {
+        unsigned long current_border_color = get_window_border_color(client);
+        HSDebug("client_resize %s\n", current_border_color == g_window_border_active_color ? "ACTIVE" : "NORMAL");
+        set_window_double_border(win, g_window_border_inner_color, current_border_color);
+    }
     //// send new size to client
     //// WHY SHOULD I? -> faster? only one call?
     //XConfigureEvent ce;
@@ -411,6 +429,11 @@ void client_resize_floating(HSClient* client, HSMonitor* m) {
     XSetWindowBorderWidth(g_display, client->window, *g_window_border_width);
     XMoveResizeWindow(g_display, client->window,
         rect.x, rect.y, rect.width, rect.height);
+    if (*g_double_window_border) {
+        unsigned long current_border_color = get_window_border_color(client);
+        HSDebug("client_resize %s\n", current_border_color == g_window_border_active_color ? "ACTIVE" : "NORMAL");
+        set_window_double_border(client->window, g_window_border_inner_color, current_border_color);
+    }
 }
 
 XRectangle client_outer_floating_rect(HSClient* client) {
@@ -597,6 +620,22 @@ int client_set_property_command(int argc, char** argv) {
     return 0;
 }
 
+void window_update_border(Window window, unsigned long color) {
+    if (*g_double_window_border) {
+        set_window_double_border(window, g_window_border_inner_color, color);
+    } else {
+        XSetWindowBorder(g_display, window, color);
+    }
+}
+
+unsigned long get_window_border_color(HSClient* client) {
+    Window win = client->window;
+    unsigned long current_border_color = (win == frame_focused_window(g_cur_frame) ? g_window_border_active_color : g_window_border_normal_color);
+    if (client->urgent)
+        current_border_color = g_window_border_urgent_color;
+    return current_border_color;
+}
+
 static bool is_client_urgent(void* key, HSClient* client, void* data) {
     (void) key;
     (void) data;
diff --git a/src/clientlist.h b/src/clientlist.h
index b484aa2..65d9866 100644
--- a/src/clientlist.h
+++ b/src/clientlist.h
@@ -76,6 +76,8 @@ void window_show(Window win);
 void window_hide(Window win);
 void window_set_visible(Window win, bool visible);
 
+void window_update_border(Window window, unsigned long color);
+unsigned long get_window_border_color(HSClient* client);
 
 #endif
 
diff --git a/src/settings.c b/src/settings.c
index c036a0b..654f4e0 100644
--- a/src/settings.c
+++ b/src/settings.c
@@ -52,6 +52,7 @@ SettingsPair g_settings[] = {
     SET_STRING( "window_border_active_color",      "red",       CL_COLORS   ),
     SET_STRING( "window_border_normal_color",      "blue",      CL_COLORS   ),
     SET_STRING( "window_border_urgent_color",      "orange",    CL_COLORS   ),
+    SET_STRING( "window_border_inner_color",       "black",     CL_COLORS   ),
     SET_INT(    "always_show_frame",               0,           RELAYOUT    ),
     SET_INT(    "default_direction_external_only", 0,           NULL        ),
     SET_INT(    "default_frame_layout",            0,           FR_COLORS   ),
@@ -62,8 +63,9 @@ SettingsPair g_settings[] = {
     SET_INT(    "raise_on_focus",                  0,           NULL        ),
     SET_INT(    "raise_on_click",                  1,           NULL        ),
     SET_INT(    "gapless_grid",                    1,           RELAYOUT    ),
-    SET_INT(    "smart_window_surroundings",       0,           RELAYOUT    ),
     SET_INT(    "smart_frame_surroundings",        0,           RELAYOUT    ),
+    SET_INT(    "smart_window_surroundings",       0,           RELAYOUT    ),
+    SET_INT(    "double_window_border",            0,           RELAYOUT    ),
     SET_INT(    "monitors_locked",                 0,           LOCK_CHANGED),
     SET_INT(    "auto_detect_monitors",            0,           NULL        ),
     SET_STRING( "tree_style",                      "*| +`--.",  FR_COLORS   ),
diff --git a/src/utils.c b/src/utils.c
index 0a4d203..f71dcbb 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -304,4 +304,42 @@ void* table_find(void* start, size_t elem_size, size_t count,
     return NULL;
 }
 
+void set_window_double_border(Window win, unsigned long inner_border_color, unsigned long outer_border_color) {
+    XWindowAttributes wa;
+
+    if (!XGetWindowAttributes(g_display, win, &wa))
+        return;
+
+    int border_width = wa.border_width;
 
+    if (border_width < 2)
+        return;
+
+    HSDebug("set_window_double_border %ix%i+%i+%i\n", wa.width, wa.height, wa.x, wa.y);
+
+    int width = wa.width;
+    int height = wa.height;
+
+    unsigned int depth = DefaultDepth(g_display, DefaultScreen(g_display));
+
+    int full_width = width + 2 * border_width;
+    int full_height = height + 2 * border_width;
+
+    XSegment segments[4] =
+    {
+        { width, 0, width, height },
+        { full_width - 1, 0, full_width - 1, height },
+        { 0, height, width, height },
+        { 0, full_height - 1, width, full_height - 1 }
+    };
+
+    Pixmap pix = XCreatePixmap(g_display, g_root, full_width, full_height, depth);
+    GC gc = XCreateGC(g_display, pix, 0, NULL);
+    XSetForeground(g_display, gc, outer_border_color);
+    XFillRectangle(g_display, pix, gc, 0, 0, full_width, full_height);
+    XSetForeground(g_display, gc, inner_border_color);
+    XDrawSegments(g_display, pix, gc, segments, 4);
+    XDrawPoint(g_display, pix, gc, full_width - 1, full_height - 1);
+    XSetWindowBorderPixmap(g_display, win, pix);
+    XFreePixmap(g_display, pix);
+}
diff --git a/src/utils.h b/src/utils.h
index 99412dc..cbff41b 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -63,6 +63,11 @@ bool memberequals_int(void* pmember, void* needle);
 
 void* table_find(void* start, size_t elem_size, size_t count,
                  size_t member_offset, MemberEquals equals, void* needle);
+
+void set_window_double_border(Window win,
+                              unsigned long inner_border_color,
+                              unsigned long outer_border_color);
+
 #define STATIC_TABLE_FIND(TYPE, TABLE, MEMBER, EQUALS, NEEDLE)  \
     ((TYPE*) table_find((TABLE),                                \
                         sizeof(TABLE[0]),                       \
-- 
1.7.11.2

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/