glTex[Sub]Image* should not throw an exception, but should cause a GL_INVALID_ENUM error on unrecognized formats or types

4 files changed, 165 insertions, 6 deletions

diff --git a/AUTHORS b/AUTHORS
index 7994f80e..65354d07 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -138,3 +138,4 @@ a license to everyone to use it as detailed in LICENSE.)
 * Guillaume Blanc <guillaumeblanc.sc@gmail.com>
 * Usagi Ito <usagi@WonderRabbitProject.net>
 * Camilo Polymeris <cpolymeris@gmail.com>
+* Jason Green <jason@transgaming.com> (copyright owned by TransGaming, Inc.)
diff --git a/src/library_gl.js b/src/library_gl.js
index 851b01b1..2659a9d9 100644
--- a/src/library_gl.js
+++ b/src/library_gl.js
@@ -431,21 +431,42 @@ var LibraryGL = {
               sizePerPixel = 2;
               break;
             default:
-              throw 'Invalid format (' + format + ')';
+              GL.recordError(0x0500); // GL_INVALID_ENUM
+#if GL_ASSERTIONS
+              Module.printErr('GL_INVALID_ENUM in glTex[Sub]Image, type: ' + type + ', format: ' + format);
+#endif
+              return {
+                pixels: null,
+                internalFormat: 0x0
+              };
           }
           break;
         case 0x1403 /* GL_UNSIGNED_SHORT */:
           if (format == 0x1902 /* GL_DEPTH_COMPONENT */) {
             sizePerPixel = 2;
           } else {
-            throw 'Invalid format (' + format + ')';
+            GL.recordError(0x0500); // GL_INVALID_ENUM
+#if GL_ASSERTIONS
+            Module.printErr('GL_INVALID_ENUM in glTex[Sub]Image, type: ' + type + ', format: ' + format);
+#endif
+            return {
+              pixels: null,
+              internalFormat: 0x0
+            };
           }
           break;
         case 0x1405 /* GL_UNSIGNED_INT */:
           if (format == 0x1902 /* GL_DEPTH_COMPONENT */) {
             sizePerPixel = 4;
           } else {
-            throw 'Invalid format (' + format + ')';
+            GL.recordError(0x0500); // GL_INVALID_ENUM
+#if GL_ASSERTIONS
+            Module.printErr('GL_INVALID_ENUM in glTex[Sub]Image, type: ' + type + ', format: ' + format);
+#endif
+            return {
+              pixels: null,
+              internalFormat: 0x0
+            };
           }
           break;
         case 0x84FA /* UNSIGNED_INT_24_8_WEBGL */:
@@ -468,12 +489,26 @@ var LibraryGL = {
               sizePerPixel = 4*4;
               break;
             default:
-              throw 'Invalid format (' + format + ')';
+              GL.recordError(0x0500); // GL_INVALID_ENUM
+#if GL_ASSERTIONS
+              Module.printErr('GL_INVALID_ENUM in glTex[Sub]Image, type: ' + type + ', format: ' + format);
+#endif
+              return {
+                pixels: null,
+                internalFormat: 0x0
+              };
           }
           internalFormat = GLctx.RGBA;
           break;
         default:
-          throw 'Invalid type (' + type + ')';
+          GL.recordError(0x0500); // GL_INVALID_ENUM
+#if GL_ASSERTIONS
+          Module.printErr('GL_INVALID_ENUM in glTex[Sub]Image, type: ' + type + ', format: ' + format);
+#endif
+          return {
+            pixels: null,
+            internalFormat: 0x0
+          };
       }
       var bytes = GL.computeImageSize(width, height, sizePerPixel, GL.unpackAlignment);
       if (type == 0x1401 /* GL_UNSIGNED_BYTE */) {
@@ -488,7 +523,7 @@ var LibraryGL = {
       return {
         pixels: pixels,
         internalFormat: internalFormat
-      }
+      };
     },
 
 #if GL_FFP_ONLY
