image/color: tweak the YCbCr to RGBA conversion formula.
Before, calling the RGBA method of YCbCr color would return red values
in the range [0x0080, 0xff80]. After, the range is [0x0000, 0xffff] and
is consistent with what Gray colors' RGBA method returns. In particular,
pure black, pure white and every Gray color in between are now exactly
representable as a YCbCr color.
This fixes a regression from Go 1.4 (where YCbCr{0x00, 0x80, 0x80} was
no longer equivalent to pure black), introduced by golang.org/cl/8073 in
the Go 1.5 development cycle. In Go 1.4, the +0x80 rounding was not
noticable when Cb == 0x80 && Cr == 0x80, because the YCbCr to RGBA
conversion truncated to 8 bits before multiplying by 0x101, so the
output range was [0x0000, 0xffff].
The TestYCbCrRoundtrip fuzzy-match tolerance grows from 1 to 2 because
the YCbCr to RGB conversion now maps to an ever-so-slightly larger
range, along with the usual imprecision of accumulating rounding errors.
Also s/int/int32/ in ycbcr.go. The conversion shouldn't overflow either
way, as int is always at least 32 bits, but it does make it clearer that
the computation doesn't depend on sizeof(int).
Fixes #11691
Change-Id: I538ca0adf7e040fa96c5bc8b3aef4454535126b9
Reviewed-on: https://go-review.googlesource.com/12220
Reviewed-by: Rob Pike <r@golang.org>
diff --git a/src/image/color/ycbcr.go b/src/image/color/ycbcr.go
index f8c1326..4bcb07d 100644
--- a/src/image/color/ycbcr.go
+++ b/src/image/color/ycbcr.go
@@ -11,9 +11,10 @@
// Cb = -0.1687*R - 0.3313*G + 0.5000*B + 128
// Cr = 0.5000*R - 0.4187*G - 0.0813*B + 128
// http://www.w3.org/Graphics/JPEG/jfif3.pdf says Y but means Y'.
- r1 := int(r)
- g1 := int(g)
- b1 := int(b)
+
+ r1 := int32(r)
+ g1 := int32(g)
+ b1 := int32(b)
yy := (19595*r1 + 38470*g1 + 7471*b1 + 1<<15) >> 16
cb := (-11056*r1 - 21712*g1 + 32768*b1 + 257<<15) >> 16
cr := (32768*r1 - 27440*g1 - 5328*b1 + 257<<15) >> 16
@@ -42,9 +43,10 @@
// G = Y' - 0.34414*(Cb-128) - 0.71414*(Cr-128)
// B = Y' + 1.77200*(Cb-128)
// http://www.w3.org/Graphics/JPEG/jfif3.pdf says Y but means Y'.
- yy1 := int(y)<<16 + 1<<15
- cb1 := int(cb) - 128
- cr1 := int(cr) - 128
+
+ yy1 := int32(y) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(cb) - 128
+ cr1 := int32(cr) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
@@ -96,12 +98,12 @@
// fmt.Printf("0x%04x 0x%04x 0x%04x\n", r0, g0, b0)
// fmt.Printf("0x%04x 0x%04x 0x%04x\n", r1, g1, b1)
// prints:
- // 0x7e19 0x808e 0x7dba
+ // 0x7e18 0x808e 0x7db9
// 0x7e7e 0x8080 0x7d7d
- yy1 := int(c.Y)<<16 + 1<<15
- cb1 := int(c.Cb) - 128
- cr1 := int(c.Cr) - 128
+ yy1 := int32(c.Y) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(c.Cb) - 128
+ cr1 := int32(c.Cr) - 128
r := (yy1 + 91881*cr1) >> 8
g := (yy1 - 22554*cb1 - 46802*cr1) >> 8
b := (yy1 + 116130*cb1) >> 8
diff --git a/src/image/color/ycbcr_test.go b/src/image/color/ycbcr_test.go
index 6bf53f1..d64e38e 100644
--- a/src/image/color/ycbcr_test.go
+++ b/src/image/color/ycbcr_test.go
@@ -5,6 +5,7 @@
package color
import (
+ "fmt"
"testing"
)
@@ -15,8 +16,18 @@
return y - x
}
+func eq(c0, c1 Color) error {
+ r0, g0, b0, a0 := c0.RGBA()
+ r1, g1, b1, a1 := c1.RGBA()
+ if r0 != r1 || g0 != g1 || b0 != b1 || a0 != a1 {
+ return fmt.Errorf("got 0x%04x 0x%04x 0x%04x 0x%04x\nwant 0x%04x 0x%04x 0x%04x 0x%04x",
+ r0, g0, b0, a0, r1, g1, b1, a1)
+ }
+ return nil
+}
+
// TestYCbCrRoundtrip tests that a subset of RGB space can be converted to YCbCr
-// and back to within 1/256 tolerance.
+// and back to within 2/256 tolerance.
func TestYCbCrRoundtrip(t *testing.T) {
for r := 0; r < 256; r += 7 {
for g := 0; g < 256; g += 5 {
@@ -24,8 +35,9 @@
r0, g0, b0 := uint8(r), uint8(g), uint8(b)
y, cb, cr := RGBToYCbCr(r0, g0, b0)
r1, g1, b1 := YCbCrToRGB(y, cb, cr)
- if delta(r0, r1) > 1 || delta(g0, g1) > 1 || delta(b0, b1) > 1 {
- t.Fatalf("\nr0, g0, b0 = %d, %d, %d\nr1, g1, b1 = %d, %d, %d", r0, g0, b0, r1, g1, b1)
+ if delta(r0, r1) > 2 || delta(g0, g1) > 2 || delta(b0, b1) > 2 {
+ t.Fatalf("\nr0, g0, b0 = %d, %d, %d\ny, cb, cr = %d, %d, %d\nr1, g1, b1 = %d, %d, %d",
+ r0, g0, b0, y, cb, cr, r1, g1, b1)
}
}
}
@@ -52,6 +64,15 @@
}
}
+// TestYCbCrGray tests that YCbCr colors are a superset of Gray colors.
+func TestYCbCrGray(t *testing.T) {
+ for i := 0; i < 256; i++ {
+ if err := eq(YCbCr{uint8(i), 0x80, 0x80}, Gray{uint8(i)}); err != nil {
+ t.Errorf("i=0x%02d:\n%v", i, err)
+ }
+ }
+}
+
// TestCMYKRoundtrip tests that a subset of RGB space can be converted to CMYK
// and back to within 1/256 tolerance.
func TestCMYKRoundtrip(t *testing.T) {
@@ -62,7 +83,8 @@
c, m, y, k := RGBToCMYK(r0, g0, b0)
r1, g1, b1 := CMYKToRGB(c, m, y, k)
if delta(r0, r1) > 1 || delta(g0, g1) > 1 || delta(b0, b1) > 1 {
- t.Fatalf("\nr0, g0, b0 = %d, %d, %d\nr1, g1, b1 = %d, %d, %d", r0, g0, b0, r1, g1, b1)
+ t.Fatalf("\nr0, g0, b0 = %d, %d, %d\nc, m, y, k = %d, %d, %d, %d\nr1, g1, b1 = %d, %d, %d",
+ r0, g0, b0, c, m, y, k, r1, g1, b1)
}
}
}
@@ -91,6 +113,15 @@
}
}
+// TestCMYKGray tests that CMYK colors are a superset of Gray colors.
+func TestCMYKGray(t *testing.T) {
+ for i := 0; i < 256; i++ {
+ if err := eq(CMYK{0x00, 0x00, 0x00, uint8(255 - i)}, Gray{uint8(i)}); err != nil {
+ t.Errorf("i=0x%02d:\n%v", i, err)
+ }
+ }
+}
+
func TestPalette(t *testing.T) {
p := Palette{
RGBA{0xff, 0xff, 0xff, 0xff},
diff --git a/src/image/decode_example_test.go b/src/image/decode_example_test.go
index 21e90fe..81fa037 100644
--- a/src/image/decode_example_test.go
+++ b/src/image/decode_example_test.go
@@ -61,22 +61,22 @@
}
// Output:
// bin red green blue alpha
- // 0x0000-0x0fff: 353 759 7228 0
- // 0x1000-0x1fff: 629 2944 1036 0
- // 0x2000-0x2fff: 1075 2319 984 0
- // 0x3000-0x3fff: 838 2291 988 0
- // 0x4000-0x4fff: 540 1302 542 0
- // 0x5000-0x5fff: 319 971 263 0
- // 0x6000-0x6fff: 316 377 178 0
- // 0x7000-0x7fff: 581 280 216 0
- // 0x8000-0x8fff: 3457 228 274 0
- // 0x9000-0x9fff: 2294 237 334 0
- // 0xa000-0xafff: 938 283 370 0
- // 0xb000-0xbfff: 322 338 401 0
- // 0xc000-0xcfff: 229 386 295 0
- // 0xd000-0xdfff: 263 416 281 0
- // 0xe000-0xefff: 538 433 312 0
- // 0xf000-0xffff: 2758 1886 1748 15450
+ // 0x0000-0x0fff: 364 790 7242 0
+ // 0x1000-0x1fff: 645 2967 1039 0
+ // 0x2000-0x2fff: 1072 2299 979 0
+ // 0x3000-0x3fff: 820 2266 980 0
+ // 0x4000-0x4fff: 537 1305 541 0
+ // 0x5000-0x5fff: 319 962 261 0
+ // 0x6000-0x6fff: 322 375 177 0
+ // 0x7000-0x7fff: 601 279 214 0
+ // 0x8000-0x8fff: 3478 227 273 0
+ // 0x9000-0x9fff: 2260 234 329 0
+ // 0xa000-0xafff: 921 282 373 0
+ // 0xb000-0xbfff: 321 335 397 0
+ // 0xc000-0xcfff: 229 388 298 0
+ // 0xd000-0xdfff: 260 414 277 0
+ // 0xe000-0xefff: 516 428 298 0
+ // 0xf000-0xffff: 2785 1899 1772 15450
}
const data = `
diff --git a/src/image/draw/draw_test.go b/src/image/draw/draw_test.go
index 29951bf..a58f0f4 100644
--- a/src/image/draw/draw_test.go
+++ b/src/image/draw/draw_test.go
@@ -163,8 +163,8 @@
// The source pixel is {0, 0, 136} in YCbCr-space, which is {11, 38, 0, 255} in RGB-space.
{"ycbcr", vgradCr(), fillAlpha(255), Over, color.RGBA{11, 38, 0, 255}},
{"ycbcrSrc", vgradCr(), fillAlpha(255), Src, color.RGBA{11, 38, 0, 255}},
- {"ycbcrAlpha", vgradCr(), fillAlpha(192), Over, color.RGBA{42, 29, 0, 255}},
- {"ycbcrAlphaSrc", vgradCr(), fillAlpha(192), Src, color.RGBA{8, 29, 0, 192}},
+ {"ycbcrAlpha", vgradCr(), fillAlpha(192), Over, color.RGBA{42, 28, 0, 255}},
+ {"ycbcrAlphaSrc", vgradCr(), fillAlpha(192), Src, color.RGBA{8, 28, 0, 192}},
{"ycbcrNil", vgradCr(), nil, Over, color.RGBA{11, 38, 0, 255}},
{"ycbcrNilSrc", vgradCr(), nil, Src, color.RGBA{11, 38, 0, 255}},
// Uniform mask (100%, 75%, nil) and variable Gray source.
diff --git a/src/image/internal/imageutil/gen.go b/src/image/internal/imageutil/gen.go
index 6779b49..fc1e707 100644
--- a/src/image/internal/imageutil/gen.go
+++ b/src/image/internal/imageutil/gen.go
@@ -95,9 +95,9 @@
%s
// This is an inline version of image/color/ycbcr.go's func YCbCrToRGB.
- yy1 := int(src.Y[yi])<<16 + 1<<15
- cb1 := int(src.Cb[ci]) - 128
- cr1 := int(src.Cr[ci]) - 128
+ yy1 := int32(src.Y[yi]) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(src.Cb[ci]) - 128
+ cr1 := int32(src.Cr[ci]) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
diff --git a/src/image/internal/imageutil/impl.go b/src/image/internal/imageutil/impl.go
index d5dee46..fd7826d 100644
--- a/src/image/internal/imageutil/impl.go
+++ b/src/image/internal/imageutil/impl.go
@@ -44,9 +44,9 @@
for x := x0; x != x1; x, yi, ci = x+4, yi+1, ci+1 {
// This is an inline version of image/color/ycbcr.go's func YCbCrToRGB.
- yy1 := int(src.Y[yi])<<16 + 1<<15
- cb1 := int(src.Cb[ci]) - 128
- cr1 := int(src.Cr[ci]) - 128
+ yy1 := int32(src.Y[yi]) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(src.Cb[ci]) - 128
+ cr1 := int32(src.Cr[ci]) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
@@ -83,9 +83,9 @@
ci := ciBase + sx/2
// This is an inline version of image/color/ycbcr.go's func YCbCrToRGB.
- yy1 := int(src.Y[yi])<<16 + 1<<15
- cb1 := int(src.Cb[ci]) - 128
- cr1 := int(src.Cr[ci]) - 128
+ yy1 := int32(src.Y[yi]) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(src.Cb[ci]) - 128
+ cr1 := int32(src.Cr[ci]) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
@@ -122,9 +122,9 @@
ci := ciBase + sx/2
// This is an inline version of image/color/ycbcr.go's func YCbCrToRGB.
- yy1 := int(src.Y[yi])<<16 + 1<<15
- cb1 := int(src.Cb[ci]) - 128
- cr1 := int(src.Cr[ci]) - 128
+ yy1 := int32(src.Y[yi]) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(src.Cb[ci]) - 128
+ cr1 := int32(src.Cr[ci]) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
@@ -160,9 +160,9 @@
for x := x0; x != x1; x, yi, ci = x+4, yi+1, ci+1 {
// This is an inline version of image/color/ycbcr.go's func YCbCrToRGB.
- yy1 := int(src.Y[yi])<<16 + 1<<15
- cb1 := int(src.Cb[ci]) - 128
- cr1 := int(src.Cr[ci]) - 128
+ yy1 := int32(src.Y[yi]) * 0x10100 // Convert 0x12 to 0x121200.
+ cb1 := int32(src.Cb[ci]) - 128
+ cr1 := int32(src.Cr[ci]) - 128
r := (yy1 + 91881*cr1) >> 16
g := (yy1 - 22554*cb1 - 46802*cr1) >> 16
b := (yy1 + 116130*cb1) >> 16
