Some fixes for library_webgl.js in CAN_ADDRESS_GB mode

This is an overall size win too after compression.
sbc100 · Jan 31, 2024 · 989a09e · 989a09e
1 parent f7651be
commit 989a09e
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 37 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -798,8 +798,12 @@ jobs:
     steps:
       - run-tests-chrome:
           title: "browser"
-          # skip test_4gb_fail as it OOMs on the current bot
-          test_targets: "browser skip:browser.test_4gb_fail"
+          # Skip test_4gb_fail as it OOMs on the current bot
+          # Include an representative selection of browser2gb
+          test_targets: "
+            browser skip:browser.test_4gb_fail
+            browser2gb.test_gles2_uniform_arrays
+            "
   test-browser-chrome-wasm64:
     executor: bionic
     steps:

diff --git a/src/library_webgl.js b/src/library_webgl.js
@@ -82,7 +82,12 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
     return HEAPU16;
   },
 
-  $heapAccessShiftForWebGLHeap: (heap) => 31 - Math.clz32(heap.BYTES_PER_ELEMENT),
+  $toTypedArrayIndex: (pointer, heap) =>
+#if MEMORY64
+    pointer / heap.BYTES_PER_ELEMENT,
+#else
+    pointer >>> (31 - Math.clz32(heap.BYTES_PER_ELEMENT)),
+#endif
 
 #if MIN_WEBGL_VERSION == 1
   $webgl_enable_ANGLE_instanced_arrays: (ctx) => {
@@ -1595,22 +1600,20 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
     return colorChannels[format - 0x1902]||1;
   },
 
-  $emscriptenWebGLGetTexPixelData__deps: ['$computeUnpackAlignedImageSize', '$colorChannelsInGlTextureFormat', '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'],
+  $emscriptenWebGLGetTexPixelData__deps: ['$computeUnpackAlignedImageSize', '$colorChannelsInGlTextureFormat', '$heapObjectForWebGLType', '$toTypedArrayIndex'],
   $emscriptenWebGLGetTexPixelData: (type, format, width, height, pixels, internalFormat) => {
     var heap = heapObjectForWebGLType(type);
-    var shift = heapAccessShiftForWebGLHeap(heap);
-    var byteSize = 1<<shift;
-    var sizePerPixel = colorChannelsInGlTextureFormat(format) * byteSize;
+    var sizePerPixel = colorChannelsInGlTextureFormat(format) * heap.BYTES_PER_ELEMENT;
     var bytes = computeUnpackAlignedImageSize(width, height, sizePerPixel, GL.unpackAlignment);
 #if GL_ASSERTIONS
-    assert((pixels >> shift) << shift == pixels, 'Pointer to texture data passed to texture get function must be aligned to the byte size of the pixel type!');
+    assert(pixels % heap.BYTES_PER_ELEMENT == 0, 'Pointer to texture data passed to texture get function must be aligned to the byte size of the pixel type!');
 #endif
-    return heap.subarray(pixels >> shift, pixels + bytes >> shift);
+    return heap.subarray(toTypedArrayIndex(pixels, heap), toTypedArrayIndex(pixels + bytes, heap));
   },
 
   glTexImage2D__deps: ['$emscriptenWebGLGetTexPixelData'
 #if MAX_WEBGL_VERSION >= 2
-                       , '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
+                       , '$heapObjectForWebGLType', '$toTypedArrayIndex'
 #endif
   ],
   glTexImage2D: (target, level, internalFormat, width, height, border, format, type, pixels) => {
@@ -1641,7 +1644,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
         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, toTypedArrayIndex(pixels, heap));
       } else {
         GLctx.texImage2D(target, level, internalFormat, width, height, border, format, type, null);
       }
@@ -1653,7 +1656,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
 
   glTexSubImage2D__deps: ['$emscriptenWebGLGetTexPixelData'
 #if MAX_WEBGL_VERSION >= 2
-                          , '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
+                          , '$heapObjectForWebGLType', '$toTypedArrayIndex'
 #endif
   ],
   glTexSubImage2D: (target, level, xoffset, yoffset, width, height, format, type, pixels) => {
@@ -1674,7 +1677,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
         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, toTypedArrayIndex(pixels, heap));
       } else {
         GLctx.texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, null);
       }
@@ -1688,7 +1691,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
 
   glReadPixels__deps: ['$emscriptenWebGLGetTexPixelData'
 #if MAX_WEBGL_VERSION >= 2
-                       , '$heapObjectForWebGLType', '$heapAccessShiftForWebGLHeap'
+                       , '$heapObjectForWebGLType', '$toTypedArrayIndex'
 #endif
   ],
   glReadPixels: (x, y, width, height, format, type, pixels) => {
@@ -1700,7 +1703,7 @@ for (/**@suppress{duplicate}*/var i = 0; i < {{{ GL_POOL_TEMP_BUFFERS_SIZE }}};
         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, toTypedArrayIndex(pixels, heap));
       }
       return;
     }

