Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
Fix layers rendering after fill-extrusion
Browse files Browse the repository at this point in the history
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
  • Loading branch information
astojilj committed Jul 9, 2019
1 parent 929824e commit c852c04
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 33 deletions.
18 changes: 0 additions & 18 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,7 @@
"render-tests/text-rotate/with-offset": "https://github.com/mapbox/mapbox-gl-native/issues/11872",
"render-tests/video/default": "skip - https://github.com/mapbox/mapbox-gl-native/issues/601",
"render-tests/background-color/colorSpace-hcl": "needs issue",
"render-tests/combinations/fill-extrusion-translucent--heatmap-translucent": "needs investigation",
"render-tests/combinations/heatmap-translucent--background-opaque": "needs investigation",
"render-tests/combinations/heatmap-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/background-opaque--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/background-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/circle-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--background-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--circle-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--hillshade-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--fill-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--line-translucent": "needs investigation",
"render-tests/combinations/fill-extrusion-translucent--symbol-translucent": "needs investigation",
"render-tests/combinations/hillshade-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/fill-opaque--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/fill-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/line-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/raster-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/combinations/symbol-translucent--fill-extrusion-translucent": "needs investigation",
"render-tests/feature-state/composite-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12613",
"render-tests/feature-state/data-expression": "https://github.com/mapbox/mapbox-gl-native/issues/12613",
"render-tests/feature-state/vector-source": "https://github.com/mapbox/mapbox-gl-native/issues/12613",
Expand Down
4 changes: 4 additions & 0 deletions src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ bool RenderFillExtrusionLayer::hasCrossfade() const {
return getCrossfade<FillExtrusionLayerProperties>(evaluatedProperties).t != 1;
}

bool RenderFillExtrusionLayer::is3D() const {
return true;
}

