From efb6f9c4532958c47ec6bfe21d3b568402aaf5ea Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Wed, 17 Jul 2019 11:33:41 +0900 Subject: [PATCH] graphicscommand: Remove CopyPixels command This is an optimization. This change enables to avoid reading pixels from GPU when extending an image. Updates #897 --- internal/graphicscommand/command.go | 39 ----------------------------- internal/graphicscommand/image.go | 18 ------------- internal/restorable/image.go | 9 ++++--- 3 files changed, 6 insertions(+), 60 deletions(-) diff --git a/internal/graphicscommand/command.go b/internal/graphicscommand/command.go index ae5fe13a4..a10667d19 100644 --- a/internal/graphicscommand/command.go +++ b/internal/graphicscommand/command.go @@ -361,45 +361,6 @@ func (c *replacePixelsCommand) CanMerge(dst, src *Image, color *affine.ColorM, m return false } -type copyPixelsCommand struct { - dst *Image - src *Image -} - -func (c *copyPixelsCommand) String() string { - return fmt.Sprintf("copy-pixels: dst: %p <- src: %p", c.dst, c.src) -} - -func (c *copyPixelsCommand) Exec(indexOffset int) error { - p, err := c.src.image.Pixels() - if err != nil { - return err - } - if c.dst.width < c.src.width || c.dst.height < c.src.height { - return fmt.Errorf("graphicscommand: the destination size (%d, %d) must include the source size (%d, %d)", c.dst.width, c.dst.height, c.src.width, c.src.height) - } - c.dst.image.ReplacePixels(p, 0, 0, c.src.width, c.src.height) - return nil -} - -func (c *copyPixelsCommand) NumVertices() int { - return 0 -} - -func (c *copyPixelsCommand) NumIndices() int { - return 0 -} - -func (c *copyPixelsCommand) AddNumVertices(n int) { -} - -func (c *copyPixelsCommand) AddNumIndices(n int) { -} - -func (c *copyPixelsCommand) CanMerge(dst, src *Image, color *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address) bool { - return false -} - type pixelsCommand struct { result []byte img *Image diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 9dd255825..2fe34a4ac 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -145,24 +145,6 @@ func (i *Image) ReplacePixels(p []byte, x, y, width, height int) { i.lastCommand = lastCommandReplacePixels } -// CopyPixels is basically same as Pixels and ReplacePixels, but reading pixels from GPU is done lazily. -func (i *Image) CopyPixels(src *Image) { - if i.lastCommand == lastCommandDrawTriangles { - if i.width != src.width || i.height != src.height { - panic("graphicscommand: Copy for a part after DrawTriangles is forbidden") - } - } - - c := ©PixelsCommand{ - dst: i, - src: src, - } - theCommandQueue.Enqueue(c) - - // The execution is basically same as replacing pixels. - i.lastCommand = lastCommandReplacePixels -} - func (i *Image) IsInvalidated() bool { if i.screen { // The screen image might not have a texture, and in this case it is impossible to detect whether diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 1ea55894c..3a0aa3b82 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -158,9 +158,9 @@ func (i *Image) Extend(width, height int) *Image { newImg := NewImage(width, height) - // Do not use DrawTriangles here. ReplacePixels will be called on a part of newImg later, and it looked like - // ReplacePixels on a part of image deletes other region that are rendered by DrawTriangles (#593, #758). - newImg.image.CopyPixels(i.image) + if i.basePixels != nil && i.basePixels.pixels != nil { + newImg.image.ReplacePixels(i.basePixels.pixels, 0, 0, w, h) + } // Copy basePixels. newImg.basePixels = &Pixels{ @@ -359,6 +359,9 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { return } + // It looked like ReplacePixels on a part of image deletes other region that are rendered by DrawTriangles + // (#593, #758). + if len(i.drawTrianglesHistory) > 0 { panic("restorable: ReplacePixels for a part after DrawTriangles is forbidden") }