Skip to content

WebGL bindings: Fix shifts for >=2GB heaps #17832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

WebGL bindings: Fix shifts for >=2GB heaps #17832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f0cfa2
WebGL bindings: Fix shifts for >=2GB heaps. Fixes #17539
kripken Sep 12, 2022
1227a32
comment
kripken Sep 12, 2022
a9a7bb0
add disabled test
kripken Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/library_webgl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,24 @@ var LibraryGL = {
return HEAPU16;
},

// Returns the appropriate amount of shifts for a heap (based on the element
// size in the heap).
$heapAccessShiftForWebGLHeap: function(heap) {
return 31 - Math.clz32(heap.BYTES_PER_ELEMENT);
},

// Receives a pointer and a heap, and shifts the pointer the appropriate
// amount of times for that heap (based on the element size in the heap).
$getShiftedPtrForWebGLHeap__deps: ['$heapAccessShiftForWebGLHeap'],
$getShiftedPtrForWebGLHeap: function(ptr, heap) {
var shifts = heapAccessShiftForWebGLHeap(heap);
#if CAN_ADDRESS_2GB
return ptr >>> shifts;
#else
return ptr >> shifts;
#endif
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I think this would be nice to do compile-time, #15433 can help with that. This way there wouldn't be a WebGL-specific function emitted to grow code size. But lgtm for now.


#if MIN_WEBGL_VERSION == 1
_webgl_enable_ANGLE_instanced_arrays: function(ctx) {
// Extension available in WebGL 1 from Firefox 26 and Google Chrome 30 onwards. Core feature in WebGL 2.
Expand Down Expand Up @@ -1518,7 +1532,7 @@ var LibraryGL = {
glTexImage2D__sig: 'viiiiiiiii',
glTexImage2D__deps: ['$emscriptenWebGLGetTexPixelData'
#if MAX_WEBGL_VERSION >= 2
, '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
, '$heapObjectForWebGLType', '$getShiftedPtrForWebGLHeap'
#endif
],
glTexImage2D: function(target, level, internalFormat, width, height, border, format, type, pixels) {
Expand Down Expand Up @@ -1547,7 +1561,7 @@ var LibraryGL = {
GLctx.texImage2D(target, level, internalFormat, width, height, border, format, type, pixels);
} else if (pixels) {
var heap = heapObjectForWebGLType(type);
GLctx.texImage2D(target, level, internalFormat, width, height, border, format, type, heap, pixels >> heapAccessShiftForWebGLHeap(heap));
GLctx.texImage2D(target, level, internalFormat, width, height, border, format, type, heap, getShiftedPtrForWebGLHeap(pixels, heap));
} else {
GLctx.texImage2D(target, level, internalFormat, width, height, border, format, type, null);
}
Expand All @@ -1560,7 +1574,7 @@ var LibraryGL = {
glTexSubImage2D__sig: 'viiiiiiiii',
glTexSubImage2D__deps: ['$emscriptenWebGLGetTexPixelData'
#if MAX_WEBGL_VERSION >= 2
, '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
, '$heapObjectForWebGLType', '$getShiftedPtrForWebGLHeap'
#endif
],
glTexSubImage2D: function(target, level, xoffset, yoffset, width, height, format, type, pixels) {
Expand All @@ -1579,7 +1593,7 @@ var LibraryGL = {
GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, pixels);
} else if (pixels) {
var heap = heapObjectForWebGLType(type);
GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, heap, pixels >> heapAccessShiftForWebGLHeap(heap));
GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, heap, getShiftedPtrForWebGLHeap(pixels, heap));
} else {
GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, null);
}
Expand All @@ -1594,7 +1608,7 @@ var LibraryGL = {
glReadPixels__sig: 'viiiiiii',
glReadPixels__deps: ['$emscriptenWebGLGetTexPixelData'
#if MAX_WEBGL_VERSION >= 2
, '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
, '$heapObjectForWebGLType', '$getShiftedPtrForWebGLHeap'
#endif
],
glReadPixels: function(x, y, width, height, format, type, pixels) {
Expand All @@ -1604,7 +1618,7 @@ var LibraryGL = {
GLctx.readPixels(x, y, width, height, format, type, pixels);
} else {
var heap = heapObjectForWebGLType(type);
GLctx.readPixels(x, y, width, height, format, type, heap, pixels >> heapAccessShiftForWebGLHeap(heap));
GLctx.readPixels(x, y, width, height, format, type, heap, getShiftedPtrForWebGLHeap(pixels, heap));
}
return;
}
Expand Down
8 changes: 4 additions & 4 deletions src/library_webgl2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,26 +161,26 @@ var LibraryWebGL2 = {
},

glTexImage3D__sig: 'viiiiiiiiii',
glTexImage3D__deps: ['$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'],
glTexImage3D__deps: ['$heapObjectForWebGLType', '$getShiftedPtrForWebGLHeap'],
glTexImage3D: function(target, level, internalFormat, width, height, depth, border, format, type, pixels) {
if (GLctx.currentPixelUnpackBufferBinding) {
GLctx['texImage3D'](target, level, internalFormat, width, height, depth, border, format, type, pixels);
} else if (pixels) {
var heap = heapObjectForWebGLType(type);
GLctx['texImage3D'](target, level, internalFormat, width, height, depth, border, format, type, heap, pixels >> heapAccessShiftForWebGLHeap(heap));
GLctx['texImage3D'](target, level, internalFormat, width, height, depth, border, format, type, heap, getShiftedPtrForWebGLHeap(pixels, heap));
} else {
GLctx['texImage3D'](target, level, internalFormat, width, height, depth, border, format, type, null);
}
},

glTexSubImage3D__sig: 'viiiiiiiiiii',
glTexSubImage3D__deps: ['$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'],
glTexSubImage3D__deps: ['$heapObjectForWebGLType', '$getShiftedPtrForWebGLHeap'],
glTexSubImage3D: function(target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, pixels) {
if (GLctx.currentPixelUnpackBufferBinding) {
GLctx['texSubImage3D'](target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, pixels);
} else if (pixels) {
var heap = heapObjectForWebGLType(type);
GLctx['texSubImage3D'](target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, heap, pixels >> heapAccessShiftForWebGLHeap(heap));
GLctx['texSubImage3D'](target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, heap, getShiftedPtrForWebGLHeap(pixels, heap));
} else {
GLctx['texSubImage3D'](target, level, xoffset, yoffset, zoffset, width, height, depth, format, type, null);
}
Expand Down
29 changes: 29 additions & 0 deletions test/gl_teximage.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
#include "GLES2/gl2.h"
#include "SDL/SDL.h"

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <emscripten.h>
#include <emscripten/html5.h>
#include <unistd.h>

typedef enum {
Expand Down Expand Up @@ -47,6 +49,27 @@ static void clear_gl_errors()

int main(int argc, char *argv[])
{
#ifdef TEST_WEBGL2_2GB
// Init in WebGL mode.
emscripten_set_canvas_element_size("#canvas", 256, 256);
EmscriptenWebGLContextAttributes attr;
emscripten_webgl_init_context_attributes(&attr);
attr.alpha = attr.depth = attr.stencil = attr.antialias = attr.preserveDrawingBuffer = attr.failIfMajorPerformanceCaveat = 0;
attr.enableExtensionsByDefault = 1;
attr.premultipliedAlpha = 0;
attr.majorVersion = 2;
attr.minorVersion = 0;
EMSCRIPTEN_WEBGL_CONTEXT_HANDLE ctx = emscripten_webgl_create_context("#canvas", &attr);
emscripten_webgl_make_context_current(ctx);

// Allocate 2GB to force the texture data to have an address in the upper
// 2GB region.
for (int i = 0; i < 2; i++) {
void* wasted = malloc(1024 * 1024 * 1024);
assert(wasted);
}
#endif

TestStatus passed = TEST_STATUS_SUCCESS;
SDL_Surface *screen;

Expand Down Expand Up @@ -79,6 +102,12 @@ int main(int argc, char *argv[])
exit_with_status(TEST_STATUS_FAILURE);
}

#ifdef TEST_WEBGL2_2GB
// Pointer must use 32 bits to test the possible bug with shifting the
// pointer in texImage2D (we need to shift it as unsigned, not signed).
assert((size_t)pixels >= (size_t)0x80000000UL);
#endif

// 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();
Expand Down
7 changes: 7 additions & 0 deletions test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,13 @@ def test_sdl_glshader2(self):
def test_gl_glteximage(self):
self.btest('gl_teximage.c', '1', args=['-lGL', '-lSDL'])

@unittest.skip("can only work in chrome canary atm (Sep 2022)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use @disabled normally, which I think is just a synonym

@requires_graphics_hardware
def test_gl_glteximage_gl2_4gb(self):
# test glTexImage2D in WebGL2, where using >2GB of memory may be tricky: we
# have a pointer there that is shifted, and we must shift it as unsigned.
self.btest('gl_teximage.c', '1', args=['-lGL', '-lSDL', '-sINITIAL_MEMORY=3GB', '-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2_2GB'])

@parameterized({
'': ([],),
'pthreads': (['-sUSE_PTHREADS', '-sPROXY_TO_PTHREAD', '-sOFFSCREEN_FRAMEBUFFER'],),
Expand Down