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.

Sign up for GitHub

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

Jump to bottom

expose context antialiasing and draw 3d things into main framebuffer #7821

Merged
merged 2 commits into from
Feb 25, 2019
Merged

expose context antialiasing and draw 3d things into main framebuffer #7821

merged 2 commits into from
Feb 25, 2019

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Jan 25, 2019

The primary goal is to support antialiasing for custom layers (#7543) by exposing an option to use the context-provided msaa antialiasing. This only works in the main framebuffer so we first need to switch to drawing custom layers an buildings into the main framebuffer. Previously they were drawn into an offscreen framebuffer.

Drawing into the main framebuffer

Drawing directly into the main frame buffer is better because:

  • it avoids fragment-expensive copies
  • it unblocks using msaa antialiasing (which only works in the main

New method for transparent fill-extrusions

Drawing opaque extrusions and custom layers involved few changes. Transparent extrusions needed to be implemented differently. They now use a two pass approach:

  • the first pass only draws depth
  • the second pass only colors a surface if it's depth matches the depth from the previous pass. The stencil buffer is used to prevent shading coincident fragments twice.

The way fill-extrusion-opacity works across layers was already kind of weird. This pr makes no attempt to change any of that behaviour. It should be completely backwards compatible.

Stencil buffer changes

I made some tweaks to how we use the stencil buffer so that we could do this. The changes should improve performance as well because we avoid doing expensive stencil clears.

  • stencil masks are now drawn only if the layer needs them, not the source
  • the stencil buffer is only cleared when we run out of possible IDs
  • making it possible to assign a stencil id that can be used for transparent buildings

Antialiasing

This fixes #7543 by exposing a new antialias map option that is used during gl context creation.

Launch Checklist

  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page

@kkaefer can you review the first commit with two-pass extrusion rendering and stencil buffer changes?

@mourner can you review the second commit that adds the antialias option

@ansis ansis requested review from mourner and kkaefer January 25, 2019 21:52
mourner
mourner approved these changes Jan 28, 2019
View reviewed changes
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.

Wow, this is awesome!

kkaefer
kkaefer approved these changes Jan 30, 2019
View reviewed changes
Copy link
Contributor

@kkaefer kkaefer left a comment

Choose a reason for hiding this comment

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

It seems like this subtly changes the way we render fill layers as well since they're now slightly antialiased. We've removed our fake antialiasing using GL_LINE in #5541, so I guess this is a net-improvement.

src/render/draw_fill_extrusion.js
StencilMode.disabled,
ColorMode.disabled);

// Then draw all the extrusions a second type, only coloring fragments if they have the
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: "a second time"

@ansis
draw extrusions and custom layers into main framebuffer
4eef3c9 
Previously, both fill-extrusions and custom layers were drawn into a
offscreen framebuffer before being copied over to the main framebuffer.

Drawing directly into the main frame buffer is better because:
- it avoids fragment-expensive copies
- it unblocks using msaa antialiasing (which only works in the main

Drawing opaque extrusions and custom layers involved few changes.
Transparent extrusions needed to be implemented differently. They now
use a two pass approach:
- the first pass only draws depth
- the second pass only colors a sufrace if it's depth matches

Some tweaks were made to how we draw stencil masks so that we could use
the stencil buffer in the second pass to prevent double-shading. The
changes should reduce the amount of stencil clears that happen.

The way fill-extrusion-opacity works across layers was already kind of
weird. This pr makes no attempt to change any of that behaviour. It
should be completely backwards compatible.
@ansis
expose option to create context with antialiasing
6ee5daf 
fix #7543
@ansis ansis force-pushed the fill-extrusions-main-framebuffer branch from 6cba5b9 to 6ee5daf Compare February 25, 2019 18:07
@ansis ansis merged commit ed3d0e1 into master Feb 25, 2019
@ansis ansis mentioned this pull request Feb 25, 2019
Closed
@ansis ansis mentioned this pull request Mar 5, 2019
Closed
@Pessimistress Pessimistress mentioned this pull request Mar 17, 2019
Closed
@mourner mourner deleted the fill-extrusions-main-framebuffer branch March 21, 2019 14:42
@tristen tristen mentioned this pull request Apr 5, 2019
Closed
@Wykks Wykks mentioned this pull request May 24, 2019
Closed
astojilj added a commit to mapbox/mapbox-gl-native that referenced this pull request Jul 7, 2019
@astojilj
Fix layers rendering after fill-extrusion
a89473c 
This fixes following issues:
* Fix some false passing combinations/fill-extrusion-translucent--XXXX tests
* Fix and enable other, failing but ignored, combinations/fill-extrusion-translucent--XXXX tests
* Fix rendering of layers that are on top of fill-extrusion layers

state.getProjMatrix(nearClippedProjMatrix, 100) caused that tests with size 64x64 were not
rendering fill extrusions: far plane calculated as 96.9 and near plane set to 100 was the cause.
near plane is changed from hardcoded 100 to depend on state.getCameraToCenterDistance() - producing
similar value but one that follows max zoom.

This caused that e.g. combinations/fill-extrusion-translucent--fill-opaque was falsely passing as
only fill-opaque layer got rendered.

combinations/fill-extrusion-translucent--XXXX tests expose regression #14844 (comment) in #14844, #14779.

Fix (opaquePassCutoff, is3D) is ported from mapbox/mapbox-gl-js#7821

Fixes: #14844, #14779, #15039
@astojilj astojilj mentioned this pull request Jul 7, 2019
Merged
astojilj added a commit to mapbox/mapbox-gl-native that referenced this pull request Jul 9, 2019
@astojilj
Fix layers rendering after fill-extrusion
c852c04 
This fixes following issues:
* Fix some false passing combinations/fill-extrusion-translucent--XXXX tests
* Fix and enable other, failing but ignored, combinations/fill-extrusion-translucent--XXXX tests
* Fix rendering of layers that are on top of fill-extrusion layers

state.getProjMatrix(nearClippedProjMatrix, 100) caused that tests with size 64x64 were not
rendering fill extrusions: far plane calculated as 96.9 and near plane set to 100 was the cause.
near plane is changed from hardcoded 100 to depend on state.getCameraToCenterDistance() - producing
similar value but one that follows max zoom.

This caused that e.g. combinations/fill-extrusion-translucent--fill-opaque was falsely passing as
only fill-opaque layer got rendered.

combinations/fill-extrusion-translucent--XXXX tests expose regression #14844 (comment) in #14844, #14779.

Fix (opaquePassCutoff, is3D) is ported from mapbox/mapbox-gl-js#7821

Fixes: #14844, #14779, #15039
astojilj added a commit to mapbox/mapbox-gl-native that referenced this pull request Jul 9, 2019
@astojilj
Fix layers rendering after fill-extrusion
2c5528e 
This fixes following issues:
* Fix some false passing combinations/fill-extrusion-translucent--XXXX tests
* Fix and enable other, failing but ignored, combinations/fill-extrusion-translucent--XXXX tests
* Fix rendering of layers that are on top of fill-extrusion layers

state.getProjMatrix(nearClippedProjMatrix, 100) caused that tests with size 64x64 were not
rendering fill extrusions: far plane calculated as 96.9 and near plane set to 100 was the cause.
near plane is changed from hardcoded 100 to depend on state.getCameraToCenterDistance() - producing
similar value but one that follows max zoom.

This caused that e.g. combinations/fill-extrusion-translucent--fill-opaque was falsely passing as
only fill-opaque layer got rendered.

combinations/fill-extrusion-translucent--XXXX tests expose regression #14844 (comment) in #14844, #14779.

Fix (opaquePassCutoff, is3D) is ported from mapbox/mapbox-gl-js#7821

Fixes: #14844, #14779, #15039
@andrewharvey andrewharvey mentioned this pull request Jul 13, 2019
Merged
1 task
@asheemmamoowala asheemmamoowala mentioned this pull request Aug 27, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mourner mourner mourner approved these changes

@kkaefer kkaefer kkaefer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

antialiasing for custom layers
3 participants
@ansis @mourner @kkaefer