Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alpha channel ignored on borders #162

Closed
phryk opened this issue Mar 11, 2015 · 11 comments
Closed

Alpha channel ignored on borders #162

phryk opened this issue Mar 11, 2015 · 11 comments
Assignees
Milestone

Comments

@phryk
Copy link

phryk commented Mar 11, 2015

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
Copy link
Member

psychon commented Mar 11, 2015

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
Copy link
Member

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

@Elv13
Copy link
Member

Elv13 commented Mar 11, 2015

[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
Copy link
Member

psychon commented Mar 15, 2015

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
Copy link
Author

phryk commented Mar 15, 2015

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
Copy link
Member

psychon commented Mar 15, 2015

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
Copy link
Author

phryk commented Mar 15, 2015

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

@psychon
Copy link
Member

psychon commented Mar 15, 2015

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 psychon self-assigned this Apr 4, 2015
@psychon
Copy link
Member

psychon commented Sep 21, 2015

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.

actionless added a commit to actionless/awesome that referenced this issue Mar 2, 2018
@psychon psychon removed the need info label Mar 2, 2018
@psychon psychon added this to the v4.3 milestone Mar 2, 2018
@psychon
Copy link
Member

psychon commented Mar 2, 2018

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 added a commit to actionless/awesome that referenced this issue Mar 2, 2018
@actionless
Copy link
Member

actionless commented Mar 2, 2018

@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)

psychon added a commit to psychon/awesome that referenced this issue Mar 2, 2018
Up to now, we always asked the X11 server for color allocation ("which
pixel value corresponds to (r,g,b)?", an AllocCollor request).

This commit adds direct support for TrueColor and DirectColor visuals.
In such a visual, the X11 server gives tells us where the red, green,
and blue color components are in a pixel value and we can then just
directly compute the pixel value.

Additionally, this commit adds code that assumes that in a depth=32
visual, the remaining values (after handling red, green, blue) is the
alpha channel for colors. Thus, this adds support for transparent client
borders.

This commit also touches code for the systray. However, the systray must
always use the X11 server's default visual and that one always(?) has
depth=24, i.e. does not support an alpha channel. Thus, the systray
background still cannot be transparent.

Also, in theory this commit should support visuals where some color
component does not have 8 bits, for example RGB565. However, this is
just theoretic and I have no idea how to actually test this (without
jumping through too many hoops).

Fixes: awesomeWM#162
Signed-off-by: Uli Schlachter <psychon@znc.in>
psychon added a commit to psychon/awesome that referenced this issue Mar 3, 2018
Up to now, we always asked the X11 server for color allocation ("which
pixel value corresponds to (r,g,b)?", an AllocCollor request).

This commit adds direct support for TrueColor and DirectColor visuals.
In such a visual, the X11 server gives tells us where the red, green,
and blue color components are in a pixel value and we can then just
directly compute the pixel value.

Additionally, this commit adds code that assumes that in a depth=32
visual, the remaining values (after handling red, green, blue) is the
alpha channel for colors. Thus, this adds support for transparent client
borders.

This commit also touches code for the systray. However, the systray must
always use the X11 server's default visual and that one always(?) has
depth=24, i.e. does not support an alpha channel. Thus, the systray
background still cannot be transparent.

Also, in theory this commit should support visuals where some color
component does not have 8 bits, for example RGB565. However, this is
just theoretic and I have no idea how to actually test this (without
jumping through too many hoops).

Fixes: awesomeWM#162
Signed-off-by: Uli Schlachter <psychon@znc.in>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants