906671 - Use different temporary gl textures in CompositorOGL for different texture target

Status: RESOLVED INVALID

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

Followup of bug 875211.
It is bad to use a gl texture with several shared textures having different texture targets.

Comment 1

[Nicolas Silva [:nical]]

Attachment: temp-tex-target.patch

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 792254 [details] [diff] [review]
temp-tex-target.patch

Review of attachment 792254 [details] [diff] [review]:
-----------------------------------------------------------------

need one more round :-)

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +284,1 @@
>  {

MOZ_ASSERT that index is not too large, check the biggest LOCAL_GL_TEXTUREn constant.

@@ +284,5 @@
>  {
>    size_t index = aTextureUnit - LOCAL_GL_TEXTURE0;
> +
> +  // select the correct array of textures depending on the texture target
> +  nsTArray<GLuint>* textureArrayPtr = 0;

nullptr

@@ +293,5 @@
> +    case LOCAL_GL_TEXTURE_EXTERNAL:
> +      textureArrayPtr = &mTexturesExternal;
> +      break;
> +    default:
> +      MOZ_CRASH("unsupported texture target.");

Please print aTextureTarget as 0x%x. Will help debugging.

@@ +302,2 @@
>    // lazily grow the array of temporary textures
> +  if (array.Length() <= index) {

You want: EnsureLengthAtLeast
http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h#1384

Comment 3

[Nicolas Silva [:nical]]

The new way to bind gralloc buffers (with CompositableQuirks) has the same issue.

Comment 4

[Nicolas Silva [:nical]]

err nevermind, actually it does not has the issue since we bind to a new egl image each time.