Merge pull request #2118 from inolen/sdl_lazy_init_fix

initialize pixel buffer inside of makeSurface for SDL_SWSURFACE

3 files changed, 52 insertions, 20 deletions

diff --git a/src/library_sdl.js b/src/library_sdl.js
index c1cb36d0..a4be849e 100644
--- a/src/library_sdl.js
+++ b/src/library_sdl.js
@@ -214,26 +214,33 @@ var LibrarySDL = {
 
     makeSurface: function(width, height, flags, usePageCanvas, source, rmask, gmask, bmask, amask) {
       flags = flags || 0;
-      var surf = _malloc({{{ C_STRUCTS.SDL_Surface.__size__ }}});  // SDL_Surface has 15 fields of quantum size
-      var pixelFormat = _malloc({{{ C_STRUCTS.SDL_PixelFormat.__size__ }}});
-      flags |= 1; // SDL_HWSURFACE - this tells SDL_MUSTLOCK that this needs to be locked
+      var is_SDL_HWSURFACE = flags & 0x00000001;
+      var is_SDL_HWPALETTE = flags & 0x00200000;
+      var is_SDL_OPENGL = flags & 0x04000000;
 
+      var surf = _malloc({{{ C_STRUCTS.SDL_Surface.__size__ }}});
+      var pixelFormat = _malloc({{{ C_STRUCTS.SDL_PixelFormat.__size__ }}});
       //surface with SDL_HWPALETTE flag is 8bpp surface (1 byte)
-      var is_SDL_HWPALETTE = flags & 0x00200000;  
       var bpp = is_SDL_HWPALETTE ? 1 : 4;
- 
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.flags, 'flags', 'i32') }}};        // SDL_Surface.flags
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.format, 'pixelFormat', 'void*') }}};// SDL_Surface.format TODO
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.w, 'width', 'i32') }}};        // SDL_Surface.w
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.h, 'height', 'i32') }}};       // SDL_Surface.h
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.pitch, 'width * bpp', 'i32') }}};      // SDL_Surface.pitch, assuming RGBA or indexed for now,
-                                                                               // since that is what ImageData gives us in browsers
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.pixels, '0', 'void*') }}};     // SDL_Surface.pixels, lazily initialized inside of SDL_LockSurface
-      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.clip_rect, '0', 'i32*') }}};     // SDL_Surface.offset
+      var buffer = 0;
+
+      // preemptively initialize this for software surfaces,
+      // otherwise it will be lazily initialized inside of SDL_LockSurface
+      if (!is_SDL_HWSURFACE && !is_SDL_OPENGL) {
+        buffer = _malloc(width * height * 4);
+      }
 
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.flags, 'flags', 'i32') }}};
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.format, 'pixelFormat', 'void*') }}};
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.w, 'width', 'i32') }}};
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.h, 'height', 'i32') }}};
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.pitch, 'width * bpp', 'i32') }}};  // assuming RGBA or indexed for now,
+                                                                                        // since that is what ImageData gives us in browsers
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.pixels, 'buffer', 'void*') }}};
+      {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.clip_rect, '0', 'i32*') }}};
       {{{ makeSetValue('surf', C_STRUCTS.SDL_Surface.refcount, '1', 'i32') }}};
 
-      {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.format, cDefine('SDL_PIXELFORMAT_RGBA8888'), 'i32') }}};// SDL_PIXELFORMAT_RGBA8888
+      {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.format, cDefine('SDL_PIXELFORMAT_RGBA8888'), 'i32') }}};
       {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.palette, '0', 'i32') }}};// TODO
       {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.BitsPerPixel, 'bpp * 8', 'i8') }}};
       {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.BytesPerPixel, 'bpp', 'i8') }}};
@@ -244,8 +251,7 @@ var LibrarySDL = {
       {{{ makeSetValue('pixelFormat', C_STRUCTS.SDL_PixelFormat.Amask, 'amask || 0xff000000', 'i32') }}};
 
       // Decide if we want to use WebGL or not
-      var useWebGL = (flags & 0x04000000) != 0; // SDL_OPENGL
-      SDL.GL = SDL.GL || useWebGL;
+      SDL.GL = SDL.GL || is_SDL_OPENGL;
       var canvas;
       if (!usePageCanvas) {
         if (SDL.canvasPool.length > 0) {
@@ -265,7 +271,7 @@ var LibrarySDL = {
         stencil: (SDL.glAttributes[7 /*SDL_GL_STENCIL_SIZE*/] > 0)
       };
       
-      var ctx = Browser.createContext(canvas, useWebGL, usePageCanvas, webGLContextAttributes);
+      var ctx = Browser.createContext(canvas, is_SDL_OPENGL, usePageCanvas, webGLContextAttributes);
             
       SDL.surfaces[surf] = {
         width: width,
@@ -273,7 +279,7 @@ var LibrarySDL = {
         canvas: canvas,
         ctx: ctx,
         surf: surf,
-        buffer: 0,
+        buffer: buffer,
         pixelFormat: pixelFormat,
         alpha: 255,
         flags: flags,
@@ -1072,8 +1078,9 @@ var LibrarySDL = {
 
     var surfData = SDL.surfaces[surf];
 
-    surfData.locked--;
-    if (surfData.locked > 0) return;
+    if (!surfData.locked || --surfData.locked > 0) {
+      return;
+    }
 
     // Copy pixel data to image
     if (surfData.isFlagSet(0x00200000 /* SDL_HWPALETTE */)) {
diff --git a/tests/sdl_swsurface.c b/tests/sdl_swsurface.c
new file mode 100644
index 00000000..93141857
--- /dev/null
+++ b/tests/sdl_swsurface.c
@@ -0,0 +1,22 @@
+#include <assert.h>
+#include <stdio.h>
+#include <SDL/SDL.h>
+
+int main(int argc, char** argv) {
+  SDL_Init(SDL_INIT_VIDEO);
+  SDL_Surface *screen = SDL_SetVideoMode(256, 256, 32, SDL_SWSURFACE);
+
+  // pixels should always be initialized for software surfaces,
+  // without having to call SDL_LockSurface / SDL_UnlockSurface
+  assert(screen->pixels != NULL);
+
+  SDL_Quit();
+
+#if EMSCRIPTEN
+  int result = 1;
+  REPORT_RESULT();
+#endif
+
+  return 0;
+}
+
diff --git a/tests/test_browser.py b/tests/test_browser.py
index 8bcd708e..275f41c6 100644
--- a/tests/test_browser.py
+++ b/tests/test_browser.py
@@ -660,6 +660,9 @@ If manually bisecting:
     shutil.move(os.path.join(self.get_dir(), 'datafile.txt'), 'datafile.txt.renamedsoitcannotbefound');
     self.run_browser('page.html', '', '/report_result?1')
 
+  def test_sdl_swsurface(self):
+    self.btest('sdl_swsurface.c', expected='1')
+
   def test_sdl_image(self):
     # load an image file, get pixel data. Also O2 coverage for --preload-file, and memory-init
     shutil.copyfile(path_from_root('tests', 'screenshot.jpg'), os.path.join(self.get_dir(), 'screenshot.jpg'))