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

image/draw: optimize DrawMask when drawing a Uniform Image onto a Paletted Image #35938

Closed
pjbgtnj opened this issue Dec 2, 2019 · 6 comments
Closed

Comments

@pjbgtnj
Copy link

@pjbgtnj pjbgtnj commented Dec 2, 2019

What version of Go are you using (go version)? 1.13.4

$ go version
go version devel +a18608a Mon Dec 2 20:12:54 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes - and github master branch too

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

Wrote a program that ran much slower than expected. The following program will reproduce the slow speed:

package main

import (
	"fmt"
	"image"
	"image/color"
	"image/color/palette"
	"image/draw"
	"time"
)

func main() {
	startTime := time.Now()
	red := &color.RGBA{255, 0, 0, 0}
	rect := image.Rect(-1000, -1000, 1000, 1000)
	palettedImage := image.NewPaletted(rect, palette.WebSafe)
	uniformImage := image.NewUniform(red)
	draw.Draw(palettedImage, rect, uniformImage, image.Point{}, draw.Src)
	numPoints := rect.Dx() * rect.Dy()
	took := time.Since(startTime)
	fmt.Printf("Took: %s for %d points - %f pps\n", took, numPoints, float64(numPoints)/took.Seconds())
}

What did you expect to see?

It should run very quickly, not take several seconds. The DrawMask function in image/draw/draw.go is called a lot for the specific application I'm working with and this is the bottleneck.

What did you see instead?

I expected it to run nearly instantly

Here is a proposed fix:

diff --git a/src/image/draw/draw.go b/src/image/draw/draw.go
index 932a544..3a0157d 100644
--- a/src/image/draw/draw.go
+++ b/src/image/draw/draw.go
@@ -181,8 +181,25 @@ func DrawMask(dst Image, r image.Rectangle, src image.Image, sp image.Point, mas
                return
        case *image.Paletted:
                if op == Src && mask == nil && !processBackward(dst, r, src, sp) {
-                       drawPaletted(dst0, r, src, sp, false)
-                       return
+                        switch src0 := src.(type) {
+                        case *image.Uniform:
+                                i0 := dst0.PixOffset(r.Min.X, r.Min.Y)
+                                i1 := i0 + r.Dx()
+                                colorIdx := uint8(dst0.Palette.Index(src0.C))
+                                for i := i0; i < i1; i++ {
+                                        dst0.Pix[i] = colorIdx
+                                }
+                                firstRow := dst0.Pix[i0:i1]
+                                for y := r.Min.Y + 1; y < r.Max.Y; y++ {
+                                        i0 += dst0.Stride
+                                        i1 += dst0.Stride
+                                        copy(dst0.Pix[i0:i1], firstRow)
+                                }
+                                return
+                        default:
+                                drawPaletted(dst0, r, src, sp, false)
+                                return
+                        }
                }
        }

@pjbgtnj
Copy link
Author

@pjbgtnj pjbgtnj commented Dec 2, 2019

Difference in speed between current version and proposed change:

Current Version: Took: 3.722995689s for 4000000 points - 1074403.607777 pps

With Change:      Took: 3.170383ms for 4000000 points - 1261677216.916694 pps

Loading

@cagedmantis cagedmantis changed the title image/draw DrawMask slow when drawing a Uniform Image onto a Paletted Image image/draw: optimize DrawMask when drawing a Uniform Image onto a Paletted Image Dec 3, 2019
@cagedmantis cagedmantis added this to the Backlog milestone Dec 3, 2019
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Dec 3, 2019

@bradfitz
You've been active in this package. Please let me know if this should be directed toward anybody else.

Loading

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 4, 2019

@nigeltao primarily owns image/*.

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Dec 24, 2019

Proposed fix looks fine for whenever the Go 1.15 tree opens, after the Go 1.14 release.

The recommended process for contributing patches is documented at
https://golang.org/doc/contribute.html

Or, if you're happy without formal authorship attribution, in terms of the Author field in the git log, I'm happy to send out the code review with this fix.

Loading

@pjbgtnj
Copy link
Author

@pjbgtnj pjbgtnj commented Dec 24, 2019

@nigeltao - I'm fine without formal authorship but I will look into following the recommended process in the future.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 25, 2020

Change https://golang.org/cl/230118 mentions this issue: image/draw: optimize paletted dst + uniform src

Loading

@gopherbot gopherbot closed this in 42c4899 Apr 27, 2020
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
name            old time/op  new time/op  delta
PalettedFill-4  5.74ms ± 1%  0.01ms ± 1%  -99.78%  (p=0.008 n=5+5)
PalettedRGBA-4  3.34ms ± 3%  3.33ms ± 0%     ~     (p=0.690 n=5+5)

Fixes golang#35938

Thanks to pjbgtnj for the suggestion.

Change-Id: I07b494482cce918f556e196c5a4b481b4c16de3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230118
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
@golang golang locked and limited conversation to collaborators Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants