Per review:

* Add convenience methods to clarify use of renderbuffer::dirty * Add OffscreenTexture constructor with rbo ref param so that initial framebuffer creation/binding can bind depth rbo at the same time
mapbox · Sep 8, 2017 · 0efb0b7 · 0efb0b7
1 parent 044617d
commit 0efb0b7
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 26 deletions.
diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp
@@ -454,10 +454,19 @@ Framebuffer Context::createFramebuffer(const Texture& color) {
     return { color.size, std::move(fbo) };
 }
 
-void
-Context::attachRenderbuffer(const Renderbuffer<RenderbufferType::DepthComponent>& depthTarget) {
-    MBGL_CHECK_ERROR(glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, depthTarget.renderbuffer));
+Framebuffer
+Context::createFramebuffer(const Texture& color,
+                           const Renderbuffer<RenderbufferType::DepthComponent>& depth) {
+    if (color.size != depth.size) {
+        throw std::runtime_error("Renderbuffer size mismatch");
+    }
+    auto fbo = createFramebuffer();
+    bindFramebuffer = fbo;
+    MBGL_CHECK_ERROR(glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+                                            color.texture, 0));
+    MBGL_CHECK_ERROR(glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, depth.renderbuffer));
     checkFramebuffer();
+    return { color.size, std::move(fbo) };
 }
 
 UniqueTexture

diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp
@@ -96,8 +96,8 @@ class Context : private util::noncopyable {
     Framebuffer createFramebuffer(const Texture&,
                                   const Renderbuffer<RenderbufferType::DepthStencil>&);
     Framebuffer createFramebuffer(const Texture&);
-
-    void attachRenderbuffer(const Renderbuffer<RenderbufferType::DepthComponent>&);
+    Framebuffer createFramebuffer(const Texture&,
+                                  const Renderbuffer<RenderbufferType::DepthComponent>&);
 
     template <typename Image,
               TextureFormat format = Image::channels == 4 ? TextureFormat::RGBA

diff --git a/src/mbgl/gl/renderbuffer.hpp b/src/mbgl/gl/renderbuffer.hpp
@@ -16,6 +16,15 @@ class Renderbuffer {
     using type = std::integral_constant<RenderbufferType, renderbufferType>;
     Size size;
     UniqueRenderbuffer renderbuffer;
+
+    void shouldClear(bool clear) {
+        dirty = clear;
+    }
+    bool needsClearing() {
+        return dirty;
+    }
+
+private:
     bool dirty;
 };
 

diff --git a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp
@@ -55,17 +55,18 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters, RenderSource*
         const auto size = parameters.context.viewport.getCurrentValue().size;
 
         if (!renderTexture || renderTexture->getSize() != size) {
-            renderTexture = OffscreenTexture(parameters.context, size);
+            renderTexture = OffscreenTexture(parameters.context, size, *parameters.staticData.depthRenderbuffer);
         }
 
         renderTexture->bind();
-        renderTexture->attachRenderbuffer(*parameters.staticData.depthRenderbuffer);
 
-        parameters.context.setStencilMode(gl::StencilMode::disabled());
         optional<float> depthClearValue = {};
-        if (parameters.staticData.depthRenderbuffer->dirty) depthClearValue = 1.0;
+        if (parameters.staticData.depthRenderbuffer->needsClearing()) depthClearValue = 1.0;
+        // Flag the depth buffer as no longer needing to be cleared for the remainder of this pass.
+        parameters.staticData.depthRenderbuffer->shouldClear(false);
+
+        parameters.context.setStencilMode(gl::StencilMode::disabled());
         parameters.context.clear(Color{ 0.0f, 0.0f, 0.0f, 0.0f }, depthClearValue, {});
-        parameters.staticData.depthRenderbuffer->dirty = false;
 
         if (evaluated.get<FillExtrusionPattern>().from.empty()) {
             for (const RenderTile& tile : renderTiles) {

diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp
@@ -401,7 +401,7 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {
             parameters.staticData.depthRenderbuffer =
                 parameters.context.createRenderbuffer<gl::RenderbufferType::DepthComponent>(size);
         }
-        parameters.staticData.depthRenderbuffer->dirty = true;
+        parameters.staticData.depthRenderbuffer->shouldClear(true);
 
         uint32_t i = static_cast<uint32_t>(order.size()) - 1;
         for (auto it = order.begin(); it != order.end(); ++it, --i) {

diff --git a/src/mbgl/util/offscreen_texture.cpp b/src/mbgl/util/offscreen_texture.cpp
@@ -15,11 +15,21 @@ class OffscreenTexture::Impl {
         : context(context_), size(std::move(size_)) {
         assert(!size.isEmpty());
     }
+    Impl(gl::Context& context_,
+         const Size size_,
+         gl::Renderbuffer<gl::RenderbufferType::DepthComponent>& depth_)
+        : context(context_), size(std::move(size_)), depth(std::move(depth_)) {
+        assert(!size.isEmpty());
+    }
 
     void bind() {
         if (!framebuffer) {
             texture = context.createTexture(size, gl::TextureFormat::RGBA);
-            framebuffer = context.createFramebuffer(*texture);
+            if (depth) {
+                framebuffer = context.createFramebuffer(*texture, *depth);
+            } else {
+                framebuffer = context.createFramebuffer(*texture);
+            }
         } else {
             context.bindFramebuffer = framebuffer->framebuffer;
         }
@@ -29,13 +39,6 @@ class OffscreenTexture::Impl {
         context.viewport = { 0, 0, size };
     }
 
-    void attachRenderbuffer(gl::Renderbuffer<gl::RenderbufferType::DepthComponent>& renderbuffer) {
-        if (!attachedRenderbuffer || attachedRenderbuffer != &renderbuffer.renderbuffer) {
-            context.attachRenderbuffer(renderbuffer);
-            attachedRenderbuffer = &renderbuffer.renderbuffer;
-        }
-    }
-
     PremultipliedImage readStillImage() {
         return context.readFramebuffer<PremultipliedImage>(size);
     }
@@ -54,7 +57,7 @@ class OffscreenTexture::Impl {
     const Size size;
     optional<gl::Framebuffer> framebuffer;
     optional<gl::Texture> texture;
-    gl::UniqueRenderbuffer* attachedRenderbuffer = nullptr;
+    optional<gl::Renderbuffer<gl::RenderbufferType::DepthComponent>> depth;
 };
 
 OffscreenTexture::OffscreenTexture(gl::Context& context,
@@ -63,17 +66,19 @@ OffscreenTexture::OffscreenTexture(gl::Context& context,
     assert(!size.isEmpty());
 }
 
+OffscreenTexture::OffscreenTexture(gl::Context& context,
+                                   const Size size,
+                                   gl::Renderbuffer<gl::RenderbufferType::DepthComponent>& renderbuffer)
+    : impl(std::make_unique<Impl>(context, std::move(size), renderbuffer)) {
+    assert(!size.isEmpty());
+}
+
 OffscreenTexture::~OffscreenTexture() = default;
 
 void OffscreenTexture::bind() {
     impl->bind();
 }
 
-void OffscreenTexture::attachRenderbuffer(
-    gl::Renderbuffer<gl::RenderbufferType::DepthComponent>& renderbuffer) {
-    impl->attachRenderbuffer(renderbuffer);
-}
-
 PremultipliedImage OffscreenTexture::readStillImage() {
     return impl->readStillImage();
 }

diff --git a/src/mbgl/util/offscreen_texture.hpp b/src/mbgl/util/offscreen_texture.hpp
@@ -13,12 +13,14 @@ class OffscreenTexture {
 public:
     OffscreenTexture(gl::Context&,
                      Size size = { 256, 256 });
+    OffscreenTexture(gl::Context&,
+                     Size size,
+                     gl::Renderbuffer<gl::RenderbufferType::DepthComponent>&);
     ~OffscreenTexture();
     OffscreenTexture(OffscreenTexture&&);
     OffscreenTexture& operator=(OffscreenTexture&&);
 
     void bind();
-    void attachRenderbuffer(gl::Renderbuffer<gl::RenderbufferType::DepthComponent>&);
 
     PremultipliedImage readStillImage();