Skip to content

Commit 3a0cda4

Browse files
nigeltaonigeltao
committed
image/draw: have draw.Src preserve NRGBA colors
This reverts a behavior change introduced in Go 1.18 (commit 9f69a44; CL 340049). In Go 1.17 and earlier, draw.Draw(etc, draw.Src) with image.NRGBA dst and src images would pass through a (heap allocated) color.Color interface value holding a color.NRGBA concrete value. Threading that color.NRGBA value all the way through preserves non-premultiplied alpha transparency information (distinguishing e.g. transparent blue from transparent red). CL 340049 optimized out that heap allocation (per pixel), calling new SetRGBA64At and RGBA64At methods instead. However, these methods (like the existing image/color Color.RGBA method) work in premultiplied alpha, so any distinction between transparent colors is lost. This commit re-introduces the preservation of distinct transparencies, when dst and src are both *image.NRGBA (or both *image.NRGBA64) and the op is draw.Src. Fixes #51893 Change-Id: Id9c64bfeeaecc458586f169f50b99d6c8aa52a7f Reviewed-on: https://go-review.googlesource.com/c/go/+/396795 Trust: Nigel Tao <nigeltao@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
1 parent d3362fc commit 3a0cda4

File tree

3 files changed

+121
-15
lines changed

3 files changed

+121
-15
lines changed

‎doc/go1.19.html

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,19 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
7676
<p>
7777
TODO: complete this section
7878
</p>
79+
80+
<dl id="image/draw"><dt><a href="/pkg/image/draw/">image/draw</a></dt>
81+
<dd>
82+
<p><!-- CL 396795 -->
83+
<code>Draw</code> with the <code>Src</code> operator preserves
84+
non-premultiplied-alpha colors when destination and source images are
85+
both <code>*image.NRGBA</code> (or both <code>*image.NRGBA64</code>).
86+
This reverts a behavior change accidentally introduced by a Go 1.18
87+
library optimization, to match the behavior in Go 1.17 and earlier.
88+
</p>
89+
</dd>
90+
</dl><!-- image/draw -->
91+
7992
<dl id="net"><dt><a href="/pkg/net/">net</a></dt>
8093
<dd>
8194
<p><!-- CL 386016 -->

‎src/image/draw/draw.go

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ func DrawMask(dst Image, r image.Rectangle, src image.Image, sp image.Point, mas
121121

122122
// Fast paths for special cases. If none of them apply, then we fall back
123123
// to general but slower implementations.
124+
//
125+
// For NRGBA and NRGBA64 image types, the code paths aren't just faster.
126+
// They also avoid the information loss that would otherwise occur from
127+
// converting non-alpha-premultiplied color to and from alpha-premultiplied
128+
// color. See TestDrawSrcNonpremultiplied.
124129
switch dst0 := dst.(type) {
125130
case *image.RGBA:
126131
if op == Over {
@@ -181,7 +186,10 @@ func DrawMask(dst Image, r image.Rectangle, src image.Image, sp image.Point, mas
181186
drawFillSrc(dst0, r, sr, sg, sb, sa)
182187
return
183188
case *image.RGBA:
184-
drawCopySrc(dst0, r, src0, sp)
189+
d0 := dst0.PixOffset(r.Min.X, r.Min.Y)
190+
s0 := src0.PixOffset(sp.X, sp.Y)
191+
drawCopySrc(
192+
dst0.Pix[d0:], dst0.Stride, r, src0.Pix[s0:], src0.Stride, sp, 4*r.Dx())
185193
return
186194
case *image.NRGBA:
187195
drawNRGBASrc(dst0, r, src0, sp)
@@ -222,6 +230,26 @@ func DrawMask(dst Image, r image.Rectangle, src image.Image, sp image.Point, mas
222230
return
223231
}
224232
}
233+
case *image.NRGBA:
234+
if op == Src && mask == nil {
235+
if src0, ok := src.(*image.NRGBA); ok {
236+
d0 := dst0.PixOffset(r.Min.X, r.Min.Y)
237+
s0 := src0.PixOffset(sp.X, sp.Y)
238+
drawCopySrc(
239+
dst0.Pix[d0:], dst0.Stride, r, src0.Pix[s0:], src0.Stride, sp, 4*r.Dx())
240+
return
241+
}
242+
}
243+
case *image.NRGBA64:
244+
if op == Src && mask == nil {
245+
if src0, ok := src.(*image.NRGBA64); ok {
246+
d0 := dst0.PixOffset(r.Min.X, r.Min.Y)
247+
s0 := src0.PixOffset(sp.X, sp.Y)
248+
drawCopySrc(
249+
dst0.Pix[d0:], dst0.Stride, r, src0.Pix[s0:], src0.Stride, sp, 8*r.Dx())
250+
return
251+
}
252+
}
225253
}
226254

227255
x0, x1, dx := r.Min.X, r.Max.X, 1
@@ -449,27 +477,28 @@ func drawCopyOver(dst *image.RGBA, r image.Rectangle, src *image.RGBA, sp image.
449477
}
450478
}
451479

452-
func drawCopySrc(dst *image.RGBA, r image.Rectangle, src *image.RGBA, sp image.Point) {
453-
n, dy := 4*r.Dx(), r.Dy()
454-
d0 := dst.PixOffset(r.Min.X, r.Min.Y)
455-
s0 := src.PixOffset(sp.X, sp.Y)
456-
var ddelta, sdelta int
457-
if r.Min.Y <= sp.Y {
458-
ddelta = dst.Stride
459-
sdelta = src.Stride
460-
} else {
480+
// drawCopySrc copies bytes to dstPix from srcPix. These arguments roughly
481+
// correspond to the Pix fields of the image package's concrete image.Image
482+
// implementations, but are offset (dstPix is dst.Pix[dpOffset:] not dst.Pix).
483+
func drawCopySrc(
484+
dstPix []byte, dstStride int, r image.Rectangle,
485+
srcPix []byte, srcStride int, sp image.Point,
486+
bytesPerRow int) {
487+
488+
d0, s0, ddelta, sdelta, dy := 0, 0, dstStride, srcStride, r.Dy()
489+
if r.Min.Y > sp.Y {
461490
// If the source start point is higher than the destination start
462491
// point, then we compose the rows in bottom-up order instead of
463492
// top-down. Unlike the drawCopyOver function, we don't have to check
464493
// the x coordinates because the built-in copy function can handle
465494
// overlapping slices.
466-
d0 += (dy - 1) * dst.Stride
467-
s0 += (dy - 1) * src.Stride
468-
ddelta = -dst.Stride
469-
sdelta = -src.Stride
495+
d0 = (dy - 1) * dstStride
496+
s0 = (dy - 1) * srcStride
497+
ddelta = -dstStride
498+
sdelta = -srcStride
470499
}
471500
for ; dy > 0; dy-- {
472-
copy(dst.Pix[d0:d0+n], src.Pix[s0:s0+n])
501+
copy(dstPix[d0:d0+bytesPerRow], srcPix[s0:s0+bytesPerRow])
473502
d0 += ddelta
474503
s0 += sdelta
475504
}

‎src/image/draw/draw_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,70 @@ func TestFill(t *testing.T) {
622622
}
623623
}
624624

625+
func TestDrawSrcNonpremultiplied(t *testing.T) {
626+
var (
627+
opaqueGray = color.NRGBA{0x99, 0x99, 0x99, 0xff}
628+
transparentBlue = color.NRGBA{0x00, 0x00, 0xff, 0x00}
629+
transparentGreen = color.NRGBA{0x00, 0xff, 0x00, 0x00}
630+
transparentRed = color.NRGBA{0xff, 0x00, 0x00, 0x00}
631+
632+
opaqueGray64 = color.NRGBA64{0x9999, 0x9999, 0x9999, 0xffff}
633+
transparentPurple64 = color.NRGBA64{0xfedc, 0x0000, 0x7654, 0x0000}
634+
)
635+
636+
// dst and src are 1x3 images but the dr rectangle (and hence the overlap)
637+
// is only 1x2. The Draw call should affect dst's pixels at (1, 10) and (2,
638+
// 10) but the pixel at (0, 10) should be untouched.
639+
//
640+
// The src image is entirely transparent (and the Draw operator is Src) so
641+
// the two touched pixels should be set to transparent colors.
642+
//
643+
// In general, Go's color.Color type (and specifically the Color.RGBA
644+
// method) works in premultiplied alpha, where there's no difference
645+
// between "transparent blue" and "transparent red". It's all "just
646+
// transparent" and canonically "transparent black" (all zeroes).
647+
//
648+
// However, since the operator is Src (so the pixels are 'copied', not
649+
// 'blended') and both dst and src images are *image.NRGBA (N stands for
650+
// Non-premultiplied alpha which *does* distinguish "transparent blue" and
651+
// "transparent red"), we prefer that this distinction carries through and
652+
// dst's touched pixels should be transparent blue and transparent green,
653+
// not just transparent black.
654+
{
655+
dst := image.NewNRGBA(image.Rect(0, 10, 3, 11))
656+
dst.SetNRGBA(0, 10, opaqueGray)
657+
src := image.NewNRGBA(image.Rect(1, 20, 4, 21))
658+
src.SetNRGBA(1, 20, transparentBlue)
659+
src.SetNRGBA(2, 20, transparentGreen)
660+
src.SetNRGBA(3, 20, transparentRed)
661+
662+
dr := image.Rect(1, 10, 3, 11)
663+
Draw(dst, dr, src, image.Point{1, 20}, Src)
664+
665+
if got, want := dst.At(0, 10), opaqueGray; got != want {
666+
t.Errorf("At(0, 10):\ngot %#v\nwant %#v", got, want)
667+
}
668+
if got, want := dst.At(1, 10), transparentBlue; got != want {
669+
t.Errorf("At(1, 10):\ngot %#v\nwant %#v", got, want)
670+
}
671+
if got, want := dst.At(2, 10), transparentGreen; got != want {
672+
t.Errorf("At(2, 10):\ngot %#v\nwant %#v", got, want)
673+
}
674+
}
675+
676+
// Check image.NRGBA64 (not image.NRGBA) similarly.
677+
{
678+
dst := image.NewNRGBA64(image.Rect(0, 0, 1, 1))
679+
dst.SetNRGBA64(0, 0, opaqueGray64)
680+
src := image.NewNRGBA64(image.Rect(0, 0, 1, 1))
681+
src.SetNRGBA64(0, 0, transparentPurple64)
682+
Draw(dst, dst.Bounds(), src, image.Point{0, 0}, Src)
683+
if got, want := dst.At(0, 0), transparentPurple64; got != want {
684+
t.Errorf("At(0, 0):\ngot %#v\nwant %#v", got, want)
685+
}
686+
}
687+
}
688+
625689
// TestFloydSteinbergCheckerboard tests that the result of Floyd-Steinberg
626690
// error diffusion of a uniform 50% gray source image with a black-and-white
627691
// palette is a checkerboard pattern.

0 commit comments

Comments
 (0)