diff --git a/test/code_size/hello_webgl2_wasm.json b/test/code_size/hello_webgl2_wasm.json
@@ -1,10 +1,10 @@
 {
   "a.html": 569,
   "a.html.gz": 379,
-  "a.js": 4540,
-  "a.js.gz": 2345,
+  "a.js": 4589,
+  "a.js.gz": 2341,
   "a.wasm": 10451,
   "a.wasm.gz": 6724,
-  "total": 15560,
-  "total_gz": 9448
+  "total": 15609,
+  "total_gz": 9444
 }
diff --git a/test/code_size/hello_webgl2_wasm2js.json b/test/code_size/hello_webgl2_wasm2js.json
@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 17746,
-  "a.js.gz": 7982,
+  "a.js": 17795,
+  "a.js.gz": 7978,
   "a.mem": 3123,
   "a.mem.gz": 2693,
-  "total": 21436,
-  "total_gz": 11054
+  "total": 21485,
+  "total_gz": 11050
 }
diff --git a/test/code_size/hello_webgl_wasm.json b/test/code_size/hello_webgl_wasm.json
@@ -1,10 +1,10 @@
 {
   "a.html": 569,
   "a.html.gz": 379,
-  "a.js": 4027,
-  "a.js.gz": 2173,
+  "a.js": 4075,
+  "a.js.gz": 2170,
   "a.wasm": 10451,
   "a.wasm.gz": 6724,
-  "total": 15047,
-  "total_gz": 9276
+  "total": 15095,
+  "total_gz": 9273
 }
diff --git a/test/code_size/hello_webgl_wasm2js.json b/test/code_size/hello_webgl_wasm2js.json
@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 17219,
-  "a.js.gz": 7814,
+  "a.js": 17267,
+  "a.js.gz": 7813,
   "a.mem": 3123,
   "a.mem.gz": 2693,
-  "total": 20909,
-  "total_gz": 10886
+  "total": 20957,
+  "total_gz": 10885
 }
diff --git a/test/other/metadce/test_metadce_hello_O0.gzsize b/test/other/metadce/test_metadce_hello_O0.gzsize
@@ -1 +1 @@
-8408
+8403
diff --git a/test/other/metadce/test_metadce_hello_O0.jssize b/test/other/metadce/test_metadce_hello_O0.jssize
@@ -1 +1 @@
-23069
+23059
diff --git a/test/other/metadce/test_metadce_minimal_O0.gzsize b/test/other/metadce/test_metadce_minimal_O0.gzsize
@@ -1 +1 @@
-7238
+7232
diff --git a/test/other/metadce/test_metadce_minimal_O0.jssize b/test/other/metadce/test_metadce_minimal_O0.jssize
@@ -1 +1 @@
-19693
+19683
diff --git a/test/other/test_unoptimized_code_size.js.size b/test/other/test_unoptimized_code_size.js.size
@@ -1 +1 @@
-58237
+58227
diff --git a/test/test_browser.py b/test/test_browser.py
@@ -23,7 +23,7 @@
 
 from common import BrowserCore, RunnerCore, path_from_root, has_browser, EMTEST_BROWSER, Reporting
 from common import create_file, parameterized, ensure_dir, disabled, test_file, WEBIDL_BINDER
-from common import read_file, also_with_minimal_runtime, EMRUN, no_wasm64, no_4gb
+from common import read_file, also_with_minimal_runtime, EMRUN, no_wasm64, no_2gb, no_4gb
 from tools import shared
 from tools import ports
 from tools import utils
@@ -5669,6 +5669,7 @@ def test_emmalloc_memgrowth(self, *args):
 
   @no_firefox('no 4GB support yet')
   @no_4gb('uses MAXIMUM_MEMORY')
+  @no_2gb('uses MAXIMUM_MEMORY')
   def test_2gb_fail(self):
     # TODO Convert to an actual browser test when it reaches stable.
     #      For now, keep this in browser as this suite runs serially, which
@@ -5901,6 +5902,13 @@ def test_emrun(self):
       self.assertContained('hello, error stream!', stderr)
 
 
+class browser2gb(browser):
+  def setUp(self):
+    super().setUp()
+    self.set_setting('INITIAL_MEMORY', '2200mb')
+    self.set_setting('GLOBAL_BASE', '2gb')
+
+
 class browser64(browser):
   def setUp(self):
     super().setUp()