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

Use WebGL2 in render tests by default #12701

Merged
merged 9 commits into from
May 15, 2023
Merged

Use WebGL2 in render tests by default #12701

merged 9 commits into from
May 15, 2023

Conversation

stepankuzmin
Copy link
Contributor

@stepankuzmin stepankuzmin commented May 8, 2023

Switches render tests to use WebGL2 by default. Adds WebGL1 render tests for:

  • Chrome on Linux
  • Firefox on Linux
  • Chrome on macOS
  • Safari on macOS

Also, increases the allowed value for the heatmap render tests, which were failing in Firefox.

Screenshot 2023-05-08 at 18 47 59

Note: sometimes, test-render-mac-safari-dev (WebGL2) fails with

Global error: AssertionError: null == true at http://localhost:7357/dist/mapbox-gl-dev.js, line 1774

Launch Checklist

  • briefly describe the changes in this PR
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog></changelog>

@stepankuzmin stepankuzmin added the skip changelog Used for PRs that do not need a changelog entry label May 8, 2023
@stepankuzmin stepankuzmin marked this pull request as ready for review May 8, 2023 16:17
@stepankuzmin stepankuzmin requested a review from a team as a code owner May 8, 2023 16:17
@stepankuzmin stepankuzmin requested a review from mourner May 8, 2023 16:21
@mourner
Copy link
Member

mourner commented May 8, 2023

The heatmap failures look not just like platform differences but more like the difference between the full and no-half-float fallback paths — since there are expected pics for both in that particular test, worth investigating what's happening there before we up the allowed diff.

@stepankuzmin
Copy link
Contributor Author

stepankuzmin commented May 9, 2023

@mourner It appears that the expected-half-float.png has a lower difference than expected.png, and Firefox uses it despite whether WebGL2 is disabled or enabled. So I assume it's WebGL2 that affects these tests

Firefox with WebGL2 disabled

Screenshot 2023-05-09 at 12 25 56

Firefox with WebGL2 enabled

https://output.circle-artifacts.com/output/job/dcceb834-aef0-4f2f-baa1-87eb0e287cdf/artifacts/0/test/integration/render-tests/index.html

Screenshot 2023-05-09 at 12 25 24

@mourner
Copy link
Member

mourner commented May 9, 2023

@stepankuzmin this looks like a genuine bug — FF/WebGL2 renders heatmaps with low res (see some pixel artifacts) as if it bypasses the half-float code path. We can merge the PR anyway but we should make sure to bring those allowed values back after fixing the root cause.

@astojilj
Copy link
Contributor

astojilj commented May 9, 2023

.

@astojilj
Copy link
Contributor

astojilj commented May 9, 2023

We do this, and spec wise, it looks correct:

        if (!isWebGL2) this.extTextureHalfFloat = gl.getExtension('OES_texture_half_float');
        if (isWebGL2 || (this.extTextureHalfFloat && gl.getExtension('OES_texture_half_float_linear'))) {
            this.extRenderToTextureHalfFloat = gl.getExtension('EXT_color_buffer_half_float');
        }

EXT_color_buffer_half_float was originally WebGL1 extension and relatively recently applied as WebGL2 extension (2020) KhronosGroup/WebGL#3146.

I didn't do a thorough check (https://hg.mozilla.org/mozilla-central/log?rev=EXT_color_buffer_half_float)- it is worth checking if it is implemented as WebGL 2 extension in Firefox and report an issue there.

It was removed in https://bugzilla.mozilla.org/show_bug.cgi?id=1271830 but didn't see a PR that's adding it back to WebGL 2 extension list

@stepankuzmin
Copy link
Contributor Author

stepankuzmin commented May 10, 2023

It looks like the latest Firefox misses the EXT_color_buffer_half_float extension. It's also missing on the https://webglreport.com/?v=2 with the latest Firefox

Screenshot 2023-05-10 at 16 12 29

@stepankuzmin
Copy link
Contributor Author

stepankuzmin commented May 11, 2023

@mourner @astojilj I've filled a bug in the Firefox tracker for that https://bugzilla.mozilla.org/show_bug.cgi?id=1832535

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

This looks good, but let's create an issue to make sure to follow up on FF heatmaps & the allowed diff.

@stepankuzmin stepankuzmin merged commit e078bf1 into main May 15, 2023
32 checks passed
@stepankuzmin stepankuzmin deleted the useWebGL2 branch May 15, 2023 13:12
stepankuzmin added a commit that referenced this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Used for PRs that do not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants