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

Update ANGLE binaries. #305

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

null77
Copy link

@null77 null77 commented Dec 19, 2024

  • the binaries are pulled from Chrome 131 binary dirs
  • load the entry points via a loader, using code from ANGLE
  • use the WebGL compatibility mode extension in ANGLE
  • replace Linux and Mac gyp builds with binaries

Note that currently there are a couple WebGL regressions because of an outdated conformance test suite.

This contribution is funded by https://higharc.com/

@null77
Copy link
Author

null77 commented Dec 19, 2024

Hello @dhritzkiv ! This is the first part of supporting WebGL 2 in headless-gl. I redid how ANGLE is integrated. Given that modern ANGLE is based on GN, and building is not as well supported in node, it's much easier simply to bundle binaries for all platforms. I tested with my Windows machine, Macbook and WSL 2.

There are some regressions as noted - the gl-conformance test suite is a decade behind mainline WebGL testing. I spent some time looking at migrating to a newer base for gl-conformance, but it's not trivial because the shims are so out of date. I think a better approach might be to implement a shim library that shims specific functions in the test harness, and keeps the rest of the functions unchanged, using some kind of preprocessing script. Anyway I could use help on figuring out what to do here - there weren't many regressions in my local testing so one trivial option is simply suppressing the failing tests. For WebGL 2 testing we'd need a more updated solution.

Please let me know if you have any questions or concerns about the approach or would like anything changed.

@dhritzkiv
Copy link
Member

Huge effort! Amazing. Thanks.

I'll begin reviewing this in the coming days as I find free time during the holidays. I've pulled this PR down, built it, and can confirm that it builds, and that tests mostly pass (as you've alluded to) with 3 failures out of 15953 on my Mac.

I'll think on what do about the conformance tests. We could selectively ignore/disable the currently failing tests, and instead allow the rest of the test suite to indicate –during upcoming/ongoing development– that everything is functioning acceptably. Or we could patch up the decade-old conformance tests to match more modern test expectations.

Lastly, now that the ANGLE binaries are included in the repo, would you agree that removing the angle submodule is the correct move?

@null77
Copy link
Author

null77 commented Dec 19, 2024

Great! Glad you were able to get it going locally. Looking forward to your feedback. Yes, I think removing references to ANGLE code in the repository is the right way to go.

@null77 null77 force-pushed the update-angle-binaries branch from 594d9d6 to e7f5ec0 Compare December 29, 2024 22:31
@null77
Copy link
Author

null77 commented Dec 29, 2024

Hey @dhritzkiv , I just wanted to let you know I force-pushed a new rev here. There were two fixes:

  • the module loading was looking in the incorrect paths when used as a node module. I fixed this using code from ANGLE.
  • I found the right list of extensions needed for correct operation and added the requesting code, fixing the TODO.

@null77 null77 force-pushed the update-angle-binaries branch from e7f5ec0 to c24bbee Compare January 9, 2025 17:46
@dhritzkiv
Copy link
Member

Any idea why _vertexAttribDivisorANGLE is not a function is occurring?

@null77
Copy link
Author

null77 commented Jan 9, 2025

Any idea why _vertexAttribDivisorANGLE is not a function is occurring?

I'm pretty sure it's because of npm ci calling npm run install which calls "prebuild-install || node-gyp rebuild" .. the prebuild step succeeds in downloading the old binaries and node-gyp rebuild never gets called. I'm not too familiar with how prebuild works so I could use your help here.

@dhritzkiv
Copy link
Member

Ah, good insight. That is indeed what's happening. I'll see what can be done about it.

@dhritzkiv
Copy link
Member

I pushed a change to CI workflows that should resolve the build issue. Mind rebasing your changes?

@null77 null77 force-pushed the update-angle-binaries branch from c24bbee to b73bb1f Compare January 10, 2025 17:41
@dhritzkiv
Copy link
Member

Tests fail mostly expectedly: 4 on macOS, 2 on Windows.

However, there are 484 errors on Ubuntu, most of them OUT_OF_MEMORY related to texture size tests. Likely an inadequately sized allocated buffer with xvfb. I'll take a look, and won't let this block the merge of this PR.

@null77
Copy link
Author

null77 commented Jan 10, 2025

Tests fail mostly expectedly: 4 on macOS, 2 on Windows.

However, there are 484 errors on Ubuntu, most of them OUT_OF_MEMORY related to texture size tests. Likely an inadequately sized allocated buffer with xvfb. I'll take a look, and won't let this block the merge of this PR.

Thanks, this is a tricky test because it pushes up against resource limits where behaviour can get a bit unstable.

- the binaries are pulled from Chrome 131 binary dirs
- load the entry points via a loader, using code from ANGLE
- use the WebGL compatibility mode extension in ANGLE
- replace Linux and Mac gyp builds with binaries
- enable requested ANGLE WebGL extensions on demand
- force ANGLE to use a GLES 2.0 context for WebGL
- turn on more extension conformance tests
- work around ANGLE multisample ext enum bug
- remove the ANGLE submodule since it's no longer used

This contribution is funded by https://higharc.com/
@null77 null77 force-pushed the update-angle-binaries branch from b73bb1f to c7dbb0a Compare January 10, 2025 20:43
@null77
Copy link
Author

null77 commented Jan 10, 2025

One more update: I realized I had uploaded an incorrect version of the patch. Fixed and re-uploaded. Now aside from the ubuntu OOM error, there's only 1 test regression. This test is the link program in use spec change that would be fixed with the gl-conformance update.

@dhritzkiv dhritzkiv merged commit ee42968 into stackgl:master Jan 13, 2025
0 of 9 checks passed
@null77 null77 deleted the update-angle-binaries branch January 13, 2025 15:33
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