584840 - Fix texParameter methods

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

Attachment: Fix texParameter

* validate parameters
* don't use NativeJSContext anymore
* remove from quickstubs blacklist

Comment 1

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

Comment on attachment 463271 [details] [diff] [review]
Fix texParameter

># HG changeset patch
># Parent 280bc8db9c03b94525a1e97c63d5409da6a5fefe
>diff --git a/content/canvas/src/WebGLContextGL.cpp b/content/canvas/src/WebGLContextGL.cpp
>--- a/content/canvas/src/WebGLContextGL.cpp
>+++ b/content/canvas/src/WebGLContextGL.cpp
>@@ -1787,144 +1787,84 @@ WebGLContext::GetProgramInfoLog(nsIWebGL
> 
>     log.SetLength(k);
> 
>     CopyASCIItoUTF16(log, retval);
> 
>     return NS_OK;
> }
> 
>-/* XXX fix */
>-/* void texParameter (); */
> NS_IMETHODIMP
> WebGLContext::TexParameterf(WebGLenum target, WebGLenum pname, WebGLfloat param)
> {
>-    NativeJSContext js;
>-    if (NS_FAILED(js.error))
>-        return js.error;
>-
>-    if (js.argc != 3)
>-        return NS_ERROR_DOM_SYNTAX_ERR;
>-
>-    MakeContextCurrent();
>-
>-    gl->fTexParameterf (target, pname, param);
>-
>-    return NS_OK;
>+    return ErrorInvalidOperation("texParameterf is useless in the current WebGL and OpenGL ES spec, "
>+                                 "as there are no floating-point texture parameters.");
> }
>+
> NS_IMETHODIMP
> WebGLContext::TexParameteri(WebGLenum target, WebGLenum pname, WebGLint param)
> {
>-    NativeJSContext js;
>-    if (NS_FAILED(js.error))
>-        return js.error;
>-
>-    if (js.argc != 3)
>-        return NS_ERROR_DOM_SYNTAX_ERR;
>+    if (!ValidateTextureTargetEnum(target, "texParameteri: target"))
>+        return NS_OK;
>+
>+    PRBool pname_and_param_are_incompatible = PR_FALSE;
>+
>+    switch (pname) {
>+        case LOCAL_GL_TEXTURE_MIN_FILTER:
>+            switch (param) {
>+                case LOCAL_GL_NEAREST:
>+                case LOCAL_GL_LINEAR:
>+                case LOCAL_GL_NEAREST_MIPMAP_NEAREST:
>+                case LOCAL_GL_LINEAR_MIPMAP_NEAREST:
>+                case LOCAL_GL_NEAREST_MIPMAP_LINEAR:
>+                case LOCAL_GL_LINEAR_MIPMAP_LINEAR:
>+                    break;
>+                default:
>+                    pname_and_param_are_incompatible = PR_TRUE;
>+            }
>+            break;
>+        case LOCAL_GL_TEXTURE_MAG_FILTER:
>+            switch (param) {
>+                case LOCAL_GL_NEAREST:
>+                case LOCAL_GL_LINEAR:
>+                    break;
>+                default:
>+                    pname_and_param_are_incompatible = PR_TRUE;
>+            }
>+            break;
>+        case LOCAL_GL_TEXTURE_WRAP_S:
>+        case LOCAL_GL_TEXTURE_WRAP_T:
>+            switch (param) {
>+                case LOCAL_GL_CLAMP_TO_EDGE:
>+                case LOCAL_GL_MIRRORED_REPEAT:
>+                case LOCAL_GL_REPEAT:
>+                    break;
>+                default:
>+                    pname_and_param_are_incompatible = PR_TRUE;
>+            }
>+            break;
>+        default:
>+            return ErrorInvalidEnumInfo("texParameteri: pname", pname);
>+    }
>+
>+    if (pname_and_param_are_incompatible)
>+        return ErrorInvalidEnumInfo("texParameteri: param", param);
>+
>+    WebGLTexture *boundTexture =
>+        target == LOCAL_GL_TEXTURE_2D ? mBound2DTextures[mActiveTexture] : mBoundCubeMapTextures[mActiveTexture];
>+
>+    if (!boundTexture)
>+        return ErrorInvalidOperation("texParameteri: no texture bound for this target and active texture index");
> 
>     MakeContextCurrent();
>-
>     gl->fTexParameteri (target, pname, param);
> 
>     return NS_OK;
> }
> 
>-#if 0
>-NS_IMETHODIMP
>-WebGLContext::TexParameter()
>-{
>-    NativeJSContext js;
>-    if (NS_FAILED(js.error))
>-        return js.error;
>-
>-    if (js.argc != 3)
>-        return NS_ERROR_DOM_SYNTAX_ERR;
>-
>-    jsuint targetVal;
>-    jsuint pnameVal;
>-    if (!::JS_ValueToECMAUint32(js.ctx, js.argv[0], &targetVal) ||
>-        !::JS_ValueToECMAUint32(js.ctx, js.argv[1], &pnameVal))
>-        return NS_ERROR_DOM_SYNTAX_ERR;
>-
>-    if (targetVal != LOCAL_GL_TEXTURE_2D &&
>-        targetVal != LOCAL_GL_TEXTURE_CUBE_MAP)
>-    {
>-        return NS_ERROR_DOM_SYNTAX_ERR;
>-    }
>-
>-    MakeContextCurrent();
>-    switch (pnameVal) {
>-        case LOCAL_GL_TEXTURE_MIN_FILTER: {
>-            jsuint ival;
>-            if (!::JS_ValueToECMAUint32(js.ctx, js.argv[2], &ival) ||
>-                (ival != LOCAL_GL_NEAREST &&
>-                 ival != LOCAL_GL_LINEAR &&
>-                 ival != LOCAL_GL_NEAREST_MIPMAP_NEAREST &&
>-                 ival != LOCAL_GL_LINEAR_MIPMAP_NEAREST &&
>-                 ival != LOCAL_GL_NEAREST_MIPMAP_LINEAR &&
>-                 ival != LOCAL_GL_LINEAR_MIPMAP_LINEAR))
>-                return NS_ERROR_DOM_SYNTAX_ERR;
>-            gl->fTexParameteri (targetVal, pnameVal, ival);
>-        }
>-            break;
>-        case LOCAL_GL_TEXTURE_MAG_FILTER: {
>-            jsuint ival;
>-            if (!::JS_ValueToECMAUint32(js.ctx, js.argv[2], &ival) ||
>-                (ival != LOCAL_GL_NEAREST &&
>-                 ival != LOCAL_GL_LINEAR))
>-                return NS_ERROR_DOM_SYNTAX_ERR;
>-            gl->fTexParameteri (targetVal, pnameVal, ival);
>-        }
>-            break;
>-        case LOCAL_GL_TEXTURE_WRAP_S:
>-        case LOCAL_GL_TEXTURE_WRAP_T: {
>-            jsuint ival;
>-            if (!::JS_ValueToECMAUint32(js.ctx, js.argv[2], &ival) ||
>-                (ival != LOCAL_GL_CLAMP &&
>-                 ival != LOCAL_GL_CLAMP_TO_EDGE &&
>-                 ival != LOCAL_GL_REPEAT))
>-                return NS_ERROR_DOM_SYNTAX_ERR;
>-            gl->fTexParameteri (targetVal, pnameVal, ival);
>-        }
>-            break;
>-        case LOCAL_GL_GENERATE_MIPMAP: {
>-            jsuint ival;
>-            if (js.argv[2] == JSVAL_TRUE)
>-                ival = 1;
>-            else if (js.argv[2] == JSVAL_FALSE)
>-                ival = 0;
>-            else if (!::JS_ValueToECMAUint32(js.ctx, js.argv[2], &ival) ||
>-                     (ival != 0 && ival != 1))
>-                return NS_ERROR_DOM_SYNTAX_ERR;
>-            gl->fTexParameteri (targetVal, pnameVal, ival);
>-        }
>-            break;
>-        case LOCAL_GL_TEXTURE_MAX_ANISOTROPY_EXT: {
>-#if 0
>-            if (GLEW_EXT_texture_filter_anisotropic) {
>-                jsdouble dval;
>-                if (!::JS_ValueToNumber(js.ctx, js.argv[2], &dval))
>-                    return NS_ERROR_DOM_SYNTAX_ERR;
>-                gl->fTexParameterf (targetVal, pnameVal, (float) dval);
>-            } else {
>-                return NS_ERROR_NOT_IMPLEMENTED;
>-            }
>-#else
>-            return NS_ERROR_NOT_IMPLEMENTED;
>-#endif
>-        }
>-            break;
>-        default:
>-            return NS_ERROR_DOM_SYNTAX_ERR;
>-    }
>-
>-    return NS_OK;
>-}
>-#endif
>-
> NS_IMETHODIMP
> WebGLContext::GetTexParameter(WebGLenum target, WebGLenum pname, nsIVariant **retval)
> {
>     nsCOMPtr<nsIWritableVariant> wrval = do_CreateInstance("@mozilla.org/variant;1");
>     NS_ENSURE_TRUE(wrval, NS_ERROR_FAILURE);
> 
>     MakeContextCurrent();
> 
>diff --git a/js/src/xpconnect/src/dom_quickstubs.qsconf b/js/src/xpconnect/src/dom_quickstubs.qsconf
>--- a/js/src/xpconnect/src/dom_quickstubs.qsconf
>+++ b/js/src/xpconnect/src/dom_quickstubs.qsconf
>@@ -443,18 +443,16 @@ members = [
>     # custom-quickstubbed.
>     '-nsICanvasRenderingContextWebGL.getActiveAttrib',
>     '-nsICanvasRenderingContextWebGL.getActiveUniform',
>     '-nsICanvasRenderingContextWebGL.getParameter',
>     '-nsICanvasRenderingContextWebGL.getBufferParameter',
>     '-nsICanvasRenderingContextWebGL.getFramebufferAttachmentParameter',
>     '-nsICanvasRenderingContextWebGL.getRenderbufferParameter',
>     '-nsICanvasRenderingContextWebGL.getProgramParameter',
>-    '-nsICanvasRenderingContextWebGL.texParameterf',
>-    '-nsICanvasRenderingContextWebGL.texParameteri',
>     '-nsICanvasRenderingContextWebGL.getUniform',
>     '-nsICanvasRenderingContextWebGL.getVertexAttrib',
>     '-nsICanvasRenderingContextWebGL.getShaderParameter',
> 
>     # dom/indexedDB
>     'nsIIDBCursor.*',
>     'nsIIDBDatabase.*',
>     'nsIIDBDatabaseException.*',

Comment 2

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

Attachment: Fix texParameter, take 2, now convert between int and float

Following this discussion:
https://www.khronos.org/webgl/public-mailing-list/archives/1008/msg00014.html

Comment 3

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

http://hg.mozilla.org/mozilla-central/rev/6774a67212e0