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 stencil test instead of tile mask approach #9012

Merged
merged 2 commits into from
Nov 30, 2019
Merged

Conversation

astojilj
Copy link
Contributor

@astojilj astojilj commented Nov 22, 2019

Remove tile masks for raster and hillshade. Rendering happens in Z-descending order.
All the tiles with the same Z use the same stencil value. Parent uses lower stencil value so that the area covered by children gets masked.
Stencil ref values continue the range used in _tileClippingMaskIDs.

To understand if this patch makes sense, I measured the time spent in updateTileMasks for simple user interaction: drag map and wait few seconds. Used this patch for measurements.

Screen Shot 2019-11-21 at 22 58 43

Measurements of time spent in updateTileMasks:

  • Chrome on Nexus 5:
    Average 1.1ms per frame. Max, per 100 frames is in range [7ms - 30ms]. Average 9 tiles per frame processed. See the screenshot for illustration.

  • Firefox on Mac Mini 3.2 GHz 6-Core Intel Core i7, 1440p screen. Example measurements, in milliseconds:

Average 0.2435999999991327 max 1.2400000000052387 number of tiles 14.23 
Average 0.5157999999998719 max 5.580000000001746 number of tiles 40.69 
Average 0.7421999999995751 max 2.279999999998836 number of tiles 63.81
Average 1.0597999999996681 max 2.1600000000034925 number of tiles 84.35

The patch moves the masking handling from CPU to GPU. As the behavior of early depth test varies on different hardware, this PR requires FPS measurement (on browser) to rule out performance regression.

Improves file size: Size - JS — -1.6 kB -0.220% (729 kB)

FPS Measurement on Mac Mini 3.2 GHz 6-Core Intel Core i7

Comparison shows that for large number of raster tiles, using stencil to stencil out tiles on GPU performs significantly than current tile mask computation as the cost of tile mast computation surpasses the cost of extra fragment shading.

Patch, 2 layers, second (semi-transparent) layer has every other tile stenciled out: 47 FPS

Mac Mini 3.2 GHz 6-Core Intel Core i7, Intel UHD Graphics 630 1536 MB, 3440 x 1440 monitor. using debug/satellite.html page with map.repaint = true.
URL: http://localhost:9966/debug/satellite.html#12.64/38.88229/-75.67545/0/60

Screen Shot 2019-11-23 at 8 46 20

diff --git a/src/render/draw_raster.js b/src/render/draw_raster.js
index 837a568a9..9985d8aac 100644
--- a/src/render/draw_raster.js
+++ b/src/render/draw_raster.js
@@ -35,7 +35,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
     const minTileZ = coords[coords.length - 1].overscaledZ;
 
     const align = !painter.options.moving;
