diff options
author | Mike Klein <mtklein@chromium.org> | 2017-05-07 00:25:16 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-05-07 04:56:13 +0000 |
commit | ba8ca0a12985f7c715cb69898fbc60956b65613d (patch) | |
tree | 979f53dc79ed9fbab74c23e06d18229cbdae2c28 /gm/hsl.cpp | |
parent | bc63fd45d4937d91977de07a23d384f61a0e57dd (diff) |
polish up gm/hsl.cpp
- remove broken clip_color_KHR().
- rearrange a little to match spec closer
- remove some TODOs
Change-Id: I2de6aa3138455d5970e2cda74f5da6ffadc3db56
Reviewed-on: https://skia-review.googlesource.com/15681
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Diffstat (limited to 'gm/hsl.cpp')
-rw-r--r-- | gm/hsl.cpp | 65 |
1 files changed, 19 insertions, 46 deletions
diff --git a/gm/hsl.cpp b/gm/hsl.cpp index db604c02b9..39074e2681 100644 --- a/gm/hsl.cpp +++ b/gm/hsl.cpp @@ -32,9 +32,8 @@ // web: https://www.w3.org/TR/compositing-1/#blendingnonseparable // KHR: https://www.khronos.org/registry/OpenGL/extensions/KHR/KHR_blend_equation_advanced.txt // It looks like these are meant to be identical, but they disagree on at least ClipColor(). -// I think the KHR version is just wrong... it produces values >1. // -// We'll draw reference colors according to both specs, testing colors where they differ. +// I think the KHR version is just wrong... it produces values >1. So we use the web version. static float min(float r, float g, float b) { return SkTMin(r, SkTMin(g, b)); } static float max(float r, float g, float b) { return SkTMax(r, SkTMax(g, b)); } @@ -56,44 +55,27 @@ static void set_sat(float* r, float* g, float* b, float s) { *g = channel(*g); *b = channel(*b); } - -static void set_lum(float* r, float* g, float* b, float l) { - float diff = l - lum(*r,*g,*b); - *r += diff; - *g += diff; - *b += diff; - // We do clip_color() here as specified, just moved from set_lum() to blend(). -} - -static void clip_color_web(float* r, float* g, float* b) { +static void clip_color(float* r, float* g, float* b) { float l = lum(*r,*g,*b), mn = min(*r,*g,*b), mx = max(*r,*g,*b); auto clip = [=](float c) { if (mn < 0) { c = l + (c - l) * ( l) / (l - mn); } - if (mx > 1) { c = l + (c - l) * (1 - l) / (mx - l); } // <--- notice "1-l" - //SkASSERT(0 <= c); // This may end up very slightly negative... - SkASSERT(c <= 1); + if (mx > 1) { c = l + (c - l) * (1 - l) / (mx - l); } + SkASSERT(-0.0001f < c); // This may end up very slightly negative... + SkASSERT( c <= 1); return c; }; *r = clip(*r); *g = clip(*g); *b = clip(*b); } -static void clip_color_KHR(float* r, float* g, float* b) { - float l = lum(*r,*g,*b), - mn = min(*r,*g,*b), - mx = max(*r,*g,*b); - auto clip = [=](float c) { - if (mn < 0) { c = l + (c - l) * l / (l - mn); } - if (mx > 1) { c = l + (c - l) * l / (mx - l); } // <--- notice "l" - //SkASSERT(0 <= c); // This may end up very slightly negative... - //SkASSERT(c <= 1); // I think the mx > 1 logic is incorrect... - return c; - }; - *r = clip(*r); - *g = clip(*g); - *b = clip(*b); +static void set_lum(float* r, float* g, float* b, float l) { + float diff = l - lum(*r,*g,*b); + *r += diff; + *g += diff; + *b += diff; + clip_color(r,g,b); } @@ -117,7 +99,7 @@ static void saturation(float dr, float dg, float db, G = dg, B = db; set_sat(&R,&G,&B, sat(*sr,*sg,*sb)); - set_lum(&R,&G,&B, lum( dr, dg, db)); // This may seem redundant. TODO: is it? + set_lum(&R,&G,&B, lum( dr, dg, db)); // This may seem redundant, but it is not. *sr = R; *sg = G; *sb = B; @@ -149,7 +131,6 @@ static void luminosity(float dr, float dg, float db, static SkColor blend(SkColor dst, SkColor src, void (*mode)(float,float,float, float*,float*,float*), - void (*clip_color)(float*,float*,float*), bool legacy) { SkASSERT(SkColorGetA(dst) == 0xff @@ -172,14 +153,11 @@ static SkColor blend(SkColor dst, SkColor src, mode( d.fR, d.fG, d.fB, &s.fR, &s.fG, &s.fB); - clip_color(&s.fR, &s.fG, &s.fB); if (legacy) { - // We need to be a little careful here to show off clip_color_KHR()'s overflow - // while avoiding SkASSERTs inside SkColorSetRGB(). - return SkColorSetRGB((int)(s.fR * 255.0f + 0.5f) & 0xff, - (int)(s.fG * 255.0f + 0.5f) & 0xff, - (int)(s.fB * 255.0f + 0.5f) & 0xff); + return SkColorSetRGB(s.fR * 255.0f + 0.5f, + s.fG * 255.0f + 0.5f, + s.fB * 255.0f + 0.5f); } return s.toSkColor(); } @@ -189,8 +167,7 @@ DEF_SIMPLE_GM(hsl, canvas, 600, 100) { sk_tool_utils::set_portable_typeface(&label); label.setAntiAlias(true); - const char* comment = "HSL blend modes are correct when you see no circles in the squares. " - "2 circles means the standards disagree."; + const char* comment = "HSL blend modes are correct when you see no circles in the squares."; canvas->drawText(comment, strlen(comment), 10,10, label); // Just to keep things reaaaal simple, we'll only use opaque colors. @@ -217,13 +194,9 @@ DEF_SIMPLE_GM(hsl, canvas, 600, 100) { canvas->drawRect({20,20,80,80}, fg); if (test.reference) { - SkPaint web,KHR; - auto dst = bg.getColor(), - src = fg.getColor(); - web.setColor(blend(dst, src, test.reference, clip_color_web, legacy)); - KHR.setColor(blend(dst, src, test.reference, clip_color_KHR, legacy)); - canvas->drawCircle(50,50, 20, web); - canvas->drawCircle(50,50, 10, KHR); // This circle may be very wrong. + SkPaint ref; + ref.setColor(blend(bg.getColor(), fg.getColor(), test.reference, legacy)); + canvas->drawCircle(50,50, 20, ref); } const char* name = SkBlendMode_Name(test.mode); |