restorable: Reland: Do not record pixels if restoring is not requried

This change also remove the restrictions of operations on
graphicscommand.Image. For example, now DrawTriangles and
ReplacePixels can be mixed on the same image.

Fixes #1022
This commit is contained in:
Hajime Hoshi 2020-08-15 15:11:56 +09:00
parent 257d747f7d
commit ed494dbf59
9 changed files with 64 additions and 47 deletions

View File

@ -26,15 +26,6 @@ import (
"github.com/hajimehoshi/ebiten/v2/internal/png" "github.com/hajimehoshi/ebiten/v2/internal/png"
) )
type lastCommand int
const (
lastCommandNone lastCommand = iota
lastCommandClear
lastCommandDrawTriangles
lastCommandReplacePixels
)
// Image represents an image that is implemented with OpenGL. // Image represents an image that is implemented with OpenGL.
type Image struct { type Image struct {
image driver.Image image driver.Image
@ -51,8 +42,6 @@ type Image struct {
id int id int
bufferedRP []*driver.ReplacePixelsArgs bufferedRP []*driver.ReplacePixelsArgs
lastCommand lastCommand
} }
var nextID = 1 var nextID = 1
@ -151,12 +140,6 @@ func (i *Image) InternalSize() (int, int) {
// If the source image is not specified, i.e., src is nil and there is no image in the uniform variables, the // If the source image is not specified, i.e., src is nil and there is no image in the uniform variables, the
// elements for the source image are not used. // elements for the source image are not used.
func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [graphics.ShaderImageNum - 1][2]float32, vertices []float32, indices []uint16, clr *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, dstRegion, srcRegion driver.Region, shader *Shader, uniforms []interface{}) { func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [graphics.ShaderImageNum - 1][2]float32, vertices []float32, indices []uint16, clr *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, dstRegion, srcRegion driver.Region, shader *Shader, uniforms []interface{}) {
if i.lastCommand == lastCommandNone {
if !i.screen && mode != driver.CompositeModeClear {
panic("graphicscommand: the image must be cleared first")
}
}
if shader == nil { if shader == nil {
// Fast path for rendering without a shader (#1355). // Fast path for rendering without a shader (#1355).
img := srcs[0] img := srcs[0]
@ -178,12 +161,6 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [gra
i.resolveBufferedReplacePixels() i.resolveBufferedReplacePixels()
theCommandQueue.EnqueueDrawTrianglesCommand(i, srcs, offsets, vertices, indices, clr, mode, filter, address, dstRegion, srcRegion, shader, uniforms) theCommandQueue.EnqueueDrawTrianglesCommand(i, srcs, offsets, vertices, indices, clr, mode, filter, address, dstRegion, srcRegion, shader, uniforms)
if i.lastCommand == lastCommandNone && !i.screen {
i.lastCommand = lastCommandClear
} else {
i.lastCommand = lastCommandDrawTriangles
}
} }
// Sync syncs the texture data in CPU and GPU so that Pixels can return the texture data immediately. // Sync syncs the texture data in CPU and GPU so that Pixels can return the texture data immediately.
@ -218,12 +195,6 @@ func (i *Image) Pixels() ([]byte, error) {
} }
func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
// ReplacePixels for a part might invalidate the current image that are drawn by DrawTriangles (#593, #738).
if i.lastCommand == lastCommandDrawTriangles {
if x != 0 || y != 0 || i.width != width || i.height != height {
panic("graphicscommand: ReplacePixels for a part after DrawTriangles is forbidden")
}
}
i.bufferedRP = append(i.bufferedRP, &driver.ReplacePixelsArgs{ i.bufferedRP = append(i.bufferedRP, &driver.ReplacePixelsArgs{
Pixels: pixels, Pixels: pixels,
X: x, X: x,
@ -231,7 +202,6 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
Width: width, Width: width,
Height: height, Height: height,
}) })
i.lastCommand = lastCommandReplacePixels
} }
func (i *Image) IsInvalidated() bool { func (i *Image) IsInvalidated() bool {

View File

@ -69,11 +69,6 @@ func TestClear(t *testing.T) {
} }
func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) { func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Errorf("ReplacePixels must panic but not")
}
}()
const w, h = 32, 32 const w, h = 32, 32
clr := NewImage(w, h) clr := NewImage(w, h)
src := NewImage(w/2, h/2) src := NewImage(w/2, h/2)
@ -89,6 +84,8 @@ func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) {
dst.DrawTriangles([graphics.ShaderImageNum]*Image{clr}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil) dst.DrawTriangles([graphics.ShaderImageNum]*Image{clr}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil)
dst.DrawTriangles([graphics.ShaderImageNum]*Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil) dst.DrawTriangles([graphics.ShaderImageNum]*Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil)
dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1) dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1)
// TODO: Check the result.
} }
func TestShader(t *testing.T) { func TestShader(t *testing.T) {

View File

@ -173,6 +173,14 @@ func (c *context) framebufferPixels(f *framebuffer, width, height int) []byte {
return pixels return pixels
} }
func (c *context) framebufferPixelsToBuffer(f *framebuffer, buffer buffer, width, height int) {
gl.Flush()
c.bindFramebuffer(f.native)
gl.BindBuffer(gl.PIXEL_PACK_BUFFER, uint32(buffer))
gl.ReadPixels(0, 0, int32(width), int32(height), gl.RGBA, gl.UNSIGNED_BYTE, nil)
gl.BindBuffer(gl.PIXEL_PACK_BUFFER, 0)
}
func (c *context) activeTexture(idx int) { func (c *context) activeTexture(idx int) {
gl.ActiveTexture(gl.TEXTURE0 + uint32(idx)) gl.ActiveTexture(gl.TEXTURE0 + uint32(idx))
} }

View File

@ -219,6 +219,17 @@ func (c *context) framebufferPixels(f *framebuffer, width, height int) []byte {
return jsutil.Uint8ArrayToSlice(p) return jsutil.Uint8ArrayToSlice(p)
} }
func (c *context) framebufferPixelsToBuffer(f *framebuffer, buffer buffer, width, height int) {
c.ensureGL()
gl := c.gl
c.bindFramebuffer(f.native)
gl.Call("bindBuffer", gles.PIXEL_PACK_BUFFER, js.Value(buffer))
// void gl.readPixels(x, y, width, height, format, type, GLintptr offset);
gl.Call("readPixels", 0, 0, width, height, gles.RGBA, gles.UNSIGNED_BYTE, 0)
gl.Call("bindBuffer", gles.PIXEL_PACK_BUFFER, nil)
}
func (c *context) activeTexture(idx int) { func (c *context) activeTexture(idx int) {
c.ensureGL() c.ensureGL()
gl := c.gl gl := c.gl

View File

@ -160,6 +160,16 @@ func (c *context) framebufferPixels(f *framebuffer, width, height int) []byte {
return pixels return pixels
} }
func (c *context) framebufferPixelsToBuffer(f *framebuffer, buffer buffer, width, height int) {
c.ctx.Flush()
c.bindFramebuffer(f.native)
c.ctx.BindBuffer(gles.PIXEL_PACK_BUFFER, uint32(buffer))
c.ctx.ReadPixels(nil, 0, 0, int32(width), int32(height), gles.RGBA, gles.UNSIGNED_BYTE)
c.ctx.BindBuffer(gles.PIXEL_PACK_BUFFER, 0)
}
func (c *context) activeTexture(idx int) { func (c *context) activeTexture(idx int) {
c.ctx.ActiveTexture(uint32(gles.TEXTURE0 + idx)) c.ctx.ActiveTexture(uint32(gles.TEXTURE0 + idx))
} }

View File

@ -21,6 +21,7 @@ const (
ELEMENT_ARRAY_BUFFER = 0x8893 ELEMENT_ARRAY_BUFFER = 0x8893
DYNAMIC_DRAW = 0x88E8 DYNAMIC_DRAW = 0x88E8
STREAM_DRAW = 0x88E0 STREAM_DRAW = 0x88E0
PIXEL_PACK_BUFFER = 0x88EB
PIXEL_UNPACK_BUFFER = 0x88EC PIXEL_UNPACK_BUFFER = 0x88EC
SHORT = 0x1402 SHORT = 0x1402
FLOAT = 0x1406 FLOAT = 0x1406

View File

@ -22,6 +22,7 @@ const (
ELEMENT_ARRAY_BUFFER = 0x8893 ELEMENT_ARRAY_BUFFER = 0x8893
DYNAMIC_DRAW = 0x88E8 DYNAMIC_DRAW = 0x88E8
STREAM_DRAW = 0x88E0 STREAM_DRAW = 0x88E0
PIXEL_PACK_BUFFER = 0x88EB
PIXEL_UNPACK_BUFFER = 0x88EC PIXEL_UNPACK_BUFFER = 0x88EC
SHORT = 0x1402 SHORT = 0x1402
FLOAT = 0x1406 FLOAT = 0x1406

View File

@ -135,6 +135,12 @@ func (i *Image) ReplacePixels(args []*driver.ReplacePixelsArgs) {
if i.pbo.equal(*new(buffer)) { if i.pbo.equal(*new(buffer)) {
i.pbo = i.graphics.context.newPixelBufferObject(w, h) i.pbo = i.graphics.context.newPixelBufferObject(w, h)
if i.pbo.equal(*new(buffer)) {
panic("opengl: newPixelBufferObject failed")
}
if i.framebuffer != nil {
i.graphics.context.framebufferPixelsToBuffer(i.framebuffer, i.pbo, i.width, i.height)
}
} }
if i.pbo.equal(*new(buffer)) { if i.pbo.equal(*new(buffer)) {
panic("opengl: newPixelBufferObject failed") panic("opengl: newPixelBufferObject failed")

View File

@ -170,19 +170,29 @@ func (i *Image) Extend(width, height int) *Image {
panic(fmt.Sprintf("restorable: the original size (%d, %d) cannot be extended to (%d, %d)", i.width, i.height, width, height)) panic(fmt.Sprintf("restorable: the original size (%d, %d) cannot be extended to (%d, %d)", i.width, i.height, width, height))
} }
if i.stale {
panic("restorable: Extend at a stale image is forbidden")
}
if len(i.drawTrianglesHistory) > 0 {
panic("restorable: Extend after DrawTriangles is forbidden")
}
newImg := NewImage(width, height) newImg := NewImage(width, height)
newImg.SetVolatile(i.volatile) newImg.SetVolatile(i.volatile)
i.basePixels.Apply(newImg.image)
// Use DrawTriangles instead of ReplacePixels because the image i might be stale and not have its pixels
// information.
srcs := [graphics.ShaderImageNum]*Image{i}
var offsets [graphics.ShaderImageNum - 1][2]float32
sw, sh := i.image.InternalSize()
vs := quadVertices(0, 0, float32(sw), float32(sh), 0, 0, float32(sw), float32(sh), 1, 1, 1, 1)
is := graphics.QuadIndices()
dr := driver.Region{
X: 0,
Y: 0,
Width: float32(sw),
Height: float32(sh),
}
newImg.DrawTriangles(srcs, offsets, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil)
// Overwrite the history as if the image newImg is created only by ReplacePixels. Now drawTrianglesHistory
// and basePixels cannot be mixed.
newImg.drawTrianglesHistory = nil
newImg.basePixels = i.basePixels newImg.basePixels = i.basePixels
newImg.stale = i.stale
i.Dispose() i.Dispose()
@ -287,6 +297,11 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height) i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height)
} }
if !needsRestoring() || i.screen || i.volatile {
i.makeStale()
return
}
if x == 0 && y == 0 && width == w && height == h { if x == 0 && y == 0 && width == w && height == h {
if pixels != nil { if pixels != nil {
i.basePixels.AddOrReplace(pixels, 0, 0, w, h) i.basePixels.AddOrReplace(pixels, 0, 0, w, h)
@ -298,9 +313,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
return return
} }
// It looked like ReplacePixels on a part of image deletes other region that are rendered by DrawTriangles // drawTrianglesHistory and basePixels cannot be mixed.
// (#593, #758).
if len(i.drawTrianglesHistory) > 0 { if len(i.drawTrianglesHistory) > 0 {
panic("restorable: ReplacePixels for a part after DrawTriangles is forbidden") panic("restorable: ReplacePixels for a part after DrawTriangles is forbidden")
} }