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

Disables antialias for 15.4/15.5 iOS and M1 devices and provides a console warning #11615

Merged
merged 12 commits into from
Mar 18, 2022

Conversation

avpeery
Copy link
Contributor

@avpeery avpeery commented Mar 17, 2022

Closes #11609. After investigating #11609, it was discovered that maps with terrain and antialias enabled on iOS 15.4 devices experience severe rendering issues in Safari due to a WebKit bug introduced in the 15.4 release that sometimes discards multisample texture content. This approach disables antialias for 15.4 devices by checking the user agent.

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for all new functionality
  • manually test the debug page
  • 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>Fixes rendering bug for iOS 15.4 and M1 devices using maps with antialias enabled.</changelog>

src/ui/map.js Outdated
@@ -451,6 +451,15 @@ class Map extends Camera {
throw new Error(`maxPitch must be less than or equal to ${defaultMaxPitch}`);
}

// iOS 15.4 introduced a rendering bug with antialias set to `true`. Disables antialias for these devices.
if (!!options.antialias && window.navigator.userAgent && /\b(iPad|iPhone|iPod)\b/.test(window.navigator.userAgent)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an existing isSafari method in util.js that you could use.

src/ui/map.js Outdated
// iOS 15.4 introduced a rendering bug with antialias set to `true`. Disables antialias for these devices.
if (!!options.antialias && window.navigator.userAgent && /\b(iPad|iPhone|iPod)\b/.test(window.navigator.userAgent)) {
const iosVersion = window.navigator.userAgent.match(/OS ((\d+_?){2,3})\s/);
if (iosVersion && iosVersion[1] === '15_4') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we confirmed that this doesn't happen on 15.3? Would it be worth it to also check for 15.5 or other future versions in case this isn't in the next Safari minor version?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth adding 15.5 as well. Worst case we have slightly worse maps for a couple of months. Best case we avoid broken maps for months.

src/ui/map.js Outdated
const iosVersion = window.navigator.userAgent.match(/OS ((\d+_?){2,3})\s/);
if (iosVersion && iosVersion[1] === '15_4') {
options.antialias = false;
console.warn(`Antialias has been disabled for iOS 15.4 devices.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to use the warnOnce method from util.js here. I believe that would prevent multiple warnings being fired (e.g. if there's multiple maps on a page).

@ansis
Copy link
Contributor

ansis commented Mar 17, 2022

On top of what @ryanhamley added, I'd extract this into a new function in util/util.js

Before I accidentally threw together a duplicate fix. If anything from there seems helpful, go for it: https://github.com/mapbox/mapbox-gl-js/compare/safari-antialias-bug

@ansis
Copy link
Contributor

ansis commented Mar 17, 2022

TO DO : Forgot to check for M1

I don't think we can detect M1 reliably, so we should disable antialias for Safari 15.4 and 15.5 on any platform.

Cherry Pick 8ea2f7eeb70bd804c03433523a0203c5fb4b30f
@avpeery
Copy link
Contributor Author

avpeery commented Mar 18, 2022

On top of what @ryanhamley added, I'd extract this into a new function in util/util.js

Before I accidentally threw together a duplicate fix. If anything from there seems helpful, go for it: https://github.com/mapbox/mapbox-gl-js/compare/safari-antialias-bug

No worries. I preferred your approach so I cherry-picked the commit!

@avpeery avpeery requested a review from ryanhamley March 18, 2022 19:15
Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

Looks great!

@avpeery
Copy link
Contributor Author

avpeery commented Mar 18, 2022

Issue still happening on chrome and firefox on iOS

@avpeery avpeery changed the title Disables antialias for 15.4 iOS devices and provides a console warning Disables antialias for 15.4/15.5 iOS and M1 devices and provides a console warning Mar 18, 2022
@avpeery
Copy link
Contributor Author

avpeery commented Mar 18, 2022

@ansis Good to go once lint issue is fixed! (done)

@avpeery avpeery merged commit 6f74fc5 into main Mar 18, 2022
@avpeery avpeery deleted the avpeery/antialias-fix branch March 18, 2022 20:41
avpeery added a commit that referenced this pull request Mar 18, 2022
…nsole warning (#11615)

* disable antialias and add a warning for 15.4/15.5 iOS and M1 devices

Cherry Pick 8ea2f7eeb70bd804c03433523a0203c5fb4b30f

Co-authored-by: Ansis Brammanis <[email protected]>
avpeery added a commit that referenced this pull request Mar 18, 2022
* Disables antialias for 15.4/15.5 iOS and M1 devices and provides a console warning (#11615)
* Disables cooperative gestures when map is fullscreen in Safari (#11619)

Co-authored-by: Ansis Brammanis <[email protected]>
ahocevar pushed a commit to ahocevar/mapbox-gl-js that referenced this pull request Mar 23, 2022
…nsole warning (mapbox#11615)

* disable antialias and add a warning for 15.4/15.5 iOS and M1 devices

Cherry Pick 8ea2f7eeb70bd804c03433523a0203c5fb4b30f

Co-authored-by: Ansis Brammanis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants