Fix readPixels PBO segfault & implement getBufferSubData for WebGL2

src/javascript/webgl-rendering-context.js

@@ -241,6 +241,12 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
       return this._vertexGlobalState._arrayBufferBinding
     } else if (target === this.ELEMENT_ARRAY_BUFFER) {
       return this._vertexObjectState._elementArrayBufferBinding
+    } else if (this._isWebGL2()) {
+      if (target === this.PIXEL_PACK_BUFFER) {
+        return this._pixelPackBufferBinding
+      } else if (target === this.PIXEL_UNPACK_BUFFER) {
+        return this._pixelUnpackBufferBinding
+      }
     }
     return null
   }
@@ -604,8 +610,13 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
     if (!checkObject(buffer)) {
       throw new TypeError('bindBuffer(GLenum, WebGLBuffer)')
     }
+
+    const isPBO = this._isWebGL2() &&
+      (target === this.PIXEL_PACK_BUFFER || target === this.PIXEL_UNPACK_BUFFER)
+
     if (target !== this.ARRAY_BUFFER &&
-      target !== this.ELEMENT_ARRAY_BUFFER) {
+      target !== this.ELEMENT_ARRAY_BUFFER &&
+      !isPBO) {
       this.setError(this.INVALID_ENUM)
       return
     }
@@ -621,21 +632,23 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
         return
       }
       buffer._binding = target | 0
-
       super.bindBuffer(target, buffer._ | 0)
     } else {
       return
     }
 
     if (target === this.ARRAY_BUFFER) {
-      // Buffers of type ARRAY_BUFFER are bound to the global vertex state.
       this._vertexGlobalState.setArrayBuffer(buffer)
-    } else {
-      // Buffers of type ELEMENT_ARRAY_BUFFER are bound to vertex array object state.
+    } else if (target === this.ELEMENT_ARRAY_BUFFER) {
       this._vertexObjectState.setElementArrayBuffer(buffer)
+    } else if (target === this.PIXEL_PACK_BUFFER) {
+      this._pixelPackBufferBinding = buffer
+    } else if (target === this.PIXEL_UNPACK_BUFFER) {
+      this._pixelUnpackBufferBinding = buffer
     }
   }
 
+
   bindBufferBase (target, index, buffer) {
     target |= 0
     index |= 0
@@ -951,22 +964,25 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
   bufferData (target, data, usage) {
     target |= 0
     usage |= 0
+
+    const isPBOUsage = this._isWebGL2() && (
+      usage === this.STREAM_READ ||
+      usage === this.STATIC_READ ||
+      usage === this.DYNAMIC_READ
+    )
+
     if (usage !== this.STREAM_DRAW &&
       usage !== this.STATIC_DRAW &&
-      usage !== this.DYNAMIC_DRAW) {
-      this.setError(this.INVALID_ENUM)
-      return
-    }
-
-    if (target !== this.ARRAY_BUFFER &&
-      target !== this.ELEMENT_ARRAY_BUFFER) {
+      usage !== this.DYNAMIC_DRAW &&
+      !isPBOUsage) {
       this.setError(this.INVALID_ENUM)
       return
     }
 
     const active = this._getActiveBuffer(target)
     if (!active) {
-      this.setError(this.INVALID_OPERATION)
+      this.setError(active === null && this._isWebGL2() ? this.INVALID_ENUM : this.INVALID_OPERATION)
+      if (this.getError() === this.NO_ERROR) this.setError(this.INVALID_OPERATION)
       return
     }
 
@@ -982,15 +998,10 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
       }
 
       this._saveError()
-      super.bufferData(
-        target,
-        u8Data,
-        usage)
+      super.bufferData(target, u8Data, usage)
       const error = this.getError()
       this._restoreError(error)
-      if (error !== this.NO_ERROR) {
-        return
-      }
+      if (error !== this.NO_ERROR) return
 
       active._size = u8Data.length
       if (target === this.ELEMENT_ARRAY_BUFFER) {
@@ -1004,15 +1015,10 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
       }
 
       this._saveError()
-      super.bufferData(
-        target,
-        size,
-        usage)
+      super.bufferData(target, size, usage)
       const error = this.getError()
       this._restoreError(error)
-      if (error !== this.NO_ERROR) {
-        return
-      }
+      if (error !== this.NO_ERROR) return
 
       active._size = size
       if (target === this.ELEMENT_ARRAY_BUFFER) {
@@ -1027,27 +1033,19 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
     target |= 0
     offset |= 0
 
-    if (target !== this.ARRAY_BUFFER &&
-      target !== this.ELEMENT_ARRAY_BUFFER) {
-      this.setError(this.INVALID_ENUM)
+    const active = this._getActiveBuffer(target)
+    if (!active) {
+      this.setError(this.INVALID_OPERATION)
       return
     }
 
-    if (data === null) {
-      return
-    }
+    if (data === null) return
 
     if (!data || typeof data !== 'object') {
       this.setError(this.INVALID_VALUE)
       return
     }
 
-    const active = this._getActiveBuffer(target)
-    if (!active) {
-      this.setError(this.INVALID_OPERATION)
-      return
-    }
-
     if (offset < 0 || offset >= active._size) {
       this.setError(this.INVALID_VALUE)
       return
@@ -1072,10 +1070,36 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
       active._elements.set(u8Data, offset)
     }
 
-    super.bufferSubData(
-      target,
-      offset,
-      u8Data)
+    super.bufferSubData(target, offset, u8Data)
+  }
+
+  getBufferSubData (target, srcByteOffset, dstBuffer, dstOffset = 0, length = 0) {
+    target |= 0
+    srcByteOffset |= 0
+    dstOffset |= 0
+    length |= 0
+
+    const buffer = this._getActiveBuffer(target)
+    if (!buffer) {
+      this.setError(this.INVALID_OPERATION)
+      return
+    }
+
+    if (srcByteOffset < 0) {
+      this.setError(this.INVALID_VALUE)
+      return
+    }
+
+    let subView = dstBuffer
+    if (dstOffset !== 0 || length !== 0) {
+      const elementSize = dstBuffer.BYTES_PER_ELEMENT || 1
+      const start = dstOffset * elementSize
+      const byteLength = length > 0 ? (length * elementSize) : (dstBuffer.byteLength - start)
+
+      subView = new Uint8Array(dstBuffer.buffer, dstBuffer.byteOffset + start, byteLength)
+    }
+
+    return super.getBufferSubData(target, srcByteOffset, subView)
   }
 
   checkFramebufferStatus (target) {
@@ -2212,14 +2236,19 @@ class WebGLRenderingContextHelper extends NativeWebGLRenderingContext {
     width |= 0
     height |= 0
 
-    super.readPixels(
-      x,
-      y,
-      width,
-      height,
-      format,
-      type,
-      pixels)
+    if (this._isWebGL2() && this._pixelPackBufferBinding) {
+      if (typeof pixels !== 'number') {
+        this.setError(this.INVALID_OPERATION)
+        return
+      }
+      return super.readPixels(x, y, width, height, format, type, pixels)
+    }
+
+    if (typeof pixels !== 'object') {
+      throw new TypeError('readPixels(..., ArrayBufferView pixels)')
+    }
+
+    return super.readPixels(x, y, width, height, format, type, pixels)
   }
 
   renderbufferStorage (

src/native/angle-includes/GLES3/gl3.h

@@ -997,6 +997,7 @@ typedef void (GL_APIENTRYP PFNGLUNIFORMMATRIX4X2FVPROC) (GLint location, GLsizei
 typedef void (GL_APIENTRYP PFNGLUNIFORMMATRIX3X4FVPROC) (GLint location, GLsizei count, GLboolean transpose, const GLfloat *value);
 typedef void (GL_APIENTRYP PFNGLUNIFORMMATRIX4X3FVPROC) (GLint location, GLsizei count, GLboolean transpose, const GLfloat *value);
 typedef void (GL_APIENTRYP PFNGLBLITFRAMEBUFFERPROC) (GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter);
+typedef void (GL_APIENTRYP PFNGLGETBUFFERSUBDATAPROC) (GLenum target, GLintptr offset, GLsizeiptr size, void *data);
 typedef void (GL_APIENTRYP PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC) (GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height);
 typedef void (GL_APIENTRYP PFNGLFRAMEBUFFERTEXTURELAYERPROC) (GLenum target, GLenum attachment, GLuint texture, GLint level, GLint layer);
 typedef void *(GL_APIENTRYP PFNGLMAPBUFFERRANGEPROC) (GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access);
@@ -1102,6 +1103,7 @@ GL_APICALL void GL_APIENTRY glUniformMatrix4x2fv (GLint location, GLsizei count,
 GL_APICALL void GL_APIENTRY glUniformMatrix3x4fv (GLint location, GLsizei count, GLboolean transpose, const GLfloat *value);
 GL_APICALL void GL_APIENTRY glUniformMatrix4x3fv (GLint location, GLsizei count, GLboolean transpose, const GLfloat *value);
 GL_APICALL void GL_APIENTRY glBlitFramebuffer (GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter);
+GL_APICALL void GL_APIENTRY glGetBufferSubData (GLenum target, GLintptr offset, GLsizeiptr size, void *data);
 GL_APICALL void GL_APIENTRY glRenderbufferStorageMultisample (GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height);
 GL_APICALL void GL_APIENTRY glFramebufferTextureLayer (GLenum target, GLenum attachment, GLuint texture, GLint level, GLint layer);
 GL_APICALL void *GL_APIENTRY glMapBufferRange (GLenum target, GLintptr offset, GLsizeiptr length, GLbitfield access);

src/native/angle-loader/gles_loader.cc

@@ -256,6 +256,7 @@ PFNGLUNIFORMMATRIX4X2FVPROC l_glUniformMatrix4x2fv;
 PFNGLUNIFORMMATRIX3X4FVPROC l_glUniformMatrix3x4fv;
 PFNGLUNIFORMMATRIX4X3FVPROC l_glUniformMatrix4x3fv;
 PFNGLBLITFRAMEBUFFERPROC l_glBlitFramebuffer;
+PFNGLGETBUFFERSUBDATAPROC l_glGetBufferSubData;
 PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC l_glRenderbufferStorageMultisample;
 PFNGLFRAMEBUFFERTEXTURELAYERPROC l_glFramebufferTextureLayer;
 PFNGLMAPBUFFERRANGEPROC l_glMapBufferRange;
@@ -1181,6 +1182,7 @@ void LoadGLES(LoadProc loadProc)
     l_glUniformMatrix4x3fv =
         reinterpret_cast<PFNGLUNIFORMMATRIX4X3FVPROC>(loadProc("glUniformMatrix4x3fv"));
     l_glBlitFramebuffer = reinterpret_cast<PFNGLBLITFRAMEBUFFERPROC>(loadProc("glBlitFramebuffer"));
+    l_glGetBufferSubData = reinterpret_cast<PFNGLGETBUFFERSUBDATAPROC>(loadProc("glGetBufferSubData"));
     l_glRenderbufferStorageMultisample = reinterpret_cast<PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC>(
         loadProc("glRenderbufferStorageMultisample"));
     l_glFramebufferTextureLayer =

src/native/angle-loader/gles_loader.h

@@ -261,6 +261,7 @@
 #define glUniformMatrix3x4fv l_glUniformMatrix3x4fv
 #define glUniformMatrix4x3fv l_glUniformMatrix4x3fv
 #define glBlitFramebuffer l_glBlitFramebuffer
+#define glGetBufferSubData l_glGetBufferSubData
 #define glRenderbufferStorageMultisample l_glRenderbufferStorageMultisample
 #define glFramebufferTextureLayer l_glFramebufferTextureLayer
 #define glMapBufferRange l_glMapBufferRange
@@ -1103,6 +1104,7 @@ extern PFNGLUNIFORMMATRIX4X2FVPROC l_glUniformMatrix4x2fv;
 extern PFNGLUNIFORMMATRIX3X4FVPROC l_glUniformMatrix3x4fv;
 extern PFNGLUNIFORMMATRIX4X3FVPROC l_glUniformMatrix4x3fv;
 extern PFNGLBLITFRAMEBUFFERPROC l_glBlitFramebuffer;
+extern PFNGLGETBUFFERSUBDATAPROC l_glGetBufferSubData;
 extern PFNGLRENDERBUFFERSTORAGEMULTISAMPLEPROC l_glRenderbufferStorageMultisample;
 extern PFNGLFRAMEBUFFERTEXTURELAYERPROC l_glFramebufferTextureLayer;
 extern PFNGLMAPBUFFERRANGEPROC l_glMapBufferRange;

src/native/webgl.cc

@@ -1767,16 +1767,25 @@ GL_METHOD(GetShaderSource) {
 
 GL_METHOD(ReadPixels) {
   GL_BOILERPLATE;
-
+  
   GLint x = Nan::To<int32_t>(info[0]).ToChecked();
   GLint y = Nan::To<int32_t>(info[1]).ToChecked();
   GLsizei width = Nan::To<int32_t>(info[2]).ToChecked();
   GLsizei height = Nan::To<int32_t>(info[3]).ToChecked();
   GLenum format = Nan::To<int32_t>(info[4]).ToChecked();
   GLenum type = Nan::To<int32_t>(info[5]).ToChecked();
-  Nan::TypedArrayContents<char> pixels(info[6]);
 
-  glReadPixels(x, y, width, height, format, type, *pixels);
+  GLint pbo_binding = 0;
+  glGetIntegerv(GL_PIXEL_PACK_BUFFER_BINDING, &pbo_binding);
+
+  if (pbo_binding > 0) {
+    intptr_t offset = static_cast<intptr_t>(Nan::To<int64_t>(info[6]).ToChecked());
+    glReadPixels(x, y, width, height, format, type, reinterpret_cast<void*>(offset));
+  } else {
+    Nan::TypedArrayContents<char> pixels(info[6]);
+    if (!*pixels) return Nan::ThrowTypeError("Invalid pixel data buffer");
+    glReadPixels(x, y, width, height, format, type, *pixels);
+  }
 }
 
 GL_METHOD(GetTexParameter) {
@@ -2356,12 +2365,29 @@ GL_METHOD(CopyBufferSubData) {
 
 GL_METHOD(GetBufferSubData) {
   GL_BOILERPLATE;
-  GLenum target = Nan::To<int32_t>(info[0]).ToChecked();
-  GLintptr srcByteOffset = Nan::To<int64_t>(info[1]).ToChecked();
-  auto buffer = info[2].As<v8::ArrayBufferView>();
-  void *bufferPtr = buffer->Buffer()->GetBackingStore()->Data();
-  GLsizeiptr bufferSize = buffer->ByteLength();
-  // TODO:  glGetBufferSubData(target, srcByteOffset, bufferSize, bufferPtr);
+  GLenum target = Nan::To<uint32_t>(info[0]).ToChecked();
+  GLintptr offset = static_cast<GLintptr>(Nan::To<int64_t>(info[1]).ToChecked());
+
+  if (!info[2]->IsArrayBufferView()) {
+    return Nan::ThrowTypeError("Argument 3 must be an ArrayBufferView");
+  }
+
+  v8::Local<v8::ArrayBufferView> view = info[2].As<v8::ArrayBufferView>();
+  void *dest = static_cast<uint8_t*>(view->Buffer()->GetBackingStore()->Data()) + view->ByteOffset();
+  GLsizeiptr length = static_cast<GLsizeiptr>(view->ByteLength());
+
+  if (l_glMapBufferRange && l_glUnmapBuffer) {
+    // 0x0001 is GL_MAP_READ_BIT
+    void* gpuPtr = glMapBufferRange(target, offset, length, GL_MAP_READ_BIT);
+    if (gpuPtr) {
+      memcpy(dest, gpuPtr, length);
+      glUnmapBuffer(target);
+    } else {
+      return Nan::ThrowError("glMapBufferRange failed. Is the buffer bound?");
+    }
+  } else {
+    return Nan::ThrowError("Buffer mapping not supported by this driver.");
+  }
 }
 
 GL_METHOD(BlitFramebuffer) {