From 0c7b0b1d0bcfdab8a9db5723cf778f972423a9cf Mon Sep 17 00:00:00 2001
From: Elias Naur <mail@eliasnaur.com>
Date: Tue, 14 Dec 2021 20:45:13 +0100
Subject: [PATCH] gpu,gpu/headless: plug a resource leak when taking
 screenshots

Ever since commit 8ff654628, the headless implementation has used two
GPU backend (not renderer) instances, one for the renderer and one for
creating the offscreen texture to render into. This arrangment leaks
resources because the backends only clear temporary storage at
BeginFrame, which is not called when reading pixel data from renders.

This change adds an internal constructor, gpu.NewWithDevice, to allow
headless.Window to share its device with the renderer, fixing the leak.
It also makes the code simpler (took me a while to debug this issue); in
fact I'm surprised it even works.

This is not a great fix: it adds an exported yet internal constructor,
and the ownership transfer of the device is surprising enough to warrant
two comments.

Fixes gio#322

Signed-off-by: Elias Naur <mail@eliasnaur.com>
---
 gpu/gpu.go               |  8 ++++++++
 gpu/headless/headless.go | 14 ++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gpu/gpu.go b/gpu/gpu.go
index c07e3f8c..dd2161d2 100644
--- a/gpu/gpu.go
+++ b/gpu/gpu.go
@@ -296,11 +296,19 @@ const (
 	materialTexture
 )
 
+// New creates a GPU for the given API.
 func New(api API) (GPU, error) {
 	d, err := driver.NewDevice(api)
 	if err != nil {
 		return nil, err
 	}
+	return NewWithDevice(d)
+}
+
+// NewWithDevice creates a GPU with a pre-existing device.
+//
+// Note: for internal use only.
+func NewWithDevice(d driver.Device) (GPU, error) {
 	d.BeginFrame(nil, false, image.Point{})
 	defer d.EndFrame()
 	forceCompute := os.Getenv("GIORENDERER") == "forcecompute"
diff --git a/gpu/headless/headless.go b/gpu/headless/headless.go
index 9cbcd4bb..78353cec 100644
--- a/gpu/headless/headless.go
+++ b/gpu/headless/headless.go
@@ -69,8 +69,7 @@ func NewWindow(width, height int) (*Window, error) {
 		ctx:  ctx,
 	}
 	err = contextDo(ctx, func() error {
-		api := ctx.API()
-		dev, err := driver.NewDevice(api)
+		dev, err := driver.NewDevice(ctx.API())
 		if err != nil {
 			return err
 		}
@@ -81,12 +80,13 @@ func NewWindow(width, height int) (*Window, error) {
 			driver.BufferBindingFramebuffer,
 		)
 		if err != nil {
+			dev.Release()
 			return nil
 		}
-		gp, err := gpu.New(api)
+		// Note that the gpu takes ownership of dev.
+		gp, err := gpu.NewWithDevice(dev)
 		if err != nil {
 			fboTex.Release()
-			dev.Release()
 			return err
 		}
 		w.fboTex = fboTex
@@ -112,10 +112,8 @@ func (w *Window) Release() {
 			w.gpu.Release()
 			w.gpu = nil
 		}
-		if w.dev != nil {
-			w.dev.Release()
-			w.dev = nil
-		}
+		// w.dev is owned and freed by w.gpu.
+		w.dev = nil
 		return nil
 	})
 	if w.ctx != nil {