From 7b7eccd65e9adc38feeb35f0efe78449e705a965 Mon Sep 17 00:00:00 2001 From: Gunnsteinn Hall Date: Tue, 30 Apr 2019 19:10:52 +0000 Subject: [PATCH] V3: Detect and fix stride offsets in loaded go images (#448) * Detect and fix stride offsets in loaded go images * Benchmarks for image copying performance * Account for potential image offsets when constructing model Image data Removes need for extra copying of image data --- pdf/model/image.go | 38 ++++++++++++++------ pdf/model/image_test.go | 78 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 10 deletions(-) diff --git a/pdf/model/image.go b/pdf/model/image.go index 7a8a5d06..28d58035 100644 --- a/pdf/model/image.go +++ b/pdf/model/image.go @@ -164,7 +164,6 @@ func (img *Image) ToGoImage() (goimage.Image, error) { aidx := 0 samples := img.GetSamples() - //bytesPerColor := colorComponents * int(img.BitsPerComponent) / 8 bytesPerColor := img.ColorComponents for i := 0; i+bytesPerColor-1 < len(samples); i += bytesPerColor { var c gocolor.Color @@ -253,22 +252,32 @@ func (ih DefaultImageHandler) NewImageFromGoImage(goimg goimage.Image) (*Image, // Will not be required once the golang image/jpeg package is optimized. m = goimage.NewRGBA(goimage.Rect(0, 0, b.Dx(), b.Dy())) draw.Draw(m, m.Bounds(), goimg, b.Min, draw.Src) + b = m.Bounds() } - numPixels := len(m.Pix) / 4 + numPixels := b.Dx() * b.Dy() data := make([]byte, 3*numPixels) alphaData := make([]byte, numPixels) hasAlpha := false - for i := 0; i < numPixels; i++ { - data[3*i], data[3*i+1], data[3*i+2] = m.Pix[4*i], m.Pix[4*i+1], m.Pix[4*i+2] + i0 := m.PixOffset(b.Min.X, b.Min.Y) + i1 := i0 + b.Dx()*4 - alpha := m.Pix[4*i+3] - if alpha != 255 { - // If all alpha values are 255 (opaque), means that the alpha transparency channel is unnecessary. - hasAlpha = true + j := 0 + for y := b.Min.Y; y < b.Max.Y; y++ { + for i := i0; i < i1; i += 4 { + data[3*j], data[3*j+1], data[3*j+2] = m.Pix[i], m.Pix[i+1], m.Pix[i+2] + alpha := m.Pix[i+3] + if alpha != 255 { + // If all alpha values are 255 (opaque), means that the alpha transparency channel is unnecessary. + hasAlpha = true + } + alphaData[j] = alpha + j++ } - alphaData[i] = alpha + + i0 += m.Stride + i1 += m.Stride } imag := Image{} @@ -276,7 +285,7 @@ func (ih DefaultImageHandler) NewImageFromGoImage(goimg goimage.Image) (*Image, imag.Height = int64(b.Dy()) imag.BitsPerComponent = 8 // RGBA colormap imag.ColorComponents = 3 - imag.Data = data // buf.Bytes() + imag.Data = data imag.hasAlpha = hasAlpha if hasAlpha { @@ -294,6 +303,15 @@ func (ih DefaultImageHandler) NewGrayImageFromGoImage(goimg goimage.Image) (*Ima switch t := goimg.(type) { case *goimage.Gray: m = t + if len(m.Pix) != b.Dx()*b.Dy() { + // Detects when the image Pix data is not of correct format, typically happens + // when m.Stride does not match the image width (extra bytes at end of each line for example). + // Rearrange the data back such that the Pix data is arranged consistently. + // Disadvantage of this is that it doubles the memory use as the data is + // copied when creating the new structure. + m = goimage.NewGray(b) + draw.Draw(m, b, goimg, b.Min, draw.Src) + } default: m = goimage.NewGray(b) draw.Draw(m, b, goimg, b.Min, draw.Src) diff --git a/pdf/model/image_test.go b/pdf/model/image_test.go index 599d7dbd..1e535d2b 100644 --- a/pdf/model/image_test.go +++ b/pdf/model/image_test.go @@ -6,6 +6,9 @@ package model import ( + "image" + "image/color" + "image/draw" "testing" ) @@ -64,3 +67,78 @@ func TestImageResampling(t *testing.T) { t.Errorf("Value != 64 (%d)", img.Data[1]) } } + +func makeTestImage(x, y int, val byte) *image.RGBA { + rect := image.Rect(0, 0, x, y) + m := image.NewRGBA(rect) + + for i := 0; i < x; i++ { + for j := 0; j < y; j++ { + rgba := color.RGBA{R: val, G: val, B: val, A: 0} + m.Set(i, j, rgba) + } + } + return m +} + +func BenchmarkImageCopyingDraw(b *testing.B) { + for n := 0; n < b.N; n++ { + img := makeTestImage(100, 100, byte(n)) + b := img.Bounds() + m := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) + + // Image copying using draw.Draw. + draw.Draw(m, img.Bounds(), img, b.Min, draw.Src) + } +} + +func BenchmarkImageCopyingAtSet(b *testing.B) { + for n := 0; n < b.N; n++ { + img := makeTestImage(100, 100, byte(n)) + b := img.Bounds() + m := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) + + // Image copying with At and direct set. + for x := m.Rect.Min.X; x < m.Rect.Max.X; x++ { + for y := m.Rect.Min.Y; y < m.Rect.Max.Y; y++ { + m.Set(x, y, img.At(x, y)) + } + } + } +} + +func BenchmarkImageCopyingAtDirectSet(b *testing.B) { + for n := 0; n < b.N; n++ { + img := makeTestImage(100, 100, byte(n)) + b := img.Bounds() + m := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) + + // Image copying with At to get colors and set Pix directly. + i := 0 + for x := m.Rect.Min.X; x < m.Rect.Max.X; x++ { + for y := m.Rect.Min.Y; y < m.Rect.Max.Y; y++ { + r, g, b, a := img.At(x, y).RGBA() + + m.Pix[4*i], m.Pix[4*i+1], m.Pix[4*i+2], m.Pix[4*i+3] = byte(r), byte(g), byte(b), byte(a) + i++ + } + } + } +} + +func BenchmarkImageCopyingDirect(b *testing.B) { + for n := 0; n < b.N; n++ { + img := makeTestImage(100, 100, byte(n)) + b := img.Bounds() + m := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) + + // Image copying with direct Pix access for getting and setting. + i := 0 + for x := m.Rect.Min.X; x < m.Rect.Max.X; x++ { + for y := m.Rect.Min.Y; y < m.Rect.Max.Y; y++ { + m.Pix[4*i], m.Pix[4*i+1], m.Pix[4*i+2], m.Pix[4*i+3] = img.Pix[4*i], img.Pix[4*i+1], img.Pix[4*i+2], img.Pix[4*i+3] + i++ + } + } + } +}