From f80719ef9a081309e46c681e0d568e994b161fd3 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Tue, 26 May 2020 22:50:33 +0900 Subject: [PATCH] driver: Use slices for uniform variables instead of maps Fixes #1172 --- internal/driver/graphics.go | 2 +- internal/graphicscommand/command.go | 6 +-- internal/graphicscommand/image.go | 2 +- internal/graphicscommand/image_test.go | 4 +- internal/graphicsdriver/metal/graphics.go | 2 +- internal/graphicsdriver/opengl/graphics.go | 51 +++++++++++++-------- internal/graphicsdriver/opengl/program.go | 53 +++++++++++----------- internal/restorable/image.go | 10 ++-- internal/restorable/shader_test.go | 29 +++++++----- internal/testing/shader.go | 4 +- 10 files changed, 91 insertions(+), 72 deletions(-) diff --git a/internal/driver/graphics.go b/internal/driver/graphics.go index 797736153..21dd52e0c 100644 --- a/internal/driver/graphics.go +++ b/internal/driver/graphics.go @@ -45,7 +45,7 @@ type Graphics interface { // // uniforms represents a colletion of uniform variables. The values must be one of these types: // float32, []float32, or ImageID. - DrawShader(dst ImageID, shader ShaderID, indexLen int, indexOffset int, mode CompositeMode, uniforms map[int]interface{}) error + DrawShader(dst ImageID, shader ShaderID, indexLen int, indexOffset int, mode CompositeMode, uniforms []interface{}) error } // GraphicsNotReady represents that the graphics driver is not ready for recovering from the context lost. diff --git a/internal/graphicscommand/command.go b/internal/graphicscommand/command.go index ca0395a1a..214d65042 100644 --- a/internal/graphicscommand/command.go +++ b/internal/graphicscommand/command.go @@ -144,7 +144,7 @@ func (q *commandQueue) appendIndices(indices []uint16, offset uint16) { } // EnqueueDrawTrianglesCommand enqueues a drawing-image command. -func (q *commandQueue) EnqueueDrawTrianglesCommand(dst, src *Image, vertices []float32, indices []uint16, color *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms map[int]interface{}) { +func (q *commandQueue) EnqueueDrawTrianglesCommand(dst, src *Image, vertices []float32, indices []uint16, color *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms []interface{}) { if len(indices) > graphics.IndicesNum { panic(fmt.Sprintf("graphicscommand: len(indices) must be <= graphics.IndicesNum but not at EnqueueDrawTrianglesCommand: len(indices): %d, graphics.IndicesNum: %d", len(indices), graphics.IndicesNum)) } @@ -341,7 +341,7 @@ type drawTrianglesCommand struct { filter driver.Filter address driver.Address shader *Shader - uniforms map[int]interface{} + uniforms []interface{} } func (c *drawTrianglesCommand) String() string { @@ -424,7 +424,7 @@ func (c *drawTrianglesCommand) Exec(indexOffset int) error { } if c.shader != nil { - us := map[int]interface{}{} + us := make([]interface{}, len(c.uniforms)) for k, v := range c.uniforms { switch v := v.(type) { case *Image: diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 018bc77e7..0bb11d264 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -151,7 +151,7 @@ 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 // elements for the source image are not used. -func (i *Image) DrawTriangles(src *Image, vertices []float32, indices []uint16, clr *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms map[int]interface{}) { +func (i *Image) DrawTriangles(src *Image, vertices []float32, indices []uint16, clr *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms []interface{}) { if src != nil && src.screen { panic("graphicscommand: the screen image cannot be the rendering source") } diff --git a/internal/graphicscommand/image_test.go b/internal/graphicscommand/image_test.go index f30225a15..da43112df 100644 --- a/internal/graphicscommand/image_test.go +++ b/internal/graphicscommand/image_test.go @@ -93,8 +93,8 @@ func TestShader(t *testing.T) { ir := etesting.ShaderProgramFill(0xff, 0, 0, 0xff) s := NewShader(&ir) - us := map[int]interface{}{ - 0: []float32{w, h}, + us := []interface{}{ + []float32{w, h}, } dst.DrawTriangles(nil, vs, is, nil, driver.CompositeModeSourceOver, 0, 0, s, us) diff --git a/internal/graphicsdriver/metal/graphics.go b/internal/graphicsdriver/metal/graphics.go index 943bda9dd..d3327aedf 100644 --- a/internal/graphicsdriver/metal/graphics.go +++ b/internal/graphicsdriver/metal/graphics.go @@ -763,7 +763,7 @@ func (g *Graphics) NewShader(program *shaderir.Program) (driver.Shader, error) { panic("metal: NewShader is not implemented") } -func (g *Graphics) DrawShader(dst driver.ImageID, shader driver.ShaderID, indexLen int, indexOffset int, mode driver.CompositeMode, uniforms map[int]interface{}) error { +func (g *Graphics) DrawShader(dst driver.ImageID, shader driver.ShaderID, indexLen int, indexOffset int, mode driver.CompositeMode, uniforms []interface{}) error { panic("metal: DrawShader is not implemented") } diff --git a/internal/graphicsdriver/opengl/graphics.go b/internal/graphicsdriver/opengl/graphics.go index d0cca0d8b..3c6b37638 100644 --- a/internal/graphicsdriver/opengl/graphics.go +++ b/internal/graphicsdriver/opengl/graphics.go @@ -16,7 +16,6 @@ package opengl import ( "fmt" - "sort" "github.com/hajimehoshi/ebiten/internal/affine" "github.com/hajimehoshi/ebiten/internal/driver" @@ -167,32 +166,50 @@ func (g *Graphics) Draw(dst, src driver.ImageID, indexLen int, indexOffset int, address: address, }] - uniforms := map[string]interface{}{} + uniforms := []uniformVariable{} vw := destination.framebuffer.width vh := destination.framebuffer.height - uniforms["viewport_size"] = []float32{float32(vw), float32(vh)} + uniforms = append(uniforms, uniformVariable{ + name: "viewport_size", + value: []float32{float32(vw), float32(vh)}, + }) if colorM != nil { // ColorM's elements are immutable. It's OK to hold the reference without copying. esBody, esTranslate := colorM.UnsafeElements() - uniforms["color_matrix_body"] = esBody - uniforms["color_matrix_translation"] = esTranslate + uniforms = append(uniforms, uniformVariable{ + name: "color_matrix_body", + value: esBody, + }, uniformVariable{ + name: "color_matrix_translation", + value: esTranslate, + }) } if filter != driver.FilterNearest { srcW, srcH := source.width, source.height sw := graphics.InternalImageSize(srcW) sh := graphics.InternalImageSize(srcH) - uniforms["source_size"] = []float32{float32(sw), float32(sh)} + uniforms = append(uniforms, uniformVariable{ + name: "source_size", + value: []float32{float32(sw), float32(sh)}, + }) } if filter == driver.FilterScreen { scale := float32(destination.width) / float32(source.width) - uniforms["scale"] = scale + uniforms = append(uniforms, uniformVariable{ + name: "scale", + value: scale, + }) } - uniforms["texture/0"] = source.textureNative + uniforms = append(uniforms, uniformVariable{ + name: "texture", + value: source.textureNative, + textureIndex: 0, + }) if err := g.useProgram(program, uniforms); err != nil { return err @@ -254,7 +271,7 @@ func (g *Graphics) removeShader(shader *Shader) { delete(g.shaders, shader.id) } -func (g *Graphics) DrawShader(dst driver.ImageID, shader driver.ShaderID, indexLen int, indexOffset int, mode driver.CompositeMode, uniforms map[int]interface{}) error { +func (g *Graphics) DrawShader(dst driver.ImageID, shader driver.ShaderID, indexLen int, indexOffset int, mode driver.CompositeMode, uniforms []interface{}) error { d := g.images[dst] s := g.shaders[shader] @@ -265,21 +282,17 @@ func (g *Graphics) DrawShader(dst driver.ImageID, shader driver.ShaderID, indexL } g.context.blendFunc(mode) - us := map[string]interface{}{} - var keys []int - for k := range uniforms { - keys = append(keys, k) - } - sort.Ints(keys) + us := make([]uniformVariable, len(uniforms)) tidx := 0 - for _, k := range keys { - v := uniforms[k] + for k, v := range uniforms { + us[k].name = fmt.Sprintf("U%d", k) switch v := v.(type) { case driver.ImageID: - us[fmt.Sprintf("U%d/%d", k, tidx)] = g.images[v].textureNative + us[k].value = g.images[v].textureNative + us[k].textureIndex = tidx tidx++ default: - us[fmt.Sprintf("U%d", k)] = v + us[k].value = v } } if err := g.useProgram(s.p, us); err != nil { diff --git a/internal/graphicsdriver/opengl/program.go b/internal/graphicsdriver/opengl/program.go index 07fd6f1ed..aa1ea5dd0 100644 --- a/internal/graphicsdriver/opengl/program.go +++ b/internal/graphicsdriver/opengl/program.go @@ -16,8 +16,6 @@ package opengl import ( "fmt" - "strconv" - "strings" "github.com/hajimehoshi/ebiten/internal/driver" "github.com/hajimehoshi/ebiten/internal/graphics" @@ -239,8 +237,14 @@ func areSameFloat32Array(a, b []float32) bool { return true } +type uniformVariable struct { + name string + value interface{} + textureIndex int +} + // useProgram uses the program (programTexture). -func (g *Graphics) useProgram(program program, uniforms map[string]interface{}) error { +func (g *Graphics) useProgram(program program, uniforms []uniformVariable) error { if !g.state.lastProgram.equal(program) { g.context.useProgram(program) if g.state.lastProgram.equal(zeroProgram) { @@ -255,40 +259,35 @@ func (g *Graphics) useProgram(program program, uniforms map[string]interface{}) g.context.activeTexture(0) } - for key, u := range uniforms { - switch u := u.(type) { + for _, u := range uniforms { + switch v := u.value.(type) { case float32: - cached, ok := g.state.lastUniforms[key].(float32) - if ok && cached == u { + cached, ok := g.state.lastUniforms[u.name].(float32) + if ok && cached == v { continue } - g.context.uniformFloat(program, key, u) - g.state.lastUniforms[key] = u + g.context.uniformFloat(program, u.name, v) + g.state.lastUniforms[u.name] = v case []float32: - cached, ok := g.state.lastUniforms[key].([]float32) - if ok && areSameFloat32Array(cached, u) { + cached, ok := g.state.lastUniforms[u.name].([]float32) + if ok && areSameFloat32Array(cached, v) { continue } - g.context.uniformFloats(program, key, u) - g.state.lastUniforms[key] = u + g.context.uniformFloats(program, u.name, v) + g.state.lastUniforms[u.name] = v case textureNative: // Apparently, a texture must be bound every time. The cache is not used here. - tokens := strings.SplitN(key, "/", 2) - if len(tokens) != 2 { - return fmt.Errorf("opengl: a uniform variable name for textures must be '[name]/[index]' but %s", key) + g.context.uniformInt(program, u.name, u.textureIndex) + if g.state.lastActiveTexture != u.textureIndex { + g.context.activeTexture(u.textureIndex) + g.state.lastActiveTexture = u.textureIndex } - idx, err := strconv.Atoi(tokens[1]) - if err != nil { - return fmt.Errorf("opengl: a uniform variable name for textures must be '[name]/[index]' but %s", key) - } - g.context.uniformInt(program, tokens[0], idx) - if g.state.lastActiveTexture != idx { - g.context.activeTexture(idx) - g.state.lastActiveTexture = idx - } - g.context.bindTexture(u) + g.context.bindTexture(v) + case nil: + // TODO: Rather than using nil for skipping, check the availablity of locations at e.g., + // uniformFloats and ignore the error for unavailability. default: - return fmt.Errorf("opengl: unexpected uniform value: %v", u) + return fmt.Errorf("opengl: unexpected uniform value: %v", u.value) } } return nil diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 9e3b3cd2e..248373c5c 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -77,7 +77,7 @@ type drawTrianglesHistoryItem struct { filter driver.Filter address driver.Address shader *Shader - uniforms map[int]interface{} + uniforms []interface{} } // Image represents an image that can be restored when GL context is lost. @@ -343,8 +343,8 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } // convertUniformVariables converts the uniform variables for the lower layer (graphicscommand). -func convertUniformVariables(uniforms map[int]interface{}) map[int]interface{} { - us := map[int]interface{}{} +func convertUniformVariables(uniforms []interface{}) []interface{} { + us := make([]interface{}, len(uniforms)) for k, v := range uniforms { switch v := v.(type) { case *Image: @@ -372,7 +372,7 @@ func convertUniformVariables(uniforms map[int]interface{}) map[int]interface{} { // 9: Color G // 10: Color B // 11: Color Y -func (i *Image) DrawTriangles(img *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms map[int]interface{}) { +func (i *Image) DrawTriangles(img *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms []interface{}) { if i.priority { panic("restorable: DrawTriangles cannot be called on a priority image") } @@ -398,7 +398,7 @@ func (i *Image) DrawTriangles(img *Image, vertices []float32, indices []uint16, } // appendDrawTrianglesHistory appends a draw-image history item to the image. -func (i *Image) appendDrawTrianglesHistory(image *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms map[int]interface{}) { +func (i *Image) appendDrawTrianglesHistory(image *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, shader *Shader, uniforms []interface{}) { if i.stale || i.volatile || i.screen { return } diff --git a/internal/restorable/shader_test.go b/internal/restorable/shader_test.go index 8d22ce81e..0f7ea40d3 100644 --- a/internal/restorable/shader_test.go +++ b/internal/restorable/shader_test.go @@ -35,7 +35,7 @@ func TestShader(t *testing.T) { ir := etesting.ShaderProgramFill(0xff, 0, 0, 0xff) s := NewShader(&ir) - us := map[int]interface{}{ + us := []interface{}{ 0: []float32{1, 1}, } img.DrawTriangles(nil, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero, s, us) @@ -72,9 +72,9 @@ func TestShaderChain(t *testing.T) { ir := etesting.ShaderProgramImages(1) s := NewShader(&ir) for i := 0; i < num-1; i++ { - us := map[int]interface{}{ - 0: []float32{1, 1}, - 1: imgs[i], + us := []interface{}{ + []float32{1, 1}, + imgs[i], } imgs[i+1].DrawTriangles(nil, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero, s, us) } @@ -112,11 +112,18 @@ func TestShaderMultipleSources(t *testing.T) { ir := etesting.ShaderProgramImages(3) s := NewShader(&ir) - us := map[int]interface{}{ - 0: []float32{1, 1}, - 1: srcs[0], - 2: srcs[1], - 5: srcs[2], + // TODO: Now GLSL's optimization eliminates unused uniform variables by the program. Now nils are given, + // but it is strange to care about optimization from users. We should ignore the error when the location is + // not available. + us := []interface{}{ + []float32{1, 1}, + srcs[0], + srcs[1], + nil, // []float32{1, 1}, + nil, // []float32{0, 0, 1, 1}, + srcs[2], + nil, // []float32{1, 1}, + nil, // []float32{0, 0, 1, 1}, } dst.DrawTriangles(nil, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero, s, us) @@ -147,8 +154,8 @@ func TestShaderDispose(t *testing.T) { ir := etesting.ShaderProgramFill(0xff, 0, 0, 0xff) s := NewShader(&ir) - us := map[int]interface{}{ - 0: []float32{1, 1}, + us := []interface{}{ + []float32{1, 1}, } img.DrawTriangles(nil, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero, s, us) diff --git a/internal/testing/shader.go b/internal/testing/shader.go index 2a5ab46cf..29bc8573d 100644 --- a/internal/testing/shader.go +++ b/internal/testing/shader.go @@ -209,7 +209,7 @@ func defaultProgram() shaderir.Program { // ShaderProgramFill returns a shader intermediate representation to fill the frambuffer. // -// Uniform variables: +// Uniform variable's index and its value are: // // 0: the framebuffer size (Vec2) func ShaderProgramFill(r, g, b, a byte) shaderir.Program { @@ -262,7 +262,7 @@ func ShaderProgramFill(r, g, b, a byte) shaderir.Program { // ShaderProgramImages returns a shader intermediate representation to render the frambuffer with the given images. // -// Uniform variables: +// Uniform variables's indices and their values are: // // 0: the framebuffer size (Vec2) // 1: the first images (Sampler2D)