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
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@pjbgtnj
Copy link

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 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

@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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 3, 2019
@cagedmantis cagedmantis added this to the Backlog milestone Dec 3, 2019
@cagedmantis
Copy link
Contributor

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

@bradfitz
Copy link
Contributor

bradfitz commented Dec 4, 2019

@nigeltao primarily owns image/*.

@nigeltao
Copy link
Contributor

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.

@pjbgtnj
Copy link
Author

pjbgtnj commented Dec 24, 2019

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

@gopherbot
Copy link

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

xujianhai666 pushed 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.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

5 participants