Skip to content

Commit

Permalink
gles: Fixes for unused VAAs that have no data.
Browse files Browse the repository at this point in the history
On some drivers having a vertex attribute array enabled with no bound data will cause `GL_INVALID_OPERATION` errors, even though that vertex attribute is not used by the program.

This is a compat workaround for this case where we temporarily disable the vertex attribute array if we believe the program does not use it.
  • Loading branch information
ben-clayton committed Dec 8, 2017
1 parent bad68f3 commit 3b676df
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
33 changes: 33 additions & 0 deletions gapis/api/gles/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ func compat(ctx context.Context, device *device.Instance) (transform.Transformer
return
}

if cmd.CmdFlags(ctx, id, s).IsDrawCall() {
t := newTweaker(out, dID, cb)
disableUnusedAttribArrays(ctx, t)
defer t.revert(ctx)
}

switch cmd := cmd.(type) {
case *GlBindBuffer:
if cmd.Buffer == 0 {
Expand Down Expand Up @@ -1289,6 +1295,33 @@ func canUsePrecompiledShader(c *Context, d *device.OpenGLDriver) bool {
return c.Constants.Vendor == d.Vendor && c.Constants.Version == d.Version
}

// disableUnusedAttribArrays disables all vertex attribute arrays that are not
// used by the currently bound program. This is a compatibility fix for devices
// that will error with GL_INVALID_OPERATION if there's an enabled (but unused)
// vertex attribute array that has no array data when drawing. AFAICT, this
// particular behavior is undefined according to the spec.
func disableUnusedAttribArrays(ctx context.Context, t *tweaker) {
p := t.c.Bound.Program
if p == nil || p.ActiveResources == nil {
return
}
inputs := p.ActiveResources.ProgramInputs
used := make([]bool, t.c.Constants.MaxVertexAttribBindings)
for _, input := range inputs.Range() {
for _, l := range input.Locations.Range() {
if l >= 0 && l < GLint(len(used)) {
used[l] = true
}
}
}

for l, arr := range t.c.Bound.VertexArray.VertexAttributeArrays.Range() {
if arr.Enabled == GLboolean_GL_TRUE && l < AttributeLocation(len(used)) && !used[l] {
t.glDisableVertexAttribArray(ctx, l)
}
}
}

// It is a no-op to delete objects that do not exist.
// GAPID uses a shared context for replay - while its fine to delete ids that do
// not exist in this context, they might exist in another. Strip out any ids
Expand Down
9 changes: 9 additions & 0 deletions gapis/api/gles/tweaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ func (t *tweaker) glDisable(ctx context.Context, name GLenum) {
}
}

func (t *tweaker) glDisableVertexAttribArray(ctx context.Context, l AttributeLocation) {
arr := t.c.Bound.VertexArray.VertexAttributeArrays.Get(l)
if arr != nil && arr.Enabled == GLboolean_GL_TRUE {
t.doAndUndo(ctx,
t.cb.GlDisableVertexAttribArray(l),
t.cb.GlEnableVertexAttribArray(l))
}
}

func (t *tweaker) glDepthMask(ctx context.Context, v GLboolean) {
if o := t.c.Pixel.DepthWritemask; o != v {
t.doAndUndo(ctx,
Expand Down

0 comments on commit 3b676df

Please sign in to comment.