From c6dd0a75d959c81283c7a443f3c601c1c6a8fd5f Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Fri, 30 Nov 2018 00:59:31 +0100 Subject: [PATCH] graphicscommand: Optimize replace-image calls Now ReplacePixels command is called only when necessary. This also ensures that DrawImage must be called after ReplacePixels is called since there is a potential problem that rendering images on a texture without initializing by replacing pixels might cause problems (escpecially on Metal. Perhaps #593 might be related). --- internal/graphicscommand/image.go | 86 +++++++++++++++++++ .../graphicsdriver/opengl/context_desktop.go | 2 + internal/restorable/image.go | 4 +- internal/shareable/shareable.go | 3 +- 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index b148996f0..cbf3e6ddb 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -38,17 +38,37 @@ func MaxImageSize() int { return maxImageSize } +// imageState is a state of an image. +type imageState int + +const ( + // imageStateInit represents that the image is just allocated and not ready for DrawImages. + imageStateInit imageState = iota + + // imageStateReplacePixelsOnly represents that only ReplacePixels is acceptable. + imageStateReplacePixelsOnly + + // imageStateDrawable represents that the image is ready to draw with any commands. + imageStateDrawable + + // imageStateScreen is the special state for screen framebuffer. + // Only copying image on the screen image is allowed. + imageStateScreen +) + // Image represents an image that is implemented with OpenGL. type Image struct { image graphicsdriver.Image width int height int + state imageState } func NewImage(width, height int) *Image { i := &Image{ width: width, height: height, + state: imageStateInit, // The screen image must be inited with ReplacePixels first. } c := &newImageCommand{ result: i, @@ -63,6 +83,7 @@ func NewScreenFramebufferImage(width, height int) *Image { i := &Image{ width: width, height: height, + state: imageStateScreen, } c := &newScreenFramebufferImageCommand{ result: i, @@ -73,6 +94,26 @@ func NewScreenFramebufferImage(width, height int) *Image { return i } +// clearByReplacingPixels clears the image by replacing pixels. +// +// The implementation must use replacing-pixels way instead of drawing polygons, since +// some environments (e.g. Metal) require replacing-pixels way as initialization. +func (i *Image) clearByReplacingPixels() { + if i.state != imageStateInit { + panic("not reached") + } + c := &replacePixelsCommand{ + dst: i, + pixels: make([]byte, 4*i.width*i.height), + x: 0, + y: 0, + width: i.width, + height: i.height, + } + theCommandQueue.Enqueue(c) + i.state = imageStateDrawable +} + func (i *Image) Dispose() { c := &disposeCommand{ target: i, @@ -86,12 +127,41 @@ func (i *Image) Size() (int, int) { } func (i *Image) DrawImage(src *Image, vertices []float32, indices []uint16, clr *affine.ColorM, mode graphics.CompositeMode, filter graphics.Filter) { + switch i.state { + case imageStateInit: + // Before DrawImage, the image must be initialized with ReplacePixels. + // Especially on Metal, the image might be broken when drawing without initializing. + i.clearByReplacingPixels() + case imageStateReplacePixelsOnly: + panic("not reached") + case imageStateDrawable: + // Do nothing + case imageStateScreen: + if mode != graphics.CompositeModeCopy { + panic("not reached") + } + default: + panic("not reached") + } theCommandQueue.EnqueueDrawImageCommand(i, src, vertices, indices, clr, mode, filter) } // Pixels returns the image's pixels. // Pixels might return nil when OpenGL error happens. func (i *Image) Pixels() []byte { + switch i.state { + case imageStateInit: + i.clearByReplacingPixels() + case imageStateReplacePixelsOnly: + // Do nothing + // TODO: Check the region? + case imageStateDrawable: + // Do nothing + case imageStateScreen: + panic("not reached") + default: + panic("not reached") + } c := &pixelsCommand{ result: nil, img: i, @@ -102,6 +172,22 @@ func (i *Image) Pixels() []byte { } func (i *Image) ReplacePixels(p []byte, x, y, width, height int) { + switch i.state { + case imageStateInit: + if x == 0 && y == 0 && width == i.width && height == i.height { + i.state = imageStateDrawable + } else { + i.state = imageStateReplacePixelsOnly + } + case imageStateReplacePixelsOnly: + // Do nothing + case imageStateDrawable: + // Do nothing + case imageStateScreen: + panic("not reached") + default: + panic("not reached") + } pixels := make([]byte, len(p)) copy(pixels, p) c := &replacePixelsCommand{ diff --git a/internal/graphicsdriver/opengl/context_desktop.go b/internal/graphicsdriver/opengl/context_desktop.go index 2118663b5..b4a93d27f 100644 --- a/internal/graphicsdriver/opengl/context_desktop.go +++ b/internal/graphicsdriver/opengl/context_desktop.go @@ -146,6 +146,8 @@ func (c *context) newTexture(width, height int) (textureNative, error) { gl.TexParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST) gl.TexParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE) gl.TexParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_T, gl.CLAMP_TO_EDGE) + // If data is nil, this just allocates memory and the content is undefined. + // https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glTexImage2D.xhtml gl.TexImage2D(gl.TEXTURE_2D, 0, gl.RGBA, int32(width), int32(height), 0, gl.RGBA, gl.UNSIGNED_BYTE, nil) return nil }) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 15216a1f0..14d85dda6 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -78,7 +78,6 @@ func newImageWithoutInit(width, height int, volatile bool) *Image { // Note that Dispose is not called automatically. func NewImage(width, height int, volatile bool) *Image { i := newImageWithoutInit(width, height, volatile) - i.ReplacePixels(nil, 0, 0, width, height) return i } @@ -94,7 +93,6 @@ func NewScreenFramebufferImage(width, height int) *Image { screen: true, } theImages.add(i) - // The screen image doesn't have to be cleared. return i } @@ -163,7 +161,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { vs := graphics.QuadVertices(w2, h2, 0, 0, dw, dh, float32(width)/float32(dw), 0, 0, float32(height)/float32(dh), float32(x), float32(y), - 0, 0, 0, 0) + 1, 1, 1, 1) is := graphics.QuadIndices() i.image.DrawImage(dummyImage.image, vs, is, nil, graphics.CompositeModeCopy, graphics.FilterNearest) } diff --git a/internal/shareable/shareable.go b/internal/shareable/shareable.go index 2e7a847d8..777679c41 100644 --- a/internal/shareable/shareable.go +++ b/internal/shareable/shareable.go @@ -319,7 +319,8 @@ func (i *Image) dispose(markDisposed bool) { if !i.backend.page.IsEmpty() { // As this part can be reused, this should be cleared explicitly. x, y, w, h := i.region() - i.backend.restorable.ReplacePixels(nil, x, y, w, h) + // TODO: Now nil cannot be used here (see the test result). Fix this. + i.backend.restorable.ReplacePixels(make([]byte, 4*w*h), x, y, w, h) return }