-    for (const coord of coords) {
+    for (let i = 0; i < coords.length; i++) {
+        const coord = coords[i];
         // Set the lower zoom level to sublayer 0, and higher zoom levels to higher sublayers
         // Use gl.LESS to prevent double drawing in areas where tiles overlap.
         const depthMode = painter.depthModeForSublayer(coord.overscaledZ - minTileZ,
@@ -74,7 +75,9 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
                 uniformValues, layer.id, source.boundsBuffer,
                 painter.quadTriangleIndexBuffer, source.boundsSegments);
         } else {
-            program.draw(context, gl.TRIANGLES, depthMode, getStencilMode(coord), colorMode, CullFaceMode.disabled,
+            const stencilMode = getStencilMode(coord);
+            if (layer.paint.get('raster-opacity') < 1 && (i % 2) === 0) stencilMode.ref = 0;
+            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled,
                 uniformValues, layer.id, painter.rasterBoundsBuffer,
                 painter.quadTriangleIndexBuffer, painter.rasterBoundsSegments);
         }
Master, 2 layers, second (semi-transparent) layer has every other tile with ColorMode.disabled: 34 FPS

Screen Shot 2019-11-23 at 8 56 50

diff --git a/src/render/draw_raster.js b/src/render/draw_raster.js
index ad9598308..a7bc75e6a 100644
--- a/src/render/draw_raster.js
+++ b/src/render/draw_raster.js
@@ -8,6 +8,7 @@ import StencilMode from '../gl/stencil_mode';
 import DepthMode from '../gl/depth_mode';
 import CullFaceMode from '../gl/cull_face_mode';
 import {rasterUniformValues} from './program/raster_program';
+import ColorMode from '../gl/color_mode';
 
 import type Painter from './painter';
 import type SourceCache from '../source/source_cache';
@@ -29,7 +30,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
     const colorMode = painter.colorModeForRenderPass();
     const minTileZ = coords.length && coords[0].overscaledZ;
     const align = !painter.options.moving;
-    for (const coord of coords) {
+    for (let i = 0; i < coords.length; i++) {
+        const coord = coords[i];
         // Set the lower zoom level to sublayer 0, and higher zoom levels to higher sublayers
         // Use gl.LESS to prevent double drawing in areas where tiles overlap.
         const depthMode = painter.depthModeForSublayer(coord.overscaledZ - minTileZ,
@@ -68,7 +70,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
                 uniformValues, layer.id, source.boundsBuffer,
                 painter.quadTriangleIndexBuffer, source.boundsSegments);
         } else if (tile.maskedBoundsBuffer && tile.maskedIndexBuffer && tile.segments) {
-            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled,
+            const color = layer.paint.get('raster-opacity') < 1 && (i % 2) === 0 ? ColorMode.disabled : colorMode;
+            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, color, CullFaceMode.disabled,
                 uniformValues, layer.id, tile.maskedBoundsBuffer,
                 tile.maskedIndexBuffer, tile.segments, layer.paint,
                 painter.transform.zoom);
debug/satellite.html changed to display 2 raster layers
diff --git a/debug/satellite.html b/debug/satellite.html
index c142fa8ab..a4aac2212 100644
--- a/debug/satellite.html
+++ b/debug/satellite.html
@@ -26,6 +26,20 @@ var map = window.map = new mapboxgl.Map({
     hash: true
 });
 
+map.repaint = true;
+
+map.on('load', () => {
+    map.addLayer({
+        "id": "satellite1",
+        "type": "raster",
+        "source": "mapbox",
+        "paint" : {
+            "raster-opacity": 0.9,
+            "raster-hue-rotate": 90
+        }
+    });
+});
+
 </script>
 </body>
 </html>

Nexus 5 (larger fluctuations in measured values but the trend is noticeable)

  • master one raster layer: ~43 FPS
  • patch one layer: ~45 FPS
  • master two layers, semi transparent or opaque: ~35 FPS
  • patch two layers, semi transparent or opaque: ~39 FPS

Launch Checklist

  • briefly describe the changes in this PR
  • manually test the debug page
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
  • In separate PR, orthogonal to this change, optimize (don't call) _renderTileClippingMasks for layers when layers don't have the current pass. Currently, we are rendering (when using e.g. fill extrusions) tile clipping masks in both opaque and translucent pass.
  • Reliable FPS measurements

@astojilj astojilj requested a review from kkaefer November 22, 2019 12:33
@astojilj astojilj added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Nov 22, 2019
@astojilj astojilj self-assigned this Nov 22, 2019
@astojilj astojilj requested a review from ansis November 22, 2019 12:42
@astojilj
Copy link
Contributor Author

Improvement in frame rendering time, when all the tile are loaded is measurable:

Mac Mini 3.2 GHz 6-Core Intel Core i7, Intel UHD Graphics 630 1536 MB, 3440 x 1440 monitor. using debug/satellite.html page with map.repaint = true.
URL: http://localhost:9966/debug/satellite.html#12.64/38.88229/-75.67545/0/60

master: 42 - 48 FPS
with the patch: 58 - 60 FPS

Screen Shot 2019-11-22 at 21 56 40

@astojilj
Copy link
Contributor Author

In another measurement, used two satellite layers, and measured difference when second raster one is semi transparent vs opaque. In this case it renders two satellite layers and there is a significant contribution of time spent on preparing and issuing draw calls but, should the depth test work, we could compare impact of fragment shading.

patch semi-transparent overlay: ~47 FPS
patch opaque overlay: ~47 FPS
master (irrelevant measurement as it includes tile mask + overdraw): ~31FPS in both cases

Screen Shot 2019-11-22 at 23 33 29

Patch used in measurement:

diff --git a/debug/satellite.html b/debug/satellite.html
index c142fa8ab..663c1bb2d 100644
--- a/debug/satellite.html
+++ b/debug/satellite.html
@@ -26,6 +26,20 @@ var map = window.map = new mapboxgl.Map({
     hash: true
 });
 
+map.repaint = true;
+
+map.on('load', () => {
+    map.addLayer({
+        "id": "satellite1",
+        "type": "raster",
+        "source": "mapbox",
+        "paint" : {
+            "raster-opacity": 0.3,
+            "raster-hue-rotate": 90
+        }
+    });
+});
+
 </script>
 </body>
 </html>

@astojilj
Copy link
Contributor Author

Patch, 2 layers, second (semi-transparent) layer has every other tile stenciled out: 47 FPS

Screen Shot 2019-11-23 at 8 46 20

diff --git a/src/render/draw_raster.js b/src/render/draw_raster.js
index 837a568a9..9985d8aac 100644
--- a/src/render/draw_raster.js
+++ b/src/render/draw_raster.js
@@ -35,7 +35,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
     const minTileZ = coords[coords.length - 1].overscaledZ;
 
     const align = !painter.options.moving;
-    for (const coord of coords) {
+    for (let i = 0; i < coords.length; i++) {
+        const coord = coords[i];
         // Set the lower zoom level to sublayer 0, and higher zoom levels to higher sublayers
         // Use gl.LESS to prevent double drawing in areas where tiles overlap.
         const depthMode = painter.depthModeForSublayer(coord.overscaledZ - minTileZ,
@@ -74,7 +75,9 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
                 uniformValues, layer.id, source.boundsBuffer,
                 painter.quadTriangleIndexBuffer, source.boundsSegments);
         } else {
-            program.draw(context, gl.TRIANGLES, depthMode, getStencilMode(coord), colorMode, CullFaceMode.disabled,
+            const stencilMode = getStencilMode(coord);
+            if (layer.paint.get('raster-opacity') < 1 && (i % 2) === 0) stencilMode.ref = 0;
+            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled,
                 uniformValues, layer.id, painter.rasterBoundsBuffer,
                 painter.quadTriangleIndexBuffer, painter.rasterBoundsSegments);
         }
Master, 2 layers, second (semi-transparent) layer has every other tile with ColorMode.disabled: 34 FPS

Screen Shot 2019-11-23 at 8 56 50

diff --git a/src/render/draw_raster.js b/src/render/draw_raster.js
index ad9598308..a7bc75e6a 100644
--- a/src/render/draw_raster.js
+++ b/src/render/draw_raster.js
@@ -8,6 +8,7 @@ import StencilMode from '../gl/stencil_mode';
 import DepthMode from '../gl/depth_mode';
 import CullFaceMode from '../gl/cull_face_mode';
 import {rasterUniformValues} from './program/raster_program';
+import ColorMode from '../gl/color_mode';
 
 import type Painter from './painter';
 import type SourceCache from '../source/source_cache';
@@ -29,7 +30,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
     const colorMode = painter.colorModeForRenderPass();
     const minTileZ = coords.length && coords[0].overscaledZ;
     const align = !painter.options.moving;
-    for (const coord of coords) {
+    for (let i = 0; i < coords.length; i++) {
+        const coord = coords[i];
         // Set the lower zoom level to sublayer 0, and higher zoom levels to higher sublayers
         // Use gl.LESS to prevent double drawing in areas where tiles overlap.
         const depthMode = painter.depthModeForSublayer(coord.overscaledZ - minTileZ,
@@ -68,7 +70,8 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty
                 uniformValues, layer.id, source.boundsBuffer,
                 painter.quadTriangleIndexBuffer, source.boundsSegments);
         } else if (tile.maskedBoundsBuffer && tile.maskedIndexBuffer && tile.segments) {
-            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled,
+            const color = layer.paint.get('raster-opacity') < 1 && (i % 2) === 0 ? ColorMode.disabled : colorMode;
+            program.draw(context, gl.TRIANGLES, depthMode, stencilMode, color, CullFaceMode.disabled,
                 uniformValues, layer.id, tile.maskedBoundsBuffer,
                 tile.maskedIndexBuffer, tile.segments, layer.paint,
                 painter.transform.zoom);
debug/satellite.html changed to display 2 raster layers
diff --git a/debug/satellite.html b/debug/satellite.html
index c142fa8ab..a4aac2212 100644
--- a/debug/satellite.html
+++ b/debug/satellite.html
@@ -26,6 +26,20 @@ var map = window.map = new mapboxgl.Map({
     hash: true
 });
 
+map.repaint = true;
+
+map.on('load', () => {
+    map.addLayer({
+        "id": "satellite1",
+        "type": "raster",
+        "source": "mapbox",
+        "paint" : {
+            "raster-opacity": 0.9,
+            "raster-hue-rotate": 90
+        }
+    });
+});
+
 </script>
 </body>
 </html>

const getStencilMode = (coord) => {
stencilMode.ref = painter.nextStencilID + coord.overscaledZ - minTileZ;
return stencilMode;
};
Copy link
Member

Choose a reason for hiding this comment

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

Instead of providing a callback function to generate the value, can we just return an array with all precomputed StencilMode objects? This would also allow us to update the nextStencilID to the final value immediately rather than having a done callback. I believe that this makes the code easier to follow and has less side effects, e.g. when forgetting to call done during a refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used object for mapping Z to StencilMode.

src/render/painter.js Outdated Show resolved Hide resolved
src/render/painter.js Show resolved Hide resolved
@astojilj
Copy link
Contributor Author

@kkaefer Thanks. 34d67dd addressed review comments and adds a raster test that combines raster and vector tiles to further test stencil approach. PTAL.

Remove tile masks for raster. Rendering happens in Z-descending order.
All the tiles with the same Z use the same stencil value. Parent uses lower stencil value so that the area covered by children gets masked.
Stencil ref values continue range used in _tileClippingMaskIDs.
Return map z -> StencilMode instead of using callbacks.
overlapping-vector uses vector layer between two raster to guard
against side effects when refactoring stencil usage.
@astojilj astojilj merged commit 3c36a67 into master Nov 30, 2019
@astojilj astojilj deleted the astojilj-stencil branch November 30, 2019 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants