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

Be consistent about how the algorithm we use to modulate colors. #3010

Merged
merged 4 commits into from Sep 4, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -102,22 +102,41 @@ vec2 get_outer_corner_scale(int segment) {
return p;
}

vec4 mod_color(vec4 color, float f) {
return vec4(clamp(color.rgb * f, vec3(0.0), vec3(color.a)), color.a);
// NOTE(emilio): If you change this algorithm, do the same change
// in border.rs
vec4 mod_color(vec4 color, bool is_black, bool lighter) {
const float light_black = 0.7;
const float dark_black = 0.3;

const float dark_scale = 0.66666666;
const float light_scale = 1.0;

if (is_black) {
if (lighter) {
return vec4(vec3(light_black), color.a);
}
return vec4(vec3(dark_black), color.a);
}

if (lighter) {
return vec4(color.rgb * light_scale, color.a);
}
return vec4(color.rgb * dark_scale, color.a);
}

vec4[2] get_colors_for_side(vec4 color, int style) {
vec4 result[2];
const vec2 f = vec2(1.3, 0.7);

bool is_black = color.rgb == vec3(0.0, 0.0, 0.0);

This comment has been minimized.

Copy link
@kvark

kvark Sep 4, 2018

Member

This may not be portable. IIRC, vector comparison produces vector of booleans. We should be using dot(color.rgb,color.rgb) or all(color.rgb == vec3(0.0, 0.0, 0.0))

This comment has been minimized.

Copy link
@nical

nical Sep 4, 2018

Collaborator

I could be wrong but I think that some_vec3 == other_vec3 returns a boolean in glsl and that the equal function returns a vector of boolean.

This comment has been minimized.

Copy link
@kvark

kvark Sep 4, 2018

Member

Indeed. I just recalled a driver bug with those, hence my radar fired up the red light :)

This comment has been minimized.

Copy link
@kvark

switch (style) {
case BORDER_STYLE_GROOVE:
result[0] = mod_color(color, f.x);
result[1] = mod_color(color, f.y);
result[0] = mod_color(color, is_black, true);
result[1] = mod_color(color, is_black, false);
break;
case BORDER_STYLE_RIDGE:
result[0] = mod_color(color, f.y);
result[1] = mod_color(color, f.x);
result[0] = mod_color(color, is_black, false);
result[1] = mod_color(color, is_black, true);
break;
default:
result[0] = color;
@@ -201,40 +201,34 @@ impl<'a> DisplayListFlattener<'a> {
}

pub trait BorderSideHelpers {
fn border_color(
&self,
scale_factor_0: f32,
scale_factor_1: f32,
black_color_0: f32,
black_color_1: f32,
) -> ColorF;
fn border_color(&self, is_inner_border: bool) -> ColorF;
}

impl BorderSideHelpers for BorderSide {
fn border_color(
&self,
scale_factor_0: f32,
scale_factor_1: f32,
black_color_0: f32,
black_color_1: f32,
) -> ColorF {
match self.style {
BorderStyle::Inset => {
if self.color.r != 0.0 || self.color.g != 0.0 || self.color.b != 0.0 {
self.color.scale_rgb(scale_factor_1)
} else {
ColorF::new(black_color_0, black_color_0, black_color_0, self.color.a)
}
}
BorderStyle::Outset => {
if self.color.r != 0.0 || self.color.g != 0.0 || self.color.b != 0.0 {
self.color.scale_rgb(scale_factor_0)
} else {
ColorF::new(black_color_1, black_color_1, black_color_1, self.color.a)
}
}
_ => self.color,
fn border_color(&self, is_inner_border: bool) -> ColorF {
let lighter = match self.style {
BorderStyle::Inset => is_inner_border,
BorderStyle::Outset => !is_inner_border,
_ => return self.color,
};

// The modulate colors below are not part of the specification. They are
// derived from the Gecko source code and experimentation, and used to
// modulate the colors in order to generate colors for the inset/outset
// and groove/ridge border styles.
//
// NOTE(emilio): Gecko at least takes the background color into
// account, should we do the same? Looks a bit annoying for this.
//
// NOTE(emilio): If you change this algorithm, do the same change on
// get_colors_for_side in cs_border_segment.glsl.
if self.color.r != 0.0 || self.color.g != 0.0 || self.color.b != 0.0 {
let scale = if lighter { 1.0 } else { 2.0 / 3.0 };
return self.color.scale_rgb(scale)
}

let black = if lighter { 0.7 } else { 0.3 };
ColorF::new(black, black, black, self.color.a)
}
}

@@ -912,21 +906,8 @@ impl BorderRenderTaskInfo {
side1.style
};

// These modulate colors are not part of the specification. They
// are derived from the Gecko source code and experimentation, and
// used to modulate the colors in order to generate colors for
// the inset/outset and groove/ridge border styles.
let color0 = if flip0 {
side0.border_color(2.0 / 3.0, 1.0, 0.7, 0.3)
} else {
side0.border_color(1.0, 2.0 / 3.0, 0.3, 0.7)
};

let color1 = if flip1 {
side1.border_color(2.0 / 3.0, 1.0, 0.7, 0.3)
} else {
side1.border_color(1.0, 2.0 / 3.0, 0.3, 0.7)
};
let color0 = side0.border_color(flip0);
let color1 = side1.border_color(flip1);

add_segment(
info.task_rect,
@@ -428,7 +428,10 @@ impl BrushKind {

// Construct a brush that is a border with `border` style and `widths`
// dimensions.
pub fn new_border(border: NormalBorder, widths: BorderWidths) -> BrushKind {
pub fn new_border(mut border: NormalBorder, widths: BorderWidths) -> BrushKind {
// FIXME(emilio): Is this the best place to do this?
border.normalize(&widths);

let cache_key = BorderCacheKey::new(&border, &widths);
BrushKind::Border {
source: BorderSource::Border {
@@ -273,6 +273,33 @@ impl NormalBorder {
b.bottom.color = color;
b
}

/// Normalizes a border so that we don't render disallowed stuff, like inset
/// borders that are less than two pixels wide.
#[inline]
pub fn normalize(&mut self, widths: &BorderWidths) {
#[inline]
fn renders_small_border_solid(style: BorderStyle) -> bool {
match style {
BorderStyle::Groove |
BorderStyle::Ridge |
BorderStyle::Inset |
BorderStyle::Outset => true,
_ => false,
}
}

let normalize_side = |side: &mut BorderSide, width: f32| {
if renders_small_border_solid(side.style) && width < 2. {
side.style = BorderStyle::Solid;
}
};

normalize_side(&mut self.left, widths.left);
normalize_side(&mut self.right, widths.right);
normalize_side(&mut self.top, widths.top);
normalize_side(&mut self.bottom, widths.bottom);
}
}

#[repr(u32)]
@@ -9,7 +9,7 @@ root:
width: [ 6, 6, 6, 6 ]
border-type: normal
style: [ solid, solid, solid, solid ]
color: [ 0 0 178 1.0, 0 0 255 1.0, 0 0 255 1.0, 0 0 178 1.0 ]
color: [ 0 0 170 1.0, 0 0 255 1.0, 0 0 255 1.0, 0 0 170 1.0 ]
- type: stacking-context
bounds: [6, 6, 38, 38]
items:
@@ -18,4 +18,4 @@ root:
width: [ 6, 6, 6, 6 ]
border-type: normal
style: [ solid, solid, solid, solid ]
color: [ 0 0 255 1.0, 0 0 178 1.0, 0 0 178 1.0, 0 0 255 1.0 ]
color: [ 0 0 255 1.0, 0 0 170 1.0, 0 0 170 1.0, 0 0 255 1.0 ]
@@ -9,7 +9,7 @@ root:
width: [ 6, 6, 6, 6 ]
border-type: normal
style: [ solid, solid, solid, solid ]
color: [ 0 0 255 1.0, 0 0 178 1.0, 0 0 178 1.0, 0 0 255 1.0 ]
color: [ 0 0 255 1.0, 0 0 170 1.0, 0 0 170 1.0, 0 0 255 1.0 ]
- type: stacking-context
bounds: [6, 6, 38, 38]
items:
@@ -18,4 +18,4 @@ root:
width: [ 6, 6, 6, 6 ]
border-type: normal
style: [ solid, solid, solid, solid ]
color: [ 0 0 178 1.0, 0 0 255 1.0, 0 0 255 1.0, 0 0 178 1.0 ]
color: [ 0 0 170 1.0, 0 0 255 1.0, 0 0 255 1.0, 0 0 170 1.0 ]
Binary file not shown.
Binary file not shown.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.