From de59f6eccb33fc95d1e5b5ced487ed0cea661b95 Mon Sep 17 00:00:00 2001 From: Christian Muehlhaeuser Date: Wed, 23 May 2018 18:52:54 +0200 Subject: [PATCH 1/3] MakeColor now indicates conversion errors by returning a second value MakeColor now returns a bool as its secondary return value, indicating whether the conversion failed because the source color's alpha channel was set to 0. In such cases, the returned color will have R, G and B set to 0, too. --- README.md | 18 ++++++------------ colors.go | 7 +++++-- colors_test.go | 31 +++++++++++++++++++------------ 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index c170435..a11997c 100644 --- a/README.md +++ b/README.md @@ -124,17 +124,16 @@ Because a `colorful.Color` implements Go's `color.Color` interface (found in the `image/color` package), it can be used anywhere that expects a `color.Color`. Furthermore, you can convert anything that implements the `color.Color` interface -into a `colorful.Color` using the `MakeColorful` function: +into a `colorful.Color` using the `MakeColor` function: ```go -c := colorful.MakeColor(color.Gray16{12345}) +c, ok := colorful.MakeColor(color.Gray16{12345}) ``` -**Caveat:** Be aware that for this latter conversion (using `MakeColor`) hits a corner-case -when alpha is exactly zero. Because `color.Color` uses pre-multiplied alpha colors, -this means the RGB values are lost and it's impossible to recover them. The current -implementation `panic()`s in that case, see [issue 21](https://github.com/lucasb-eyer/go-colorful/issues/21) -for discussion and suggesting alternatives. +**Caveat:** Be aware that this latter conversion (using `MakeColor`) hits a +corner-case when alpha is exactly zero. Because `color.Color` uses pre-multiplied +alpha colors, this means the RGB values are lost (set to 0) and it's impossible +to recover them. In such a case `MakeColor` will return `false` as its second value. ### Comparing colors In the RGB color space, the Euclidian distance between colors *doesn't* correspond @@ -456,11 +455,6 @@ but that is outside the scope of this library. Thanks to [@ZirconiumX](https://github.com/ZirconiumX) for starting this investigation, see [issue #18](https://github.com/lucasb-eyer/go-colorful/issues/18) for details. -### Q: `MakeColor` panics when alpha is zero! -A: Because in that case, the conversion is undefined. See -[issue 21](https://github.com/lucasb-eyer/go-colorful/issues/21) -as well as the short caveat discussion in the ["The `color.Color` interface"](README.md#the-colorcolor-interface) section above. - Who? ==== diff --git a/colors.go b/colors.go index fae57fb..8dfabc3 100644 --- a/colors.go +++ b/colors.go @@ -22,8 +22,11 @@ func (col Color) RGBA() (r, g, b, a uint32) { } // Constructs a colorful.Color from something implementing color.Color -func MakeColor(col color.Color) Color { +func MakeColor(col color.Color) (Color, bool) { r, g, b, a := col.RGBA() + if a == 0 { + return Color{0, 0, 0}, false + } // Since color.Color is alpha pre-multiplied, we need to divide the // RGB values by alpha again in order to get back the original RGB. @@ -34,7 +37,7 @@ func MakeColor(col color.Color) Color { b *= 0xffff b /= a - return Color{float64(r) / 65535.0, float64(g) / 65535.0, float64(b) / 65535.0} + return Color{float64(r) / 65535.0, float64(g) / 65535.0, float64(b) / 65535.0}, true } // Might come in handy sometimes to reduce boilerplate code. diff --git a/colors_test.go b/colors_test.go index 0be66f3..3cd9bef 100644 --- a/colors_test.go +++ b/colors_test.go @@ -475,31 +475,38 @@ func TestClamp(t *testing.T) { func TestMakeColor(t *testing.T) { c_orig_nrgba := color.NRGBA{123, 45, 67, 255} - c_ours := MakeColor(c_orig_nrgba) + c_ours, ok := MakeColor(c_orig_nrgba) r, g, b := c_ours.RGB255() - if r != 123 || g != 45 || b != 67 { - t.Errorf("NRGBA->Colorful->RGB255 error: %v became (%v, %v, %v)", c_orig_nrgba, r, g, b) + if r != 123 || g != 45 || b != 67 || !ok { + t.Errorf("NRGBA->Colorful->RGB255 error: %v became (%v, %v, %v, %t)", c_orig_nrgba, r, g, b, ok) } c_orig_nrgba64 := color.NRGBA64{123 << 8, 45 << 8, 67 << 8, 0xffff} - c_ours = MakeColor(c_orig_nrgba64) + c_ours, ok = MakeColor(c_orig_nrgba64) r, g, b = c_ours.RGB255() - if r != 123 || g != 45 || b != 67 { - t.Errorf("NRGBA64->Colorful->RGB255 error: %v became (%v, %v, %v)", c_orig_nrgba64, r, g, b) + if r != 123 || g != 45 || b != 67 || !ok { + t.Errorf("NRGBA64->Colorful->RGB255 error: %v became (%v, %v, %v, %t)", c_orig_nrgba64, r, g, b, ok) } c_orig_gray := color.Gray{123} - c_ours = MakeColor(c_orig_gray) + c_ours, ok = MakeColor(c_orig_gray) r, g, b = c_ours.RGB255() - if r != 123 || g != 123 || b != 123 { - t.Errorf("Gray->Colorful->RGB255 error: %v became (%v, %v, %v)", c_orig_gray, r, g, b) + if r != 123 || g != 123 || b != 123 || !ok { + t.Errorf("Gray->Colorful->RGB255 error: %v became (%v, %v, %v, %t)", c_orig_gray, r, g, b, ok) } c_orig_gray16 := color.Gray16{123 << 8} - c_ours = MakeColor(c_orig_gray16) + c_ours, ok = MakeColor(c_orig_gray16) r, g, b = c_ours.RGB255() - if r != 123 || g != 123 || b != 123 { - t.Errorf("Gray16->Colorful->RGB255 error: %v became (%v, %v, %v)", c_orig_gray16, r, g, b) + if r != 123 || g != 123 || b != 123 || !ok { + t.Errorf("Gray16->Colorful->RGB255 error: %v became (%v, %v, %v, %t)", c_orig_gray16, r, g, b, ok) + } + + c_orig_rgba := color.RGBA{255, 255, 255, 0} + c_ours, ok = MakeColor(c_orig_rgba) + r, g, b = c_ours.RGB255() + if r != 0 || g != 0 || b != 0 || ok { + t.Errorf("RGBA->Colorful->RGB255 error: %v became (%v, %v, %v, %t)", c_orig_rgba, r, g, b, ok) } } From dc83f475a42114aa7508d9fb69f1cb874b0c69ad Mon Sep 17 00:00:00 2001 From: Christian Muehlhaeuser Date: Sat, 26 May 2018 08:35:27 +0200 Subject: [PATCH 2/3] Added MakeColor caveat to README FAQ --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index a11997c..f424387 100644 --- a/README.md +++ b/README.md @@ -455,6 +455,12 @@ but that is outside the scope of this library. Thanks to [@ZirconiumX](https://github.com/ZirconiumX) for starting this investigation, see [issue #18](https://github.com/lucasb-eyer/go-colorful/issues/18) for details. +### Q: Why would `MakeColor` ever fail!? +A: `MakeColor` fails when the alpha channel is zero. In that case, the +conversion is undefined. See [issue 21](https://github.com/lucasb-eyer/go-colorful/issues/21) +as well as the short caveat note in the ["The `color.Color` interface"](README.md#the-colorcolor-interface) +section above. + Who? ==== From 8d8689c89a788274e9da720dca07845ff2da9fa2 Mon Sep 17 00:00:00 2001 From: Christian Muehlhaeuser Date: Sat, 26 May 2018 08:36:25 +0200 Subject: [PATCH 3/3] Proudly added my own name to the authors paragraph in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f424387..b2887c0 100644 --- a/README.md +++ b/README.md @@ -465,7 +465,7 @@ Who? ==== This library has been developed by Lucas Beyer with contributions from -Bastien Dejean (@baskerville) and Phil Kulak (@pkulak). +Bastien Dejean (@baskerville), Phil Kulak (@pkulak) and Christian Muehlhaeuser (@muesli). License: MIT ============