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

Workaround for failure to clear depth buffer on some Android devices. #7471

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

ChrisLoer
Copy link
Contributor

@ChrisLoer ChrisLoer commented Oct 23, 2018

Fixes issue #3437.

We don't expect gl.depthRange to affect the behavior of gl.clearDepth, but experimentally, it appears that we're failing to clear the depth buffer on some Android devices, and resetting the depth range before the clear call appears to fix the problem (maybe because some drivers implement clearDepth in terms of just drawing a full screen quad to the depth buffer?). This only happens once per frame, so should be negligibly expensive.

The only place I could reproduce this issue was on a BrowserStack Galaxy S6 image running Chrome 68.0.3440.85/Android 5.0.2 -- we'll need to collect feedback from other people who have encountered #3437 to see how full a fix this is.

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes

/cc @ansis @mollymerp @chloekraw

@ChrisLoer ChrisLoer requested a review from ansis October 23, 2018 23:47
@ansis
Copy link
Contributor

ansis commented Oct 23, 2018

It might be worth checking whether mapbox-gl-native has any issues that could be solved by this fix.

@ChrisLoer
Copy link
Contributor Author

@tobrun wasn't aware of any Android rendering issues that sounded like this one. Initial feedback from #3437 is that this fix is working, and I also confirmed on the BrowserStack Note 4 image... so I think this makes sense to merge, even though I'd still like to learn more about what's going on.

@ChrisLoer ChrisLoer merged commit 42d1a5a into master Oct 24, 2018
@mollymerp mollymerp deleted the clear-depth branch October 25, 2018 17:12
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