From bdd8916bb12b31012a8cfe86596825e4b3c9fd61 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Fri, 3 Nov 2023 16:05:41 +0900 Subject: [PATCH] ebiten: replace (*ebiten.Shader).Dispose with Deallocate Updates #2808 --- internal/atlas/shader.go | 25 +++++++++----------- internal/ui/shader.go | 5 ++-- shader.go | 15 +++++++++++- shader_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 18 deletions(-) diff --git a/internal/atlas/shader.go b/internal/atlas/shader.go index 6790c0018..bb29a520f 100644 --- a/internal/atlas/shader.go +++ b/internal/atlas/shader.go @@ -38,25 +38,22 @@ func (s *Shader) ensureShader() *restorable.Shader { return s.shader } s.shader = restorable.NewShader(s.ir) - s.ir = nil + runtime.SetFinalizer(s, func(s *Shader) { + // A function from finalizer must not be blocked, but disposing operation can be blocked. + // Defer this operation until it becomes safe. (#913) + appendDeferred(func() { + s.deallocate() + }) + }) return s.shader } -// MarkDisposed marks the shader as disposed. The actual operation is deferred. -// MarkDisposed can be called from finalizers. -// -// A function from finalizer must not be blocked, but disposing operation can be blocked. -// Defer this operation until it becomes safe. (#913) -func (s *Shader) MarkDisposed() { - // As MarkDisposed can be invoked from finalizers, backendsM should not be used. - deferredM.Lock() - deferred = append(deferred, func() { - s.dispose() - }) - deferredM.Unlock() +// Deallocate deallocates the internal state. +func (s *Shader) Deallocate() { + s.deallocate() } -func (s *Shader) dispose() { +func (s *Shader) deallocate() { runtime.SetFinalizer(s, nil) if s.shader == nil { return diff --git a/internal/ui/shader.go b/internal/ui/shader.go index f94f3f10d..0d9031a92 100644 --- a/internal/ui/shader.go +++ b/internal/ui/shader.go @@ -40,9 +40,8 @@ func NewShader(ir *shaderir.Program) *Shader { } } -func (s *Shader) MarkDisposed() { - s.shader.MarkDisposed() - s.shader = nil +func (s *Shader) Deallocate() { + s.shader.Deallocate() } func (s *Shader) AppendUniforms(dst []uint32, uniforms map[string]any) []uint32 { diff --git a/shader.go b/shader.go index e815c3d13..84ddfd82d 100644 --- a/shader.go +++ b/shader.go @@ -50,11 +50,24 @@ func NewShader(src []byte) (*Shader, error) { // Dispose disposes the shader program. // After disposing, the shader is no longer available. +// +// Deprecated: as of v2.7. Use Deallocate instead. func (s *Shader) Dispose() { - s.shader.MarkDisposed() + s.shader.Deallocate() s.shader = nil } +// Deallocate deallocates the internal state of shader. +// Even after Deallocate is called, the shader is still available. +// In this case, the shader's internal state is allocated again. +// +// Usually, you don't have to call Deallocate since the internal state is automatically released by GC. +// However, if you are sure that the shader is no longer used but not sure how this shader object is referred, +// you can call Deallocate to make sure that the internal state is deallocated. +func (s *Shader) Deallocate() { + s.shader.Deallocate() +} + func (s *Shader) appendUniforms(dst []uint32, uniforms map[string]any) []uint32 { return s.shader.AppendUniforms(dst, uniforms) } diff --git a/shader_test.go b/shader_test.go index 071e48ae1..a6f5502c5 100644 --- a/shader_test.go +++ b/shader_test.go @@ -2299,3 +2299,54 @@ func Fragment(dstPos vec4, srcPos vec2, color vec4) vec4 { }) } } + +func TestShaderDeallocate(t *testing.T) { + const w, h = 16, 16 + + dst := ebiten.NewImage(w, h) + s, err := ebiten.NewShader([]byte(`//kage:unit pixels + +package main + +func Fragment(dstPos vec4, srcPos vec2, color vec4) vec4 { + return vec4(1, 0, 0, 1) +} +`)) + if err != nil { + t.Fatal(err) + } + + dst.DrawRectShader(w/2, h/2, s, nil) + + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + got := dst.At(i, j).(color.RGBA) + var want color.RGBA + if i < w/2 && j < h/2 { + want = color.RGBA{R: 0xff, A: 0xff} + } + if got != want { + t.Errorf("dst.At(%d, %d): got: %v, want: %v", i, j, got, want) + } + } + } + + // Even after Deallocate is called, the shader is still available. + s.Deallocate() + + dst.Clear() + dst.DrawRectShader(w/2, h/2, s, nil) + + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + got := dst.At(i, j).(color.RGBA) + var want color.RGBA + if i < w/2 && j < h/2 { + want = color.RGBA{R: 0xff, A: 0xff} + } + if got != want { + t.Errorf("dst.At(%d, %d): got: %v, want: %v", i, j, got, want) + } + } + } +}