From 110ba5403daad54c25e9471047c014b6933a0496 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sun, 26 Feb 2023 01:31:11 +0900 Subject: [PATCH] internal/buffered: remove redundant pixel data if possible --- internal/atlas/image.go | 8 +++++--- internal/atlas/image_test.go | 18 +++++++++--------- internal/atlas/shader_test.go | 4 ++-- internal/buffered/image.go | 12 +++++++++++- internal/restorable/image.go | 12 ++++++------ internal/restorable/images.go | 8 +++----- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/internal/atlas/image.go b/internal/atlas/image.go index 2828b6d36..869790533 100644 --- a/internal/atlas/image.go +++ b/internal/atlas/image.go @@ -522,7 +522,7 @@ func (i *Image) writePixels(pix []byte, x, y, width, height int) { i.backend.restorable.WritePixels(pixb, x, y, pw, ph) } -func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte) error { +func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, x, y, width, height int) error { backendsM.Lock() defer backendsM.Unlock() @@ -538,8 +538,10 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte } ps := i.paddingSize() - ox, oy, w, h := i.regionWithPadding() - return i.backend.restorable.ReadPixels(graphicsDriver, pixels, ox+ps, oy+ps, w-ps*2, h-ps*2) + ox, oy, _, _ := i.regionWithPadding() + x += ox + ps + y += oy + ps + return i.backend.restorable.ReadPixels(graphicsDriver, pixels, x, y, width, height) } // MarkDisposed marks the image as disposed. The actual operation is deferred. diff --git a/internal/atlas/image_test.go b/internal/atlas/image_test.go index 972f31dee..df5e941c1 100644 --- a/internal/atlas/image_test.go +++ b/internal/atlas/image_test.go @@ -125,7 +125,7 @@ func TestEnsureIsolated(t *testing.T) { } pix = make([]byte, 4*size*size) - if err := img4.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := img4.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, size, size); err != nil { t.Fatal(err) } for j := 0; j < size; j++ { @@ -217,7 +217,7 @@ func TestReputOnAtlas(t *testing.T) { } pix = make([]byte, 4*size*size) - if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, size, size); err != nil { t.Fatal(err) } for j := 0; j < size; j++ { @@ -241,7 +241,7 @@ func TestReputOnAtlas(t *testing.T) { } pix = make([]byte, 4*size*size) - if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, size, size); err != nil { t.Fatal(err) } for j := 0; j < size; j++ { @@ -329,7 +329,7 @@ func TestExtend(t *testing.T) { img1.WritePixels(p1, 0, 0, w1, h1) pix0 := make([]byte, 4*w0*h0) - if err := img0.ReadPixels(ui.GraphicsDriverForTesting(), pix0); err != nil { + if err := img0.ReadPixels(ui.GraphicsDriverForTesting(), pix0, 0, 0, w0, h0); err != nil { t.Fatal(err) } for j := 0; j < h0; j++ { @@ -348,7 +348,7 @@ func TestExtend(t *testing.T) { } pix1 := make([]byte, 4*w1*h1) - if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix1); err != nil { + if err := img1.ReadPixels(ui.GraphicsDriverForTesting(), pix1, 0, 0, w1, h1); err != nil { t.Fatal(err) } for j := 0; j < h1; j++ { @@ -395,7 +395,7 @@ func TestWritePixelsAfterDrawTriangles(t *testing.T) { dst.WritePixels(pix, 0, 0, w, h) pix = make([]byte, 4*w*h) - if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, w, h); err != nil { t.Fatal(err) } for j := 0; j < h; j++ { @@ -442,7 +442,7 @@ func TestSmallImages(t *testing.T) { dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src}, vs, is, graphicsdriver.BlendSourceOver, dr, graphicsdriver.Region{}, [graphics.ShaderImageCount - 1][2]float32{}, atlas.NearestFilterShader, nil, false) pix = make([]byte, 4*w*h) - if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, w, h); err != nil { t.Fatal(err) } for j := 0; j < h; j++ { @@ -490,7 +490,7 @@ func TestLongImages(t *testing.T) { dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src}, vs, is, graphicsdriver.BlendSourceOver, dr, graphicsdriver.Region{}, [graphics.ShaderImageCount - 1][2]float32{}, atlas.NearestFilterShader, nil, false) pix = make([]byte, 4*dstW*dstH) - if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, dstW, dstH); err != nil { t.Fatal(err) } for j := 0; j < h; j++ { @@ -717,7 +717,7 @@ func TestImageWritePixelsModify(t *testing.T) { // Check the pixels are the original ones. pix = make([]byte, 4*size*size) - if err := img.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := img.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, size, size); err != nil { t.Fatal(err) } for j := 0; j < size; j++ { diff --git a/internal/atlas/shader_test.go b/internal/atlas/shader_test.go index 1c4f28243..88e160398 100644 --- a/internal/atlas/shader_test.go +++ b/internal/atlas/shader_test.go @@ -48,7 +48,7 @@ func TestShaderFillTwice(t *testing.T) { dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{}, vs, is, graphicsdriver.BlendCopy, dr, graphicsdriver.Region{}, [graphics.ShaderImageCount - 1][2]float32{}, s1, nil, false) pix := make([]byte, 4*w*h) - if err := dst.ReadPixels(g, pix); err != nil { + if err := dst.ReadPixels(g, pix, 0, 0, w, h); err != nil { t.Error(err) } if got, want := (color.RGBA{R: pix[0], G: pix[1], B: pix[2], A: pix[3]}), (color.RGBA{R: 0x80, G: 0x80, B: 0x80, A: 0xff}); got != want { @@ -80,7 +80,7 @@ func TestImageDrawTwice(t *testing.T) { dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src1}, vs, is, graphicsdriver.BlendCopy, dr, graphicsdriver.Region{}, [graphics.ShaderImageCount - 1][2]float32{}, atlas.NearestFilterShader, nil, false) pix := make([]byte, 4*w*h) - if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix); err != nil { + if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), pix, 0, 0, w, h); err != nil { t.Error(err) } if got, want := (color.RGBA{R: pix[0], G: pix[1], B: pix[2], A: pix[3]}), (color.RGBA{R: 0x80, G: 0x80, B: 0x80, A: 0xff}); got != want { diff --git a/internal/buffered/image.go b/internal/buffered/image.go index 638581385..e124a8f2f 100644 --- a/internal/buffered/image.go +++ b/internal/buffered/image.go @@ -21,6 +21,7 @@ import ( "github.com/hajimehoshi/ebiten/v2/internal/atlas" "github.com/hajimehoshi/ebiten/v2/internal/graphics" "github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver" + "github.com/hajimehoshi/ebiten/v2/internal/restorable" "github.com/hajimehoshi/ebiten/v2/internal/shaderir" ) @@ -29,6 +30,7 @@ type Image struct { width int height int + // pixels is valid only when restorable.AlwaysReadPixelsFromGPU() returns true. pixels []byte } @@ -99,9 +101,17 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte return nil } + // If restorable.AlwaysReadPixelsFromGPU() returns false, the pixel data is cached in the restorable package. + if !restorable.AlwaysReadPixelsFromGPU() { + if err := i.img.ReadPixels(graphicsDriver, pixels, x, y, width, height); err != nil { + return err + } + return nil + } + if i.pixels == nil { pix := make([]byte, 4*i.width*i.height) - if err := i.img.ReadPixels(graphicsDriver, pix); err != nil { + if err := i.img.ReadPixels(graphicsDriver, pix, 0, 0, i.width, i.height); err != nil { return err } i.pixels = pix diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 2ca993ab0..40e740f19 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -121,7 +121,7 @@ type Image struct { // staleRegions indicates the regions to restore. // staleRegions is valid only when stale is true. - // staleRegions is not used when alwaysReadPixelsFromGPU() returns true. + // staleRegions is not used when AlwaysReadPixelsFromGPU() returns true. staleRegions []image.Rectangle imageType ImageType @@ -262,8 +262,8 @@ func (i *Image) BasePixelsForTesting() *Pixels { func (i *Image) makeStale(rect image.Rectangle) { i.stale = true - // If ReadPixels always reads pixels from GPU, staleRegions are never used. - if alwaysReadPixelsFromGPU() { + // If ReadPixels Always reads pixels from GPU, staleRegions are never used. + if AlwaysReadPixelsFromGPU() { return } @@ -414,8 +414,8 @@ func (i *Image) appendDrawTrianglesHistory(srcs [graphics.ShaderImageCount]*Imag if i.stale || !i.needsRestoring() { panic("restorable: an image must not be stale or need restoring at appendDrawTrianglesHistory") } - if alwaysReadPixelsFromGPU() { - panic("restorable: appendDrawTrianglesHistory must not be called when alwaysReadPixelsFromGPU() returns true") + if AlwaysReadPixelsFromGPU() { + panic("restorable: appendDrawTrianglesHistory must not be called when AlwaysReadPixelsFromGPU() returns true") } // TODO: Would it be possible to merge draw image history items? @@ -461,7 +461,7 @@ func (i *Image) readPixelsFromGPUIfNeeded(graphicsDriver graphicsdriver.Graphics } func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, x, y, width, height int) error { - if alwaysReadPixelsFromGPU() { + if AlwaysReadPixelsFromGPU() { if err := i.image.ReadPixels(graphicsDriver, pixels, x, y, width, height); err != nil { return err } diff --git a/internal/restorable/images.go b/internal/restorable/images.go index c49a9111f..b409c0f28 100644 --- a/internal/restorable/images.go +++ b/internal/restorable/images.go @@ -16,7 +16,6 @@ package restorable import ( "runtime" - "unsafe" "github.com/hajimehoshi/ebiten/v2/internal/debug" "github.com/hajimehoshi/ebiten/v2/internal/graphicscommand" @@ -33,10 +32,9 @@ func needsRestoring() bool { return forceRestoring || needsRestoringByGraphicsDriver } -// alwaysReadPixelsFromGPU reports whether ReadPixels alwasy reads pixels from GPU or not. -// This is true for low-end machines like 32bit architecture without much memory. -func alwaysReadPixelsFromGPU() bool { - return !needsRestoring() && unsafe.Sizeof(uintptr(0)) < 8 +// AlwaysReadPixelsFromGPU reports whether ReadPixels always reads pixels from GPU or not. +func AlwaysReadPixelsFromGPU() bool { + return !needsRestoring() } // EnableRestoringForTesting forces to enable restoring for testing.