From 4092796db3337910b5bcffcb539a05317f5e922c Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sat, 24 Feb 2018 17:28:35 -0800 Subject: [PATCH] Fix an issue where the background color rendered gray instead of black When using the default monospace font on ubuntu (typicaly DejaVuSansMono), the texture atlas would end up the bottom left pixel being a shade of gray. Since we were using (0,0) coords for whitespace cells, all whitespace cells would appear shaded gray. We now reserve a black pixel in the bottom left of the underline texture and switch to that for whitespace instead. I added Debug support to the ColorPalette while tracking this down, and figured that I might as well keep it. --- src/config.rs | 4 ++-- src/opengl/render.rs | 35 +++++++++++++++++++++++++++++++++++ term/src/color.rs | 21 ++++++++++++++++++--- 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/config.rs b/src/config.rs index b8a024fb2..304e2a765 100644 --- a/src/config.rs +++ b/src/config.rs @@ -205,12 +205,12 @@ impl From for term::color::ColorPalette { if let Some(ansi) = cfg.ansi { for (idx, col) in ansi.iter().enumerate() { - p.colors[idx] = *col; + p.colors.0[idx] = *col; } } if let Some(brights) = cfg.brights { for (idx, col) in brights.iter().enumerate() { - p.colors[idx + 8] = *col; + p.colors.0[idx + 8] = *col; } } p diff --git a/src/opengl/render.rs b/src/opengl/render.rs index c641bbf93..ba39f1dea 100644 --- a/src/opengl/render.rs +++ b/src/opengl/render.rs @@ -94,6 +94,7 @@ struct Vertex { underline: f32, strikethrough: f32, v_idx: f32, + is_white: f32, } implement_vertex!( @@ -107,6 +108,7 @@ implement_vertex!( underline, strikethrough, v_idx, + is_white, ); const VERTEX_SHADER: &str = r#" @@ -119,6 +121,7 @@ in vec4 bg_color; in float has_color; in float underline; in float v_idx; +in float is_white; uniform mat4 projection; uniform mat4 translation; @@ -130,6 +133,7 @@ out vec4 o_fg_color; out vec4 o_bg_color; out float o_has_color; out float o_underline; +out float o_is_white; // Offset from the RHS texture coordinate to the LHS. // This is an underestimation to avoid the shader interpolating @@ -141,6 +145,7 @@ void main() { o_bg_color = bg_color; o_has_color = has_color; o_underline = underline; + o_is_white = is_white; if (bg_and_line_layer) { gl_Position = projection * vec4(position, 0.0, 1.0); @@ -195,6 +200,7 @@ in vec4 o_fg_color; in vec4 o_bg_color; in float o_has_color; in float o_underline; +in float o_is_white; out vec4 color; uniform sampler2D glyph_tex; @@ -235,6 +241,8 @@ void main() { color = under_color; } } + } else if (o_is_white != 0.0) { + color = texture2D(underline_tex, tex_coords); } else { color = texture2D(glyph_tex, tex_coords); if (o_has_color == 0.0) { @@ -368,6 +376,13 @@ impl Renderer { } } + // IMPORTANT: We also use this same texture for the whitespace cells. + // the bottom left corner MUST be a blank pixel! + assert_eq!(underline_data[0 + (width * 4 * (cell_height - 1))], 0u8); + assert_eq!(underline_data[1 + (width * 4 * (cell_height - 1))], 0u8); + assert_eq!(underline_data[2 + (width * 4 * (cell_height - 1))], 0u8); + assert_eq!(underline_data[3 + (width * 4 * (cell_height - 1))], 0u8); + glium::texture::SrgbTexture2d::new( facade, glium::texture::RawImage2d::from_raw_rgba( @@ -472,6 +487,11 @@ impl Renderer { (info.x_offset, info.y_offset) }; + println!( + "load_glyph {:?} dims={},{}", + info, glyph.width, glyph.height + ); + let glyph = if glyph.width == 0 || glyph.height == 0 { // a whitespace glyph CachedGlyph { @@ -788,11 +808,23 @@ impl Renderer { vert[V_TOP_RIGHT].has_color = has_color; vert[V_BOT_LEFT].has_color = has_color; vert[V_BOT_RIGHT].has_color = has_color; + + vert[V_TOP_LEFT].is_white = 0.0; + vert[V_TOP_RIGHT].is_white = 0.0; + vert[V_BOT_LEFT].is_white = 0.0; + vert[V_BOT_RIGHT].is_white = 0.0; } &None => { // Whitespace; no texture to render let zero = (0.0, 0.0f32); + vert[V_TOP_LEFT].is_white = 1.0; + vert[V_TOP_RIGHT].is_white = 1.0; + vert[V_BOT_LEFT].is_white = 1.0; + vert[V_BOT_RIGHT].is_white = 1.0; + + // Note: these 0 coords refer to the blank pixel + // in the bottom left of the underline texture! vert[V_TOP_LEFT].tex = zero; vert[V_TOP_RIGHT].tex = zero; vert[V_BOT_LEFT].tex = zero; @@ -840,9 +872,12 @@ impl Renderer { vert.bg_color = bg_color; vert.fg_color = glyph_color; vert.underline = U_NONE; + // Note: these 0 coords refer to the blank pixel + // in the bottom left of the underline texture! vert.tex = (0.0, 0.0); vert.adjust = Default::default(); vert.has_color = 0.0; + vert.is_white = 1.0; } } diff --git a/term/src/color.rs b/term/src/color.rs index 09999d0bc..32ddd4d86 100644 --- a/term/src/color.rs +++ b/term/src/color.rs @@ -2,6 +2,8 @@ use palette; use serde::{self, Deserialize, Deserializer}; +use std::fmt; +use std::result::Result; #[allow(dead_code)] #[derive(Debug, Clone, Copy)] @@ -115,8 +117,11 @@ pub enum ColorAttribute { } #[derive(Clone)] +pub struct Palette256(pub [RgbColor; 256]); + +#[derive(Clone, Debug)] pub struct ColorPalette { - pub colors: [RgbColor; 256], + pub colors: Palette256, pub foreground: RgbColor, pub background: RgbColor, pub cursor_fg: RgbColor, @@ -125,12 +130,22 @@ pub struct ColorPalette { pub selection_bg: RgbColor, } +impl fmt::Debug for Palette256 { + fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { + // If we wanted to dump all of the entries, we'd use this: + // self.0[..].fmt(fmt) + // However, we typically don't care about those and we're interested + // in the Debug-ability of ColorPalette that embeds us. + write!(fmt, "[suppressed]") + } +} + impl ColorPalette { pub fn resolve(&self, color: &ColorAttribute) -> RgbColor { match color { &ColorAttribute::Foreground => self.foreground, &ColorAttribute::Background => self.background, - &ColorAttribute::PaletteIndex(idx) => self.colors[idx as usize], + &ColorAttribute::PaletteIndex(idx) => self.colors.0[idx as usize], &ColorAttribute::Rgb(color) => color, } } @@ -274,7 +289,7 @@ impl Default for ColorPalette { let selection_bg = RgbColor::new(0xff, 0xfa, 0xcd); ColorPalette { - colors, + colors: Palette256(colors), foreground, background, cursor_fg,