584501 - another round of texture fixes

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

Attachment: another round of texture fixes

note: this is *not yet* importing any WebKit code.
note: the is_power_of_two function there is probably going to be merged anyway with whatever WebKit uses, so no need to argue over where to put it.

This patches mostly fixes validation in texImage2D and texSubImage2D and also does these more nontrivial things:


1) this part fixes a bug, IIUC, in the case of cube maps:

-    if (mBound2DTextures[mActiveTexture])
-        mBound2DTextures[mActiveTexture]->setDimensions(width, height);
+    nsTArray<WebGLObjectRefPtr<WebGLTexture> > &
+        boundTexturesForTarget
+          = target == LOCAL_GL_TEXTURE_2D ?
+             mBound2DTextures : mBoundCubeMapTextures;
+
+    boundTexturesForTarget[mActiveTexture]->setDimensions(width, height);
 

2) the GL constants stored in WebGLContext were of type PRUint32, but that seemed wrong as they were obtained by glGetIntegerv and were often compared to GLsizei which is like PRInt32, so this patch changes them to PRInt32. Relevant part of the patch:

     // some GL constants
-    PRUint32 mGLMaxVertexAttribs;
-    PRUint32 mGLMaxTextureUnits;
-    PRUint32 mGLMaxTextureSize;
-    PRUint32 mGLMaxCubeMapTextureSize;
-    PRUint32 mGLMaxTextureImageUnits;
-    PRUint32 mGLMaxVertexTextureImageUnits;
-    PRUint32 mGLMaxVaryingVectors;
-    PRUint32 mGLMaxFragmentUniformVectors;
-    PRUint32 mGLMaxVertexUniformVectors;
+    PRInt32 mGLMaxVertexAttribs;
+    PRInt32 mGLMaxTextureUnits;
+    PRInt32 mGLMaxTextureSize;
+    PRInt32 mGLMaxCubeMapTextureSize;
+    PRInt32 mGLMaxTextureImageUnits;
+    PRInt32 mGLMaxVertexTextureImageUnits;
+    PRInt32 mGLMaxVaryingVectors;
+    PRInt32 mGLMaxFragmentUniformVectors;
+    PRInt32 mGLMaxVertexUniformVectors;

Comment 1

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

Attachment: another round of texture fixes, v2

This adds validation that the currently bound active WebGLTexture* is non-null. Fixes a crash (that I probably had just introduced). Adds helper function for that:

+    WebGLTexture *activeBoundTextureForTarget(WebGLenum target) {
+        return target == LOCAL_GL_TEXTURE_2D ? mBound2DTextures[mActiveTexture]
+                                             : mBoundCubeMapTextures[mActiveTexture];
+    }

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 463267 [details] [diff] [review]
another round of texture fixes, v2

Looks fine, one style comment:

>+    if(!activeBoundTextureForTarget(target))
>+        return ErrorInvalidOperation("copyTexImage2D: no texture bound to this target");
>+

space after "if"

Comment 3

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

http://hg.mozilla.org/mozilla-central/rev/6852e75fae24