Skip to content
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.

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

fix(test): fix unit tests after 3d-tiles-renderer update #2376

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

jailln
Copy link
Contributor

@jailln jailln commented Aug 2, 2024

Proposition that fixes unit tests on the 3d-tiles-migration branch after 3d-tiles-renderer update to 0.3.36. I'm not satisfied with the proposed solution but it works and I think it can be sufficient for this work on 3D Tiles migration I think finding a better solution should be treated seperatly in the future. Let me know if you agree with that (in which case I will open an issue dedicated to this matter).

Some notes on the implementation and possible improvements:

Unit tests were not running anymore after 3d-tiles-renderer update to 0.3.36 because of a newly added WebGLRenderer import, requiring a gl context which is not available in unit tests they are run in a node environment and not in a browser. The encountered error:

TypeError: gl.getExtension is not a function
    at getExtension (file:///home/jailln/Documents/dev/itowns/node_modules/three/build/three.module.js:746:1506)
    at Object.init (file:///home/jailln/Documents/dev/itowns/node_modules/three/build/three.module.js:746:1648)
    at initGLContext (file:///home/jailln/Documents/dev/itowns/node_modules/three/build/three.module.js:1207:926)
    at new WebGLRenderer (file:///home/jailln/Documents/dev/itowns/node_modules/three/build/three.module.js:1207:2396)
    at new <anonymous> (file:///home/jailln/Documents/dev/itowns/node_modules/3d-tiles-renderer/src/three/loaders/gltf/metadata/utilities/TextureReadUtility.js:12:22)
    at file:///home/jailln/Documents/dev/itowns/node_modules/3d-tiles-renderer/src/three/loaders/gltf/metadata/utilities/TextureReadUtility.js:10:35
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)
    at async formattedImport (/home/jailln/Documents/dev/itowns/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
    at async Object.exports.requireOrImport (/home/jailln/Documents/dev/itowns/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
    at async Object.exports.loadFilesAsync (/home/jailln/Documents/dev/itowns/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/home/jailln/Documents/dev/itowns/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/home/jailln/Documents/dev/itowns/node_modules/mocha/lib/cli/run.js:370:5)

Currently, we mock everything that's not available and needed for unit tests in bootstrap.js. To mock the webgl context, we can either mock it ourselves (but that can be complicated) or rely on an external library. Possible libraries are webgl-mock or headless-gl. webgl-mock is not maintained anymore (last version released 6 years ago). headless-gl last activity is more recent but is still not that very active. Both libraries only support webgl 1.0 and there is no official roadmap on supporting WebGL 2.0 (although we would not be the only one to need it 😀 ). This is a major problem since threejs and itowns dropped WebGL 1.0 support.

In this PR I went for the "quick way" and used webgl-mock (only because I found out about headless-gl afterwards) and I added the following:

  • Implemented WebGLRenderingContext.getParameter to return WebGL 2 for this to work. Note that this is a shortcut and it should return a different value depending on the paramater argument to get (e.g. those two ones that are used for WebGLRenderer initialization).
  • Mocked the only needed WebGL 2 function for the WebGLRenderer to get initialized: texImage3D

A better solution would probably be to use headless-gl and wait for WebGL 2.0 support (or implement it). We could also use puppeteer (but that's kind of weird to use puppeteer in unit tests). Note that maplibre also struggled with this issue and that they dropped webgl 2.0 unit tests for now (which is not really an option for us here) and that deckgl did the same.

@jailln jailln force-pushed the fix/3d-tiles-unit-test branch from a29867b to eac5514 Compare August 6, 2024 08:51
@jailln jailln requested review from Desplandis and mgermerie August 6, 2024 09:08
@jailln jailln force-pushed the fix/3d-tiles-unit-test branch from 5be7478 to b1c39c2 Compare August 6, 2024 09:09
@Desplandis
Copy link
Contributor

Desplandis commented Aug 7, 2024

First, I would like to thank you for the whole context around this issue. =)

Unit tests were not running anymore after 3d-tiles-renderer update to 0.3.36 because of a newly added WebGLRenderer import, [...]

If we could passed our own mocked instance of WebGLRenderer, this would simplify the whole issue. x)

Possible libraries are webgl-mock or headless-gl.

I think we should go for headless-gl in the long term (and what about WebGPU in the future?) but the webgl-mock implementation is sufficient for now.

Note that maplibre also maplibre/maplibre-gl-js#2420 and that they dropped webgl 2.0 unit tests for now (which is not really an option for us here) and that deckgl did the same.

Yeah saw this a few months ago, when I experimented with executing itowns in a "native" context (for easier GPU debugging). There are a few bindings to ANGLE (headless-gl being one) but they all seems to be stuck with WebGL1...

@jailln jailln merged commit 8533402 into 3d-tiles-migration Aug 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants