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 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

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

In particular, if the color is black, every browser does its best to infer the
color.

Fixes: https://bugzilla.mozilla.org/show_bug.cgi?id=1488294

This should require a couple test updates in Gecko I presume.
  • Loading branch information
emilio committed Sep 3, 2018
commit 0fa7019c1a6e17f92197638fda7a723f38ec46ea
@@ -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;
@@ -219,6 +219,9 @@ impl BorderSideHelpers for BorderSide {
//
// 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)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.