From da54ee3bc12b98c9aabc2c422552ef3175b57d07 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 3 Dec 2024 19:52:40 -0800 Subject: [PATCH 1/2] Parameterize test_webgl_offscreen_framebuffer_state_restoration. NFC (#23064) --- ...ffscreen_framebuffer_swap_with_bad_state.c | 8 ++++- test/test_browser.py | 32 +++++++++---------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/test/browser/webgl_offscreen_framebuffer_swap_with_bad_state.c b/test/browser/webgl_offscreen_framebuffer_swap_with_bad_state.c index 562841f24f3e9..759fb353485e0 100644 --- a/test/browser/webgl_offscreen_framebuffer_swap_with_bad_state.c +++ b/test/browser/webgl_offscreen_framebuffer_swap_with_bad_state.c @@ -28,8 +28,14 @@ int main() { EMSCRIPTEN_WEBGL_CONTEXT_HANDLE ctx = emscripten_webgl_create_context("#canvas", &attr); emscripten_webgl_make_context_current(ctx); -#if !TEST_WEBGL2 && TEST_VAO +#if !TEST_WEBGL2 && TEST_VERIFY_WEBGL1_VAO_SUPPORT // This test cannot run without browser support for OES_vertex_array_object. + // This check is just to verify that the browser has support; otherwise, we + // will end up testing the non-VAO path. Enabling it here does not actually do + // anything, because offscreen framebuffer has already been initialized. Note + // that if GL_SUPPORT_AUTOMATIC_ENABLE_EXTENSIONS=0, then offscreen + // framebuffer will _never_ use VAOs on WebGL 1 (unless something enables + // OES_vertex_array_object before createOffscreenFramebuffer runs). if (!emscripten_webgl_enable_extension(ctx, "OES_vertex_array_object")) { return 1; } diff --git a/test/test_browser.py b/test/test_browser.py index 3d6f309d6a121..89b7edbaa6740 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -4322,22 +4322,22 @@ def test_webgl_vao_without_automatic_extensions(self): # Tests that offscreen framebuffer state restoration works @requires_graphics_hardware - def test_webgl_offscreen_framebuffer_state_restoration(self): - for args in [ - # full state restoration path on WebGL 1.0 - ['-sMAX_WEBGL_VERSION', '-sOFFSCREEN_FRAMEBUFFER_FORBID_VAO_PATH'], - # VAO path on WebGL 1.0 - ['-sMAX_WEBGL_VERSION'], - ['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=0'], - # VAO path on WebGL 2.0 - ['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=1', '-DTEST_REQUIRE_VAO=1'], - # full state restoration path on WebGL 2.0 - ['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=1', '-sOFFSCREEN_FRAMEBUFFER_FORBID_VAO_PATH'], - # blitFramebuffer path on WebGL 2.0 (falls back to VAO on Firefox < 67) - ['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=0'], - ]: - cmd = args + ['-lGL', '-sOFFSCREEN_FRAMEBUFFER', '-DEXPLICIT_SWAP=1'] - self.btest_exit('webgl_offscreen_framebuffer_swap_with_bad_state.c', args=cmd) + @parameterized({ + # full state restoration path on WebGL 1.0 + 'gl1_no_vao': (['-sMAX_WEBGL_VERSION=1', '-sOFFSCREEN_FRAMEBUFFER_FORBID_VAO_PATH'],), + # VAO path on WebGL 1.0 + 'gl1': (['-sMAX_WEBGL_VERSION=1', '-DTEST_VERIFY_WEBGL1_VAO_SUPPORT=1'],), + 'gl1_max_gl2': (['-sMAX_WEBGL_VERSION=2'],), + # VAO path on WebGL 2.0 + 'gl2': (['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=1'],), + # full state restoration path on WebGL 2.0 + 'gl2_no_vao': (['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=1', '-sOFFSCREEN_FRAMEBUFFER_FORBID_VAO_PATH'],), + # blitFramebuffer path on WebGL 2.0 (falls back to VAO on Firefox < 67) + 'gl2_no_aa': (['-sMAX_WEBGL_VERSION=2', '-DTEST_WEBGL2=1', '-DTEST_ANTIALIAS=0'],), + }) + def test_webgl_offscreen_framebuffer_state_restoration(self, args, skip_vao=False): + cmd = args + ['-lGL', '-sOFFSCREEN_FRAMEBUFFER', '-DEXPLICIT_SWAP=1'] + self.btest_exit('webgl_offscreen_framebuffer_swap_with_bad_state.c', args=cmd) @parameterized({ '': ([],), From 58889f9f20f74d0dbaed7d49025c49b864359ae1 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 3 Dec 2024 20:53:45 -0800 Subject: [PATCH 2/2] Fix false positive writability check on cache directory (#23049) We were incorrectly reporting non-existent parent directories as non-writable. This was broken in #22801. --- test/test_other.py | 22 ++++++++++++++++++++++ tools/cache.py | 8 ++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 630edf777ed55..a509a9f7ed590 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15412,3 +15412,25 @@ def test_rust_integration_basics(self): return 0; }''') self.do_runf('main.cpp', 'Hello from rust!', emcc_args=[lib]) + + @crossplatform + def test_create_cache_directory(self): + if config.FROZEN_CACHE: + self.skipTest("test doesn't work with frozen cache") + + # Test that the cache directory (including parent directories) is + # created on demand. + with env_modify({'EM_CACHE': os.path.abspath('foo/bar')}): + self.run_process([EMCC, '-c', test_file('hello_world.c')]) + self.assertExists('foo/bar/sysroot_install.stamp') + + if not WINDOWS: + # Test that we generate a nice error when we cannot create the cache + # because it is in a read-only location. + # For some reason this doesn't work on windows, at least not in CI. + os.mkdir('rodir') + os.chmod('rodir', 0o444) + self.assertFalse(os.access('rodir', os.W_OK)) + with env_modify({'EM_CACHE': os.path.abspath('rodir/foo')}): + err = self.expect_fail([EMCC, '-c', test_file('hello_world.c')]) + self.assertContained('emcc: error: unable to create cache directory', err) diff --git a/tools/cache.py b/tools/cache.py index 3a8655449447f..445de535d5683 100644 --- a/tools/cache.py +++ b/tools/cache.py @@ -75,10 +75,10 @@ def lock(reason): def ensure(): ensure_setup() if not os.path.isdir(cachedir): - parent_dir = os.path.dirname(cachedir) - if not is_writable(parent_dir): - utils.exit_with_error(f'unable to create cache directory "{cachedir}": parent directory not writable (see https://emscripten.org/docs/tools_reference/emcc.html for info on setting the cache directory)') - utils.safe_ensure_dirs(cachedir) + try: + utils.safe_ensure_dirs(cachedir) + except Exception as e: + utils.exit_with_error(f'unable to create cache directory "{cachedir}": {e} (see https://emscripten.org/docs/tools_reference/emcc.html for info on setting the cache directory)') def erase():