diff --git a/tests/gl_teximage.c b/tests/gl_teximage.c
new file mode 100644
index 00000000..9cafce9c
--- /dev/null
+++ b/tests/gl_teximage.c
@@ -0,0 +1,120 @@
+/*
+ * GLES2 test for glTexImage2D parameters
+ *
+ * Original author: Jason Green <jason@transgaming.com>
+ *
+ */
+#include "GLES2/gl2.h"
+#include "SDL/SDL.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <emscripten.h>
+#include <unistd.h>
+
+typedef enum {
+    TEST_STATUS_SUCCESS = 0,
+    TEST_STATUS_FAILURE = 1
+} TestStatus;
+
+/* Report success or failure (1 or 0) to Emscripten's test harness. Also, exit
+ * with the given error code. */
+static void exit_with_status(TestStatus code)
+{
+#ifdef REPORT_RESULT
+    int result = (code == TEST_STATUS_SUCCESS) ? 1 : 0;
+    REPORT_RESULT();
+#endif
+
+    exit(code);
+}
+
+/* Loop over all glGetError() results until GL reports GL_NO_ERROR */
+static void clear_gl_errors()
+{
+    GLenum err;
+    do {
+        err = glGetError();
+    } while (err != GL_NO_ERROR);
+}
+
+int main(int argc, char *argv[])
+{
+    TestStatus passed = TEST_STATUS_SUCCESS;
+    SDL_Surface *screen;
+
+    if (SDL_Init(SDL_INIT_VIDEO) != 0) {
+        printf("SDL_Init failed with %s\n", SDL_GetError());
+        exit_with_status(TEST_STATUS_FAILURE);
+    }
+
+    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
+    screen = SDL_SetVideoMode(640, 480, 16, SDL_OPENGL);
+    if (!screen) {
+        printf("SDL_SetVideoMode failed with %s\n", SDL_GetError());
+        exit_with_status(TEST_STATUS_FAILURE);
+    }
+
+    GLuint texture;
+    glGenTextures(1, &texture);
+
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+
+    // Allocate space for a 32x32 image with 4 bytes per pixel.
+    // No need to fill it with any useful information, as these tests are
+    // only designed to make sure glTexImage2D doesn't crash on unsupported
+    // formats.
+    void* pixels = malloc(4 * 32 * 32);
+    if (pixels == NULL) {
+        printf("Unable to allocate pixel data\n");
+        exit_with_status(TEST_STATUS_FAILURE);
+    }
+
+    // First, try 0xffff for the internal format - should fail
+    glTexImage2D(GL_TEXTURE_2D, 0, 0xffff, 32, 32, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+    GLenum err = glGetError();
+    if (err == GL_NO_ERROR) {
+        printf("internal format == 0xffff succeeded, but should have failed\n");
+        passed = TEST_STATUS_FAILURE;
+    }
+    clear_gl_errors();
+
+    // Try 0xffff for the format - should fail
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, 0xffff, GL_UNSIGNED_BYTE, pixels);
+    err = glGetError();
+    if (err == GL_NO_ERROR) {
+        printf("format == 0xffff succeeded, but should have failed\n");
+        passed = TEST_STATUS_FAILURE;
+    }
+    clear_gl_errors();
+
+    // Try 0xffff for the type - should fail
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, GL_RGBA, 0xffff, pixels);
+    err = glGetError();
+    if (err == GL_NO_ERROR) {
+        printf("type == 0xffff succeeded, but should have failed\n");
+        passed = TEST_STATUS_FAILURE;
+    }
+    clear_gl_errors();
+
+    // Try GL_RGBA/GL_UNSIGNED_BYTE - should succeed
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+    err = glGetError();
+    if (err != GL_NO_ERROR) {
+        printf("GL_RGBA/GL_UNSIGNED_BYTE failed with %x, but should have succeeded\n", err);
+        passed = TEST_STATUS_FAILURE;
+    }
+    clear_gl_errors();
+
+    // Clean up objects
+    glBindTexture(GL_TEXTURE_2D, 0);
+    glDeleteTextures(1, &texture);
+    free(pixels);
+
+    // 'screen' is freed implicitly by SDL_Quit()
+    SDL_Quit();
+
+    exit_with_status(passed);
+}
diff --git a/tests/test_browser.py b/tests/test_browser.py
index aedc926a..c8e07b25 100644
--- a/tests/test_browser.py
+++ b/tests/test_browser.py
@@ -1425,6 +1425,9 @@ keydown(100);keyup(100); // trigger the end
   def test_sdlglshader(self):
     self.btest('sdlglshader.c', reference='sdlglshader.png', args=['-O2', '--closure', '1', '-s', 'LEGACY_GL_EMULATION=1'])
 
+  def test_gl_glteximage(self):
+    self.btest('gl_teximage.c', '1')
+
   def test_gl_ps(self):
     # pointers and a shader
     shutil.copyfile(path_from_root('tests', 'screenshot.png'), os.path.join(self.get_dir(), 'screenshot.png'))