void RenderFillExtrusionLayer::render(PaintParameters& parameters) {
assert(renderTiles);
if (parameters.pass != RenderPass::Translucent) {
Expand Down
1 change: 1 addition & 0 deletions src/mbgl/renderer/layers/render_fill_extrusion_layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class RenderFillExtrusionLayer final : public RenderLayer {
void evaluate(const PropertyEvaluationParameters&) override;
bool hasTransition() const override;
bool hasCrossfade() const override;
bool is3D() const override;
void render(PaintParameters&) override;

bool queryIntersectsFeature(
Expand Down
9 changes: 4 additions & 5 deletions src/mbgl/renderer/layers/render_fill_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,10 @@ void RenderFillLayer::render(PaintParameters& parameters) {
);
};

// Only draw the fill when it's opaque and we're drawing opaque fragments,
// or when it's translucent and we're drawing translucent fragments.
if (bucket.triangleIndexBuffer &&
(evaluated.get<FillColor>().constantOr(Color()).a >= 1.0f &&
evaluated.get<FillOpacity>().constantOr(0) >= 1.0f) == (parameters.pass == RenderPass::Opaque)) {
auto fillRenderPass = (evaluated.get<FillColor>().constantOr(Color()).a >= 1.0f
&& evaluated.get<FillOpacity>().constantOr(0) >= 1.0f
&& parameters.currentLayer >= parameters.opaquePassCutoff) ? RenderPass::Opaque : RenderPass::Translucent;
if (bucket.triangleIndexBuffer && parameters.pass == fillRenderPass) {
draw(parameters.programs.getFillLayerPrograms().fill,
gfx::Triangles(),
parameters.depthModeForSublayer(1, parameters.pass == RenderPass::Opaque
Expand Down
12 changes: 8 additions & 4 deletions src/mbgl/renderer/paint_parameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ TransformParameters::TransformParameters(const TransformState& state_)
// odd viewport sizes.
state.getProjMatrix(alignedProjMatrix, 1, true);

// Calculate a second projection matrix with the near plane clipped to 100 so as
// not to waste lots of depth buffer precision on very close empty space, for layer
// types (fill-extrusion) that use the depth buffer to emulate real-world space.
state.getProjMatrix(nearClippedProjMatrix, 100);
// Calculate a second projection matrix with the near plane moved further,
// to a tenth of the far value, so as not to waste depth buffer precision on
// very close empty space, for layer types (fill-extrusion) that use the
// depth buffer to emulate real-world space.
state.getProjMatrix(nearClippedProjMatrix, 0.1 * state.getCameraToCenterDistance());
}

PaintParameters::PaintParameters(gfx::Context& context_,
Expand Down Expand Up @@ -72,6 +73,9 @@ mat4 PaintParameters::matrixForTile(const UnwrappedTileID& tileID, bool aligned)
}

gfx::DepthMode PaintParameters::depthModeForSublayer(uint8_t n, gfx::DepthMaskType mask) const {
if (currentLayer < opaquePassCutoff) {
return gfx::DepthMode::disabled();
}
float depth = depthRangeSize + ((1 + currentLayer) * numSublayers + n) * depthEpsilon;
return gfx::DepthMode { gfx::DepthFunctionType::LessEqual, mask, { depth, depth } };
}
Expand Down
3 changes: 1 addition & 2 deletions src/mbgl/renderer/paint_parameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ class PaintParameters {
uint32_t currentLayer;
float depthRangeSize;
const float depthEpsilon = 1.0f / (1 << 16);


uint32_t opaquePassCutoff = 0;
float symbolFadeChange;
};

Expand Down
3 changes: 3 additions & 0 deletions src/mbgl/renderer/render_layer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ class RenderLayer {
// Returns true if the layer has a pattern property and is actively crossfading.
virtual bool hasCrossfade() const = 0;

// Returns true if layer writes to depth buffer by drawing using PaintParameters::depthModeFor3D().
virtual bool is3D() const { return false; }

// Returns true is the layer is subject to placement.
bool needsPlacement() const;

Expand Down
10 changes: 7 additions & 3 deletions src/mbgl/renderer/render_orchestrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,16 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(const UpdatePar
}
}

for (auto& renderItem : layerRenderItems) {
RenderLayer& renderLayer = renderItem.layer;
renderLayer.prepare({renderItem.source, *imageManager, *patternAtlas, *lineAtlas, updateParameters.transformState});
uint32_t i = static_cast<uint32_t>(layerRenderItems.size()) - 1;
for (auto it = layerRenderItems.begin(); it != layerRenderItems.end(); ++it, --i) {
RenderLayer& renderLayer = it->layer;
renderLayer.prepare({it->source, *imageManager, *patternAtlas, *lineAtlas, updateParameters.transformState});
if (renderLayer.needsPlacement()) {
layersNeedPlacement.emplace_back(renderLayer);
}
if (renderLayer.is3D() && renderTreeParameters->opaquePassCutOff == 0) {
renderTreeParameters->opaquePassCutOff = i;
}
}
// Symbol placement.
{
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/renderer/render_tile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void RenderTile::prepare(const SourcePrepareParameters& parameters) {
}

// Calculate two matrices for this tile: matrix is the standard tile matrix; nearClippedMatrix
// clips the near plane to 100 to save depth buffer precision
// has near plane moved further, to enhance depth buffer precision
const auto& transform = parameters.transform;
transform.state.matrixFor(matrix, id);
transform.state.matrixFor(nearClippedMatrix, id);
Expand Down
1 change: 1 addition & 0 deletions src/mbgl/renderer/render_tree.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class RenderTreeParameters {
TimePoint timePoint;
EvaluatedLight light;
bool has3D = false;
uint32_t opaquePassCutOff = 0;
Color backgroundColor;
float symbolFadeChange = 0.0f;
bool needsRepaint = false;
Expand Down
1 change: 1 addition & 0 deletions src/mbgl/renderer/renderer_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ void Renderer::Impl::render(const RenderTree& renderTree) {
};

parameters.symbolFadeChange = renderTreeParameters.symbolFadeChange;
parameters.opaquePassCutoff = renderTreeParameters.opaquePassCutOff;
const auto& sourceRenderItems = renderTree.getSourceRenderItems();
const auto& layerRenderItems = renderTree.getLayerRenderItems();

Expand Down

0 comments on commit c852c04

Please sign in to comment.