From c6a4e4519fb000fe835486bcb02f3253e4316a3b Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 28 Dec 2015 12:43:53 +0100 Subject: [PATCH 1/2] Correct color management for pango fonts Corrects the cases where the colorpixel is not 0xRRGGBB : we have to use the full color_t struct to describe font colors, as Pango expects RGB values and not an XCB colorpixel value. --- i3-config-wizard/main.c | 14 ++++++------- i3-input/main.c | 2 +- i3-nagbar/main.c | 46 ++++++++++++++++++++--------------------- include/libi3.h | 28 ++++++++++++------------- libi3/draw_util.c | 2 +- libi3/font.c | 10 ++++----- src/restore_layout.c | 2 +- src/sighandler.c | 6 +++--- 8 files changed, 55 insertions(+), 55 deletions(-) diff --git a/i3-config-wizard/main.c b/i3-config-wizard/main.c index bd12cd81..284f15fa 100644 --- a/i3-config-wizard/main.c +++ b/i3-config-wizard/main.c @@ -479,7 +479,7 @@ static int handle_expose() { if (current_step == STEP_WELCOME) { /* restore font color */ - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); txt(logical_px(10), 2, "You have not configured i3 yet."); txt(logical_px(10), 3, "Do you want me to generate a config at"); @@ -493,16 +493,16 @@ static int handle_expose() { txt(logical_px(85), 8, "No, I will use the defaults"); /* green */ - set_font_colors(pixmap_gc, get_colorpixel("#00FF00"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#00FF00"), draw_util_hex_to_color("#000000")); txt(logical_px(25), 6, ""); /* red */ - set_font_colors(pixmap_gc, get_colorpixel("#FF0000"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FF0000"), draw_util_hex_to_color("#000000")); txt(logical_px(31), 8, ""); } if (current_step == STEP_GENERATE) { - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); txt(logical_px(10), 2, "Please choose either:"); txt(logical_px(85), 4, "Win as default modifier"); @@ -519,7 +519,7 @@ static int handle_expose() { /* the selected modifier */ set_font(&bold_font); - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); if (modifier == MOD_Mod4) txt(logical_px(10), 4, "-> "); else @@ -527,11 +527,11 @@ static int handle_expose() { /* green */ set_font(&font); - set_font_colors(pixmap_gc, get_colorpixel("#00FF00"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#00FF00"), draw_util_hex_to_color("#000000")); txt(logical_px(25), 9, ""); /* red */ - set_font_colors(pixmap_gc, get_colorpixel("#FF0000"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FF0000"), draw_util_hex_to_color("#000000")); txt(logical_px(31), 10, ""); } diff --git a/i3-input/main.c b/i3-input/main.c index 4e1be78b..e196891c 100644 --- a/i3-input/main.c +++ b/i3-input/main.c @@ -137,7 +137,7 @@ static int handle_expose(void *data, xcb_connection_t *conn, xcb_expose_event_t xcb_poly_fill_rectangle(conn, pixmap, pixmap_gc, 1, &inner); /* restore font color */ - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); /* draw the prompt … */ if (prompt != NULL) { diff --git a/i3-nagbar/main.c b/i3-nagbar/main.c index d6ace221..cbbe9ef8 100644 --- a/i3-nagbar/main.c +++ b/i3-nagbar/main.c @@ -51,11 +51,11 @@ static button_t *buttons; static int buttoncnt; /* Result of get_colorpixel() for the various colors. */ -static uint32_t color_background; /* background of the bar */ -static uint32_t color_button_background; /* background for buttons */ -static uint32_t color_border; /* color of the button border */ -static uint32_t color_border_bottom; /* color of the bottom border */ -static uint32_t color_text; /* color of the text */ +static color_t color_background; /* background of the bar */ +static color_t color_button_background; /* background for buttons */ +static color_t color_border; /* color of the button border */ +static color_t color_border_bottom; /* color of the bottom border */ +static color_t color_text; /* color of the text */ xcb_window_t root; xcb_connection_t *conn; @@ -191,7 +191,7 @@ static void handle_button_release(xcb_connection_t *conn, xcb_button_release_eve */ static int handle_expose(xcb_connection_t *conn, xcb_expose_event_t *event) { /* re-draw the background */ - xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_background}); + xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_background.colorpixel}); xcb_poly_fill_rectangle(conn, pixmap, pixmap_gc, 1, &rect); /* restore font color */ @@ -210,14 +210,14 @@ static int handle_expose(xcb_connection_t *conn, xcb_expose_event_t *event) { w += logical_px(8); int y = rect.width; uint32_t values[3]; - values[0] = color_button_background; + values[0] = color_button_background.colorpixel; values[1] = line_width; xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND | XCB_GC_LINE_WIDTH, values); xcb_rectangle_t close = {y - w - (2 * line_width), 0, w + (2 * line_width), rect.height}; xcb_poly_fill_rectangle(conn, pixmap, pixmap_gc, 1, &close); - xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_border}); + xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_border.colorpixel}); xcb_point_t points[] = { {y - w - (2 * line_width), line_width / 2}, {y - (line_width / 2), line_width / 2}, @@ -245,11 +245,11 @@ static int handle_expose(xcb_connection_t *conn, xcb_expose_event_t *event) { /* account for left/right padding, which seems to be set to 12px (total) below */ w += logical_px(12); y -= logical_px(30); - xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_button_background}); + xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_button_background.colorpixel}); close = (xcb_rectangle_t){y - w - (2 * line_width), logical_px(2), w + (2 * line_width), rect.height - logical_px(6)}; xcb_poly_fill_rectangle(conn, pixmap, pixmap_gc, 1, &close); - xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_border}); + xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND, (uint32_t[]){color_border.colorpixel}); buttons[c].x = y - w - (2 * line_width); buttons[c].width = w; xcb_point_t points2[] = { @@ -260,8 +260,8 @@ static int handle_expose(xcb_connection_t *conn, xcb_expose_event_t *event) { {y - w - (2 * line_width), (line_width / 2) + logical_px(2)}}; xcb_poly_line(conn, XCB_COORD_MODE_ORIGIN, pixmap, pixmap_gc, 5, points2); - values[0] = color_text; - values[1] = color_button_background; + values[0] = color_text.colorpixel; + values[1] = color_button_background.colorpixel; set_font_colors(pixmap_gc, color_text, color_button_background); /* the x term seems to set left/right padding */ draw_text(buttons[c].label, pixmap, pixmap_gc, NULL, @@ -274,7 +274,7 @@ static int handle_expose(xcb_connection_t *conn, xcb_expose_event_t *event) { /* border line at the bottom */ line_width = logical_px(2); - values[0] = color_border_bottom; + values[0] = color_border_bottom.colorpixel; values[1] = line_width; xcb_change_gc(conn, pixmap_gc, XCB_GC_FOREGROUND | XCB_GC_LINE_WIDTH, values); xcb_point_t bottom[] = { @@ -448,18 +448,18 @@ int main(int argc, char *argv[]) { if (bar_type == TYPE_ERROR) { /* Red theme for error messages */ - color_button_background = get_colorpixel("#680a0a"); - color_background = get_colorpixel("#900000"); - color_text = get_colorpixel("#ffffff"); - color_border = get_colorpixel("#d92424"); - color_border_bottom = get_colorpixel("#470909"); + color_button_background = draw_util_hex_to_color("#680a0a"); + color_background = draw_util_hex_to_color("#900000"); + color_text = draw_util_hex_to_color("#ffffff"); + color_border = draw_util_hex_to_color("#d92424"); + color_border_bottom = draw_util_hex_to_color("#470909"); } else { /* Yellowish theme for warnings */ - color_button_background = get_colorpixel("#ffc100"); - color_background = get_colorpixel("#ffa8000"); - color_text = get_colorpixel("#000000"); - color_border = get_colorpixel("#ab7100"); - color_border_bottom = get_colorpixel("#ab7100"); + color_button_background = draw_util_hex_to_color("#ffc100"); + color_background = draw_util_hex_to_color("#ffa8000"); + color_text = draw_util_hex_to_color("#000000"); + color_border = draw_util_hex_to_color("#ab7100"); + color_border_bottom = draw_util_hex_to_color("#ab7100"); } font = load_font(pattern, true); diff --git a/include/libi3.h b/include/libi3.h index 90ed8989..0c69f1c0 100644 --- a/include/libi3.h +++ b/include/libi3.h @@ -392,11 +392,24 @@ char *convert_ucs2_to_utf8(xcb_char2b_t *text, size_t num_glyphs); */ xcb_char2b_t *convert_utf8_to_ucs2(char *input, size_t *real_strlen); +/* Represents a color split by color channel. */ +typedef struct color_t { + double red; + double green; + double blue; + double alpha; + + /* The colorpixel we use for direct XCB calls. */ + uint32_t colorpixel; +} color_t; + +#define COLOR_TRANSPARENT ((color_t){.red = 0.0, .green = 0.0, .blue = 0.0, .colorpixel = 0}) + /** * Defines the colors to be used for the forthcoming draw_text calls. * */ -void set_font_colors(xcb_gcontext_t gc, uint32_t foreground, uint32_t background); +void set_font_colors(xcb_gcontext_t gc, color_t foreground, color_t background); /** * Returns true if and only if the current font is a pango font. @@ -501,19 +514,6 @@ int mkdirp(const char *path, mode_t mode); } while (0) #endif -/* Represents a color split by color channel. */ -typedef struct color_t { - double red; - double green; - double blue; - double alpha; - - /* The colorpixel we use for direct XCB calls. */ - uint32_t colorpixel; -} color_t; - -#define COLOR_TRANSPARENT ((color_t){.red = 0.0, .green = 0.0, .blue = 0.0, .colorpixel = 0}) - /* A wrapper grouping an XCB drawable and both a graphics context * and the corresponding cairo objects representing it. */ typedef struct surface_t { diff --git a/libi3/draw_util.c b/libi3/draw_util.c index fa538d1a..dcd881c2 100644 --- a/libi3/draw_util.c +++ b/libi3/draw_util.c @@ -144,7 +144,7 @@ void draw_util_text(i3String *text, surface_t *surface, color_t fg_color, color_ CAIRO_SURFACE_FLUSH(surface->surface); #endif - set_font_colors(surface->gc, fg_color.colorpixel, bg_color.colorpixel); + set_font_colors(surface->gc, fg_color, bg_color); draw_text(text, surface->id, surface->gc, surface->visual_type, x, y, max_width); #ifdef CAIRO_SUPPORT diff --git a/libi3/font.c b/libi3/font.c index c09a7fb1..9e72849a 100644 --- a/libi3/font.c +++ b/libi3/font.c @@ -310,7 +310,7 @@ void free_font(void) { * Defines the colors to be used for the forthcoming draw_text calls. * */ -void set_font_colors(xcb_gcontext_t gc, uint32_t foreground, uint32_t background) { +void set_font_colors(xcb_gcontext_t gc, color_t foreground, color_t background) { assert(savedFont != NULL); switch (savedFont->type) { @@ -320,16 +320,16 @@ void set_font_colors(xcb_gcontext_t gc, uint32_t foreground, uint32_t background case FONT_TYPE_XCB: { /* Change the font and colors in the GC */ uint32_t mask = XCB_GC_FOREGROUND | XCB_GC_BACKGROUND | XCB_GC_FONT; - uint32_t values[] = {foreground, background, savedFont->specific.xcb.id}; + uint32_t values[] = {foreground.colorpixel, background.colorpixel, savedFont->specific.xcb.id}; xcb_change_gc(conn, gc, mask, values); break; } #if PANGO_SUPPORT case FONT_TYPE_PANGO: /* Save the foreground font */ - pango_font_red = ((foreground >> 16) & 0xff) / 255.0; - pango_font_green = ((foreground >> 8) & 0xff) / 255.0; - pango_font_blue = (foreground & 0xff) / 255.0; + pango_font_red = foreground.red; + pango_font_green = foreground.green; + pango_font_blue = foreground.blue; break; #endif default: diff --git a/src/restore_layout.c b/src/restore_layout.c index 5ca4cff5..af77b11d 100644 --- a/src/restore_layout.c +++ b/src/restore_layout.c @@ -133,7 +133,7 @@ static void update_placeholder_contents(placeholder_state *state) { xcb_flush(restore_conn); xcb_aux_sync(restore_conn); - set_font_colors(state->gc, config.client.placeholder.text.colorpixel, config.client.placeholder.background.colorpixel); + set_font_colors(state->gc, config.client.placeholder.text, config.client.placeholder.background); Match *swallows; int n = 0; diff --git a/src/sighandler.c b/src/sighandler.c index 555f5e55..80d2fae2 100644 --- a/src/sighandler.c +++ b/src/sighandler.c @@ -141,7 +141,7 @@ static int sig_draw_window(xcb_window_t win, int width, int height, int font_hei xcb_poly_fill_rectangle(conn, pixmap, pixmap_gc, 1, &inner); /* restore font color */ - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); char *bt_colour = "#FFFFFF"; if (backtrace_done < 0) @@ -152,14 +152,14 @@ static int sig_draw_window(xcb_window_t win, int width, int height, int font_hei for (int i = 0; crash_text_i3strings[i] != NULL; ++i) { /* fix the colour for the backtrace line when it finished */ if (i == backtrace_string_index) - set_font_colors(pixmap_gc, get_colorpixel(bt_colour), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color(bt_colour), draw_util_hex_to_color("#000000")); draw_text(crash_text_i3strings[i], pixmap, pixmap_gc, NULL, 8, 5 + i * font_height, width - 16); /* and reset the colour again for other lines */ if (i == backtrace_string_index) - set_font_colors(pixmap_gc, get_colorpixel("#FFFFFF"), get_colorpixel("#000000")); + set_font_colors(pixmap_gc, draw_util_hex_to_color("#FFFFFF"), draw_util_hex_to_color("#000000")); } /* Copy the contents of the pixmap to the real window */ From 287a0b4c3cc24a18f5063b7d5743f0b85d6852a1 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 28 Dec 2015 12:58:32 +0100 Subject: [PATCH 2/2] get_colorpixel support for non-true color displays Re-introduce fully-fledged get_colorpixel function, which enables arbitrary color depths for the display. The previous code is kept as an optimization for the case of a true color display, where a X11 roundtrip is unnecessary. --- libi3/get_colorpixel.c | 59 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/libi3/get_colorpixel.c b/libi3/get_colorpixel.c index f81ea6c2..8820938f 100644 --- a/libi3/get_colorpixel.c +++ b/libi3/get_colorpixel.c @@ -9,21 +9,28 @@ #include #include +#include "queue.h" #include "libi3.h" +struct Colorpixel { + char *hex; + uint32_t pixel; + + SLIST_ENTRY(Colorpixel) + colorpixels; +}; + +SLIST_HEAD(colorpixel_head, Colorpixel) +colorpixels; + /* - * Returns the colorpixel to use for the given hex color (think of HTML). Only - * works for true-color (vast majority of cases) at the moment, avoiding a - * roundtrip to X11. + * Returns the colorpixel to use for the given hex color (think of HTML). * * The hex_color has to start with #, for example #FF00FF. * * NOTE that get_colorpixel() does _NOT_ check the given color code for validity. * This has to be done by the caller. * - * NOTE that this function may in the future rely on a global xcb_connection_t - * variable called 'conn' to be present. - * */ uint32_t get_colorpixel(const char *hex) { char strgroups[3][3] = { @@ -34,5 +41,43 @@ uint32_t get_colorpixel(const char *hex) { uint8_t g = strtol(strgroups[1], NULL, 16); uint8_t b = strtol(strgroups[2], NULL, 16); - return (0xFF << 24) | (r << 16 | g << 8 | b); + /* Shortcut: if our screen is true color, no need to do a roundtrip to X11 */ + if (root_screen->root_depth == 24 || root_screen->root_depth == 32) { + return (0xFF << 24) | (r << 16 | g << 8 | b); + } + + /* Lookup this colorpixel in the cache */ + struct Colorpixel *colorpixel; + SLIST_FOREACH(colorpixel, &(colorpixels), colorpixels) { + if (strcmp(colorpixel->hex, hex) == 0) + return colorpixel->pixel; + } + +#define RGB_8_TO_16(i) (65535 * ((i)&0xFF) / 255) + int r16 = RGB_8_TO_16(r); + int g16 = RGB_8_TO_16(g); + int b16 = RGB_8_TO_16(b); + + xcb_alloc_color_reply_t *reply; + + reply = xcb_alloc_color_reply(conn, xcb_alloc_color(conn, root_screen->default_colormap, + r16, g16, b16), + NULL); + + if (!reply) { + LOG("Could not allocate color\n"); + exit(1); + } + + uint32_t pixel = reply->pixel; + free(reply); + + /* Store the result in the cache */ + struct Colorpixel *cache_pixel = scalloc(1, sizeof(struct Colorpixel)); + cache_pixel->hex = sstrdup(hex); + cache_pixel->pixel = pixel; + + SLIST_INSERT_HEAD(&(colorpixels), cache_pixel, colorpixels); + + return pixel; }