Preserve depth buffer between 3D layers + optimize render order #9931
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -369,6 +369,47 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { | |
| parameters.frameHistory.upload(parameters.context, 0); | ||
| } | ||
|
|
||
| // - PREPARE + CLIP ------------------------------------------------------------------------------ | ||
| { | ||
| MBGL_DEBUG_GROUP(parameters.context, "clip"); | ||
|
|
||
| // Update all clipping IDs. | ||
| for (const auto& entry : renderSources) { | ||
| if (entry.second->isEnabled()) { | ||
| entry.second->startRender(parameters); | ||
| } | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Drawing the stencil mask before the 3D pass doesn't seem necessary since we're not using it, right? Drawing the clipping mask into the stencil buffer before means that we're drawing to the FBO before the clear step, which means that the OpenGL implementation has to restore the previous frame's stencil buffer (and it might potentially produce inaccurate clipping masks). There was a problem hiding this comment. Right — I didn't dig into mapbox-gl-native/src/mbgl/renderer/renderer_impl.cpp Lines 426 to 434 in 7531448 If that's the case, maybe copying the descriptions of this (comments + debug group) exactly is misleading and should be updated to sound more like "upload buffers + generate clipping IDs." Or does uploading and generating clipping IDs need to be separated? There was a problem hiding this comment. Yeah, it looks like the code in the (old)
Looking at the current code, this is already how it's set up. I assumed that There was a problem hiding this comment. I consolidated this group with the "upload" group above it in 9e786c1. |
||
|
|
||
| // - 3D PASS ------------------------------------------------------------------------------------- | ||
| // Renders any 3D layers bottom-to-top to unique FBOs with texture attachments, but share the same | ||
| // depth rbo between them. | ||
| { | ||
| MBGL_DEBUG_GROUP(parameters.context, "3d"); | ||
| parameters.backend.bind(); | ||
|
There was a problem hiding this comment. I believe this call isn't necessary. Binding the backend means that the default framebuffer is going to be bound and that its viewport will be set accordingly. However, in the first call to |
||
| parameters.pass = RenderPass::Pass3D; | ||
|
|
||
| const auto size = parameters.context.viewport.getCurrentValue().size; | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. @kkaefer interesting — when I change this to mapbox-gl-native/src/mbgl/renderer/renderer_impl.cpp Lines 392 to 404 in 0efb0b7 on my iPhone, it appears you're right that this value is different, but that the dimensions of |
||
|
|
||
| if (!parameters.staticData.depthRenderbuffer || | ||
| parameters.staticData.depthRenderbuffer->size != size) { | ||
| parameters.staticData.depthRenderbuffer = | ||
| parameters.context.createRenderbuffer<gl::RenderbufferType::DepthComponent>(size); | ||
|
There was a problem hiding this comment. Before creating any objects here, we should first check if there are even extrusion layers in the stylesheet. Otherwise, all of those resources + backend binding won't be necessary. |
||
| } | ||
| parameters.staticData.depthRenderbuffer->dirty = true; | ||
|
|
||
| uint32_t i = static_cast<uint32_t>(order.size()) - 1; | ||
| for (auto it = order.begin(); it != order.end(); ++it, --i) { | ||
| parameters.currentLayer = i; | ||
| if (it->layer.hasRenderPass(parameters.pass)) { | ||
| MBGL_DEBUG_GROUP(parameters.context, it->layer.getID()); | ||
| it->layer.render(parameters, it->source); | ||
| } | ||
| } | ||
|
|
||
| // The main backend/framebuffer will be rebound in the clear step. | ||
| } | ||
|
|
||
| // - CLEAR ------------------------------------------------------------------------------------- | ||
| // Renders the backdrop of the OpenGL view. This also paints in areas where we don't have any | ||
| // tiles whatsoever. | ||
|
|
@@ -385,15 +426,6 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) { | |
| // - CLIPPING MASKS ---------------------------------------------------------------------------- | ||
| // Draws the clipping masks to the stencil buffer. | ||
| { | ||
| MBGL_DEBUG_GROUP(parameters.context, "clipping masks"); | ||
|
|
||
| static const style::FillPaintProperties::PossiblyEvaluated properties {}; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please document what "dirty" means in this context? (or alternatively write convenience/wrapper functions around clearing the renderbuffer).