mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.37k stars 1.33k forks source link

[qt][qml] Mapbox map ignores stacking and draws itself over other widgets #16329

Closed rinigus closed 4 years ago

rinigus commented 4 years ago

Platform: Linux/Qt 5.14 Mapbox SDK version: HEAD

I am using Mapbox GL Native (Qt) library together with unofficial QML bindings. I would like to move away from qt branch and start using master in my work, as changes in the build system landed and allow us to build the library on embedded Linux platforms easier.

However, when I have other QML items which are expected to be rendered on top of the map, those are overdrawn. For example, buttons in Pure Maps are not visible with only map shown. In case of Pure Maps, font rendering is also messed up with no fonts appearing. For simple app, just with a label and sliders on top of it (as in https://github.com/rinigus/mapbox-gl-qml/blob/master/app/qml/main.qml#L339), label and sliders are shown sometimes while you move the map and disappear when steady.

All in all, it points to something being messed up in QML drawing.

After git bisect, I have identified the commit which introduced an issue:

f9796d2a28aa55936a9bb1cb99f9aac39d29b27c is the first bad commit commit f9796d2a28aa55936a9bb1cb99f9aac39d29b27c Author: Juha Alanen juha.alanen@mapbox.com Date: Thu May 23 15:27:08 2019 +0300

    [core] Fix symbol rendering under opaque fill layers

 platform/node/test/ignores.json                  |  1 -
 src/mbgl/renderer/layers/render_symbol_layer.cpp | 14 ++------------
 src/mbgl/renderer/paint_parameters.cpp           |  5 ++---
 3 files changed, 4 insertions(+), 16 deletions(-)

When I revert the commit as in https://github.com/rinigus/mapbox-gl-native/commit/50a4744e3fb583a341eb33c5516d8b56259cec2a , all is drawn as expected and QML items overlayed over the map as they should be. Not sure the reverting as done is correct.

Adding to CC, as expected interested parties.

/cc @tmpsantos @jmalanen

tmpsantos commented 4 years ago

@rinigus this is likely to be Qt assuming a certain GL state. Can you try something like after the map is rendered?

glDepthFunc(GL_ALWAYS)
glDisable(GL_STENCIL_TEST);
glDisable(GL_DEPTH_TEST);
rinigus commented 4 years ago

I have inserted

  f->glDepthFunc(GL_ALWAYS);
  f->glDisable(GL_STENCIL_TEST);
  f->glDisable(GL_DEPTH_TEST);

into https://github.com/rinigus/mapbox-gl-qml/blob/master/src/qsgmapboxglnode.cpp#L114 . That didn't help, unfortunately.

rinigus commented 4 years ago

PS: Same if I insert it at https://github.com/rinigus/mapbox-gl-qml/blob/master/src/qsgmapboxglnode.cpp#L117

rinigus commented 4 years ago

@tmpsantos: any other ideas to check out?

tmpsantos commented 4 years ago

@rinigus this is the not the first time I see errors like this. It definitely has to do with Qt assuming some GL state that has been changed by GL Native. Few ways I debug this:

rinigus commented 4 years ago

@tmpsantos, sounds like fun! Thanks, I will look into it, probably towards the middle of April. I'll ask if I get in trouble and keep you updated.

rinigus commented 4 years ago

I have not checked with apitrace, but looked into which part of the original commit broke the rendering. When I restored the original function in src/mbgl/renderer/paint_parameters.cpp (original commit change was https://github.com/mapbox/mapbox-gl-native/commit/f9796d2a28aa55936a9bb1cb99f9aac39d29b27c#diff-313821571f0f96e5825200e9c695364e)

gfx::DepthMode PaintParameters::depthModeForSublayer(uint8_t n, gfx::DepthMaskType mask) const {
    if (currentLayer < opaquePassCutoff) {
        return gfx::DepthMode::disabled();
    }
    float nearDepth = ((1 + currentLayer) * numSublayers + n) * depthEpsilon;
    float farDepth = nearDepth + depthRangeSize;
    return gfx::DepthMode { gfx::DepthFunctionType::LessEqual, mask, { nearDepth, farDepth } };
}

all worked as expected. Without it, even raster tiles with a single route over it (no text) was already breaking QML.

Testing further, I replaced

return gfx::DepthMode { gfx::DepthFunctionType::LessEqual, mask, { depth, depth } };

from original commit with

return gfx::DepthMode { gfx::DepthFunctionType::LessEqual, mask, { depth-tol, depth } };

and looked into different tol values. Looks like ~1e-4 is sufficient to make it render fine. Smaller tol values will make it either slow or not rendering at all.

So, looking at gfx::DepthMode, it seems that on PC with Qt/Qml, Range<float> range should be a range and not just a single float value as given in the original commit. Not sure why this range was replaced with one value (depth,depth) by @jmalanen.

tmpsantos commented 4 years ago

@astojilj do you know what could be happening here?

astojilj commented 4 years ago

@rinigus QQuickWindow doesn't apparently reset glDepthRangef. You'll need to call it as we do in depth mode (or submit a patch to qt if default glDepthRangef(0, 1) resolves the issues for you).

rinigus commented 4 years ago

@astojilj , thank you very much! Indeed, by adding

  f->glDepthRangef(0, 1);

after map rendering, all works as expected. I will test it a bit and will close the issue if it is fully resolved.

rinigus commented 4 years ago

Closing it, as it looks like the issue is resolved. I have some performance problems with the non-zero pitch case in my application, but I will have to figure out the minimal code to reproduce it. As it takes longer time that I would like due to other commitments, I will open new issue if needed.

Thank you very much for your help!