awesomeWM / awesome

awesome window manager
https://awesomewm.org/
GNU General Public License v2.0
6.39k stars 598 forks source link

Alpha channel ignored on borders #162

Closed phryk closed 6 years ago

phryk commented 9 years ago

When specifying border colors in an awesome theme, the alpha channel is ignored. Far as I can tell, only the border colors are affected, but I have only played around with the values supplied in the default theme.

Current behavior:

Consider this screenshot: https://paste.xinu.at/ckn2U/

For the titlebar, the alpha channel works, with the color being "#7fff00b4". The border, being assigned a color of "#7fff0000", should be invisible, but isn't transparent at all.

Expected behavior:

For the borders to honor the alpha channel, which for the supplied example values would mean the border would be fully transparent.

PS: Someone in the IRC channel implied that the alpha channel is actively stripped/otherwisely ignored in the C part of awesome; And while the lua part of awesome does have a function to strip the alpha channel from a color, I haven't seen it applied to any border property.

psychon commented 9 years ago

Welcome to X11. A color has a red, green and blue components, each having a depth of 16 (!) bits.

The composite and render extension jump through some hoops to work around this. It might be possible to set a border color with an alpha channel. However, I bet that this will run into video driver bugs. It might be more realistic to remove the border properties from a client and instead implement client borders in lua as "fake titlebars" (if you know what I mean).

Anyone interested to write that code?

actionless commented 9 years ago

i am using compton for that, it also adds nice blur on those borders

Elv13 commented 9 years ago
[11:25] <psychon> phryk: are you using a composite manager?
[11:25] <phryk> Elv1313: So I should probably set up the current git master in ~/.local, right?
[11:26] <phryk> psychon: Yes. Because of the alpha bug?
[11:26] <psychon> yeah
[11:26] <phryk> psychon: Yeah, look at the linked screen shot, alpha works fine with xcompmgr for the tasklist, titlebar, etc
[11:27] <psychon> hm... I could move borders into lua
[11:27] <Elv1313> you can already implement border as titlebars
[11:27] <psychon> Elv1313: what do you think about removing c.border_color and c.border_width and instead implement them in awful (similar to how awful.titlebar does things)
[11:28] <Elv1313> psychon: or just use titlebars as border, it will be good for gradient and multi color borders anyway
[11:28] <normalra> psychon: would rules also have to change?
[11:28] <phryk> psychon: If it is easier to do while you're moving anyways, would it be possible to somehow set multiple borders with different width and colors? :3
[11:29] <phryk> that way you could have paddings on the inside and outside of the border by using multiple borders, with a 00 alpha channel :3
[11:30] <psychon> Elv1313: that would make the code for setting titlebars harder to use / kinda ugly
[11:30] <psychon> I wonder if we can come up with something that is "transparent" to uses in rc.lua
psychon commented 9 years ago

Hi, can someone with a composite manager give me a screenshot of what the following program looks like? Does it have a transparent "content" and "border"? https://gist.github.com/psychon/1405fbc8dd19efe92ae0

phryk commented 9 years ago

I can if you tell me how to build it.

I'm not exactly familiar with binary development (more of a scriptwhore, really). Just calling clang with that file leads to "'xcb/xcb.h' file not found". So, I'm guessing that I'm missing some linker option for xcb?

psychon commented 9 years ago

Oh, sorry!

gcc test_border_alpha.c -o test_border_alpha $(pkg-config --cflags --libs xcb)

(Or just -lxcb, although the above is "more correct")

Edit: Oh and you also need to have something like libxcb-devel installed. On debian the package is called libxcb1-dev.

phryk commented 9 years ago

test_border_alpha I'm guessing this is the result you were going for? :)

psychon commented 9 years ago

Thanks.

So it's an implementation details and not part of the API, but the Xorg server uses 0xAARRGGBB for describing colors, too. We can query for the red/green/blue part and just assume that the rest is alpha, but this isn't guaranteed to work correctly by the X11 protocol. I'll see if I can hack something up....

psychon commented 9 years ago

The following patch either terribly breaks X11 colors (which only means: systray background and window border) or you get working alpha support for window borders (no idea what will happen with the systray):

diff --git a/color.c b/color.c
index b9b37f8..c20006f 100644
--- a/color.c
+++ b/color.c
@@ -39,15 +39,18 @@
  */
 static bool
 color_parse(const char *colstr, ssize_t len,
-            uint8_t *red, uint8_t *green, uint8_t *blue)
+            uint8_t *red, uint8_t *green, uint8_t *blue, uint8_t *alpha)
 {
     unsigned long colnum;
     char *p;

+    *alpha = 0xff;
+
     colnum = strtoul(colstr + 1, &p, 16);
     if(len == 9 && (p - colstr) == 9)
     {
-        /* We ignore the alpha component */
+        /* Parse alpha */
+        *alpha = colnum & 0xff;
         colnum >>= 8;
         len -= 2;
         p -= 2;
@@ -77,7 +80,7 @@ color_init_request_t
 color_init_unchecked(color_t *color, const char *colstr, ssize_t len)
 {
     color_init_request_t req;
-    uint8_t red, green, blue;
+    uint8_t red, green, blue, alpha;

     p_clear(&req, 1);

@@ -90,22 +93,30 @@ color_init_unchecked(color_t *color, const char *colstr, ssize_t len)
     req.color = color;

     /* The color is given in RGB value */
-    if(!color_parse(colstr, len, &red, &green, &blue))
+    if(!color_parse(colstr, len, &red, &green, &blue, &alpha))
     {
         warn("awesome: error, invalid color '%s'", colstr);
         req.has_error = true;
         return req;
     }

+    /*
     req.cookie_hexa = xcb_alloc_color_unchecked(globalconf.connection,
                                                 globalconf.default_cmap,
                                                 RGB_8TO16(red),
                                                 RGB_8TO16(green),
                                                 RGB_8TO16(blue));
+                                                */

     req.has_error = false;
     req.colstr = colstr;

+    color->initialized = true;
+    color->red = red;
+    color->green = green;
+    color->blue = blue;
+    color->pixel = alpha << 24 | red << 16 | green << 8 | blue;
+
     return req;
 }

@@ -119,6 +130,7 @@ color_init_reply(color_init_request_t req)
     if(req.has_error)
         return false;

+    /*
     xcb_alloc_color_reply_t *hexa_color;

     if((hexa_color = xcb_alloc_color_reply(globalconf.connection,
@@ -135,6 +147,8 @@ color_init_reply(color_init_request_t req)

     warn("awesome: error, cannot allocate color '%s'", req.colstr);
     return false;
+    */
+    return true;
 }

 /** Push a color as a string onto the stack

What the patch does: Instead of asking the X11 server what the pixel value for some color is, this just assumes that the pixel value is 0xAARRGGBB. So this patch will break everywhere where this assumption isn't valid. This includes little endian machines (?) and X11 server with "weird" default visuals. Oh and surely lots of other cases which I don't think of right now.

psychon commented 6 years ago

Could someone (@actionless ?) test the following patch? This one should actually be commitable, but I have no idea if it works (please, don't just check if alpha works, also check that colors work, if possible).

diff --git a/color.c b/color.c
index b9b37f8f..72da44b7 100644
--- a/color.c
+++ b/color.c
@@ -39,15 +39,17 @@
  */
 static bool
 color_parse(const char *colstr, ssize_t len,
-            uint8_t *red, uint8_t *green, uint8_t *blue)
+            uint8_t *red, uint8_t *green, uint8_t *blue, uint8_t *alpha)
 {
     unsigned long colnum;
     char *p;

+    *alpha = 0xff;
+
     colnum = strtoul(colstr + 1, &p, 16);
     if(len == 9 && (p - colstr) == 9)
     {
-        /* We ignore the alpha component */
+        *alpha = colnum & 0xff;
         colnum >>= 8;
         len -= 2;
         p -= 2;
@@ -65,19 +67,58 @@ color_parse(const char *colstr, ssize_t len,
     return true;
 }

+/** Given a color component value in the range from 0x00 to 0xff and a mask that
+ * specifies where the component is, move the component into place. For example,
+ * component=0x12 and mask=0xff00 return 0x1200. Note that the mask can have a
+ * different width, for example component=0x12 and mask=0xf00 return 0x100.
+ * \param component The intensity of a color component.
+ * \param mask A mask containing a consecutive number of bits set to 1 defining
+ * where the component is.
+ * \return A pixel value containing the giving component in the given component.
+ */
+static uint32_t
+apply_mask(uint8_t component, uint32_t mask)
+{
+    unsigned int shift = 0;
+    unsigned int width = 0;
+
+    // Shift the mask right until the first set bit appears
+    while (mask != 0 && (mask & 0x1) == 0) {
+        shift++;
+        mask >>= 1;
+    }
+    // Shift the mask right until we saw all set bits
+    while ((mask & 0x1) == 1) {
+        width++;
+        mask >>= 1;
+    }
+
+    // The mask consists of [width] set bits followed by [shift] unset bits.
+    // Modify the component accordingly.
+    uint32_t result = component;
+
+    // Scale the result up to the desired width
+    if (width < 8)
+        result >>= (8 - width);
+    if (width > 8)
+        result <<= (width - 8);
+    return result << shift;
+}
+
 /** Send a request to initialize a X color.
  * If you are only interested in the rgba values and don't need the color's
  * pixel value, you should use color_init_unchecked() instead.
  * \param color color_t struct to store color into.
  * \param colstr Color specification.
  * \param len The length of colstr (which still MUST be NULL terminated).
+ * \param visual The visual for which the color is to be allocated.
  * \return request informations.
  */
 color_init_request_t
-color_init_unchecked(color_t *color, const char *colstr, ssize_t len)
+color_init_unchecked(color_t *color, const char *colstr, ssize_t len, xcb_visualtype_t *visual)
 {
     color_init_request_t req;
-    uint8_t red, green, blue;
+    uint8_t red, green, blue, alpha;

     p_clear(&req, 1);

@@ -90,21 +131,40 @@ color_init_unchecked(color_t *color, const char *colstr, ssize_t len)
     req.color = color;

     /* The color is given in RGB value */
-    if(!color_parse(colstr, len, &red, &green, &blue))
+    if(!color_parse(colstr, len, &red, &green, &blue, &alpha))
     {
         warn("awesome: error, invalid color '%s'", colstr);
         req.has_error = true;
         return req;
     }

+    req.colstr = colstr;
+    req.has_error = false;
+
+    if (visual->_class == XCB_VISUAL_CLASS_TRUE_COLOR || visual->_class == XCB_VISUAL_CLASS_DIRECT_COLOR) {
+        uint32_t pixel = 0;
+        pixel |= apply_mask(red, visual->red_mask);
+        pixel |= apply_mask(blue, visual->blue_mask);
+        pixel |= apply_mask(green, visual->green_mask);
+        if (draw_visual_depth(globalconf.screen, visual->visual_id) == 32) {
+            /* This is not actually in the X11 protocol, but we assume that this
+             * is an ARGB visual and everything unset in some mask is alpha.
+             */
+            pixel |= apply_mask(alpha, ~(visual->red_mask | visual->blue_mask | visual->green_mask));
+        }
+        req.color->pixel = pixel;
+        req.color->red   = red;
+        req.color->green = green;
+        req.color->blue  = blue;
+        req.color->initialized = true;
+        return req;
+    }
     req.cookie_hexa = xcb_alloc_color_unchecked(globalconf.connection,
                                                 globalconf.default_cmap,
                                                 RGB_8TO16(red),
                                                 RGB_8TO16(green),
                                                 RGB_8TO16(blue));

-    req.has_error = false;
-    req.colstr = colstr;

     return req;
 }
@@ -118,6 +178,8 @@ color_init_reply(color_init_request_t req)
 {
     if(req.has_error)
         return false;
+    if(req.color->initialized)
+        return true;

     xcb_alloc_color_reply_t *hexa_color;

diff --git a/color.h b/color.h
index 89593448..aaa2bccb 100644
--- a/color.h
+++ b/color.h
@@ -44,7 +44,7 @@ typedef struct
     const char *colstr;
 } color_init_request_t;

-color_init_request_t color_init_unchecked(color_t *, const char *, ssize_t);
+color_init_request_t color_init_unchecked(color_t *, const char *, ssize_t, xcb_visualtype_t *visual);
 bool color_init_reply(color_init_request_t);

 int luaA_pushcolor(lua_State *, const color_t);
diff --git a/objects/window.c b/objects/window.c
index bd069c42..3c043273 100644
--- a/objects/window.c
+++ b/objects/window.c
@@ -194,7 +194,7 @@ luaA_window_set_border_color(lua_State *L, window_t *window)
     const char *color_name = luaL_checklstring(L, -1, &len);

     if(color_name &&
-       color_init_reply(color_init_unchecked(&window->border_color, color_name, len)))
+       color_init_reply(color_init_unchecked(&window->border_color, color_name, len, globalconf.visual)))
     {
         window->border_need_update = true;
         luaA_object_emit_signal(L, -3, "property::border_color", 0);
diff --git a/systray.c b/systray.c
index aa2b1100..c3919ee3 100644
--- a/systray.c
+++ b/systray.c
@@ -355,7 +355,7 @@ luaA_systray(lua_State *L)
         color_t bg_color;
         bool force_redraw = false;

-        if(color_init_reply(color_init_unchecked(&bg_color, bg, bg_len))
+        if(color_init_reply(color_init_unchecked(&bg_color, bg, bg_len, globalconf.default_visual))
                 && globalconf.systray.background_pixel != bg_color.pixel)
         {
             uint32_t config_back[] = { bg_color.pixel };
actionless commented 6 years ago

@psychon oh, it works just perfect and i don't even having an artifact with overlapping client's corner:

2018-03-02--1519994533_641x150_scrot 2018-03-02--1519994650_94x76_scrot

(border transparency turned off in compton now)

thanks a lot for the fix!

just as a sidenote: i put some effort into dynamic persistent tag renaming (across awesome restarts/reboots) however i still haven't renamed my chrm tag after switching back to firefox almost half a year ago :-)

UPD: also with compton it still works (but border transparency is turned off there (frame-opacity=1)): 2018-03-02--1519995804_247x164_scrot

sidenote2: titlebars are also involved here, but because of client shaping tricks they are not visible but still clickable (for mouse moving/swapping and resizing)