graphicsdriver/opengl: Fix suspicious GL function calls

Before this change, the pixel object buffer is unbound just after
getting a pointer by glMapBuffer. This seemed suspicious.

This change fixes to do all pixel manipulations once between
glMapBuffer and glUnmapBuffer without changing a bound buffer.

This might fix a wrong rendering on some machines, but I am not
sure.

Updates #993

internal/graphicsdriver/opengl/context_desktop.go

@@ -530,7 +530,6 @@ func (c *context) mapPixelBuffer(buffer buffer) unsafe.Pointer {
 	_ = c.t.Call(func() error {
 		gl.BindBuffer(gl.PIXEL_UNPACK_BUFFER, uint32(buffer))
 		ptr = gl.MapBuffer(gl.PIXEL_UNPACK_BUFFER, gl.WRITE_ONLY)
-		gl.BindBuffer(gl.PIXEL_UNPACK_BUFFER, 0)
 		return nil
 	})
 	return ptr
@@ -538,7 +537,6 @@ func (c *context) mapPixelBuffer(buffer buffer) unsafe.Pointer {
 
 func (c *context) unmapPixelBuffer(buffer buffer, t textureNative, width, height int) {
 	_ = c.t.Call(func() error {
-		gl.BindBuffer(gl.PIXEL_UNPACK_BUFFER, uint32(buffer))
 		gl.UnmapBuffer(gl.PIXEL_UNPACK_BUFFER)
 		return nil
 	})

internal/graphicsdriver/opengl/image.go

@@ -18,6 +18,14 @@ import (
 	"github.com/hajimehoshi/ebiten/internal/graphics"
 )
 
+type bufferedRP struct {
+	pixels []byte
+	x      int
+	y      int
+	width  int
+	height int
+}
+
 type Image struct {
 	driver        *Driver
 	textureNative textureNative
@@ -26,6 +34,8 @@ type Image struct {
 	width         int
 	height        int
 	screen        bool
+
+	bufferedRPs []bufferedRP
 }
 
 func (i *Image) IsInvalidated() bool {
@@ -33,7 +43,6 @@ func (i *Image) IsInvalidated() bool {
 }
 
 func (i *Image) Dispose() {
-	thePBOState.ensurePBOUnmapped()
 	if i.pbo != *new(buffer) {
 		i.driver.context.deleteBuffer(i.pbo)
 	}
@@ -46,6 +55,7 @@ func (i *Image) Dispose() {
 }
 
 func (i *Image) SetAsDestination() {
+	i.resolveReplacePixels()
 	i.driver.state.destination = i
 }
 
@@ -58,7 +68,7 @@ func (i *Image) setViewport() error {
 }
 
 func (i *Image) Pixels() ([]byte, error) {
-	thePBOState.ensurePBOUnmapped()
+	i.resolveReplacePixels()
 	if err := i.ensureFramebuffer(); err != nil {
 		return nil, err
 	}
@@ -96,6 +106,29 @@ func (i *Image) ReplacePixels(p []byte, x, y, width, height int) {
 		panic("opengl: ReplacePixels cannot be called on the screen, that doesn't have a texture")
 	}
 
+	i.bufferedRPs = append(i.bufferedRPs, bufferedRP{
+		pixels: p,
+		x:      x,
+		y:      y,
+		width:  width,
+		height: height,
+	})
+}
+
+func (i *Image) SetAsSource() {
+	i.resolveReplacePixels()
+	i.driver.state.source = i
+}
+
+func (i *Image) resolveReplacePixels() {
+	if len(i.bufferedRPs) == 0 {
+		return
+	}
+
+	defer func() {
+		i.bufferedRPs = nil
+	}()
+
 	// glFlush is necessary on Android.
 	// glTexSubImage2D didn't work without this hack at least on Nexus 5x and NuAns NEO [Reloaded] (#211).
 	if i.driver.drawCalled {
@@ -103,14 +136,16 @@ func (i *Image) ReplacePixels(p []byte, x, y, width, height int) {
 	}
 	i.driver.drawCalled = false
 
-	if canUsePBO {
+	if !canUsePBO {
-		thePBOState.mapPBOIfNecessary(i)
+		for _, rp := range i.bufferedRPs {
-		thePBOState.draw(p, x, y, width, height)
+			i.driver.context.texSubImage2D(i.textureNative, rp.pixels, rp.x, rp.y, rp.width, rp.height)
-	} else {
+		}
-		i.driver.context.texSubImage2D(i.textureNative, p, x, y, width, height)
+		return
 	}
-}
 
-func (i *Image) SetAsSource() {
+	thePBOState.mapPBO(i)
-	i.driver.state.source = i
+	for _, rp := range i.bufferedRPs {
+		thePBOState.draw(rp.pixels, rp.x, rp.y, rp.width, rp.height)
+	}
+	thePBOState.unmapPBO()
 }

internal/graphicsdriver/opengl/pbo_desktop.go

@@ -36,13 +36,7 @@ type pboState struct {
 
 var thePBOState pboState
 
-func (s *pboState) mapPBOIfNecessary(img *Image) {
+func (s *pboState) mapPBO(img *Image) {
-	if s.image == img {
-		return
-	}
-
-	s.ensurePBOUnmapped()
-
 	if img.pbo == *new(buffer) {
 		w, h := graphics.InternalImageSize(img.width), graphics.InternalImageSize(img.height)
 		img.pbo = img.driver.context.newPixelBufferObject(w, h)
@@ -73,11 +67,7 @@ func (s *pboState) draw(pix []byte, x, y, width, height int) {
 	runtime.KeepAlive(mapped)
 }
 
-func (s *pboState) ensurePBOUnmapped() {
+func (s *pboState) unmapPBO() {
-	if s.mappedPBO == nil {
-		return
-	}
-
 	i := s.image
 	w, h := graphics.InternalImageSize(i.width), graphics.InternalImageSize(i.height)
 	i.driver.context.unmapPixelBuffer(i.pbo, i.textureNative, w, h)

internal/graphicsdriver/opengl/pbo_notdesktop.go

@@ -22,7 +22,7 @@ type pboState struct{}
 
 var thePBOState pboState
 
-func (s *pboState) mapPBOIfNecessary(img *Image) {
+func (s *pboState) mapPBO(img *Image) {
 	panic("opengl: PBO is not available in this environment")
 }
 
@@ -30,6 +30,6 @@ func (s *pboState) draw(pix []byte, x, y, width, height int) {
 	panic("opengl: PBO is not available in this environment")
 }
 
-func (s *pboState) ensurePBOUnmapped() {
+func (s *pboState) unmapPBO() {
-	// Do nothing
+	panic("opengl: PBO is not available in this environment")
 }

internal/graphicsdriver/opengl/program.go

@@ -265,8 +265,6 @@ func (d *Driver) useProgram(mode driver.CompositeMode, colorM *affine.ColorM, fi
 		panic("source image is not set")
 	}
 
-	thePBOState.ensurePBOUnmapped()
-
 	if err := destination.setViewport(); err != nil {
 		return err
 	}