Refactor GL context creation and headless rendering #6596

kkaefer · 2016-10-07T10:14:13Z

We currently have a View object, which is a weird mix of GL context creating, framebuffer management, as well as size + pixel ratio management. Instead, I propose refactoring this as follows:

Backend creates/destroys an OpenGL context (if needed). Contains the gl::Context object. Must be used to initialize Map and Canvas objects. A Backend can support multiple Map objects.
Map objects are initialized with a fixed pixel ratio; we don't support changing pixel ratio on-the-fly anyway because it is tied to what resources are being loaded.
Map objects can be resized, in logical units (unrelated to the pixel ratio). This affects what tiles are being loaded. It is not tied to a framebuffer or View object.
Map objects can take a lambda/std::function that is called when the map should be redrawn
View: abstract class that represents something that we can draw upon. Every render/renderStill call gets passed a View object. The Map and View objects must be initialized with the same Backend object.
- A default is present on platform bindings like iOS and macOS, this encapsulates the default framebuffer provided by the system
- OffscreenView is an implementation that draws to the framebuffer. Supports methods for obtaining the image rendered.
- OffscreenTexture is similar to OffscreenView, except that it draws to a texture which can then be used for rendering to other views.

Open questions:

Continuously rendering map objects need a way to "invalidate" the map, i.e. tell the rendering driver to request a new frame
Multiple headless maps should be able to run on the same thread, and when calling renderStillImage, it doesn't need to block as long as the event loop continues to run. We should only issue GL calls when we are finally drawing the image

/cc @mikemorris @jfirebaugh

kkaefer · 2016-10-05T16:26:12Z

The main issue I'm currently seeing is the coupling between an OpenGL context and an object we can render to. It's currently one object, represented by the View object. This duality mirrors the GLFW API, but it's not actually how headless rendering works, and even when we have one main object, we could still have other Canvas objects that we render to.

mikemorris · 2016-10-05T16:48:32Z

A few tasks that I've previously started that I think would be related to this work:

Rendering to a single GL context in HeadlessView Render to a single context in HeadlessView #6212 render to a single static context in HeadlessView #6278
Async glReadPixels() in HeadlessView Async glReadPixels() in HeadlessView? #477 Use Pixel Buffer Object for async glReadPixels #6598
Expose bindings to NodeView and pass in (from pool) instead of resizing framebuffer Expose NodeView and recycle instead of resizing #3837 [DO NOT MERGE] expose NodeView to JavaScript #2310
Reuse PremultipliedImage in HeadlessViewReuse image buffer in HeadlessView #5476 - I started reworking this locally to expose PremultipliedImage in the Node.js bindings to allow an appropriate sizes canvas to be passed in with a map.render call.

mention-bot · 2016-10-07T10:14:15Z

@kkaefer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jfirebaugh, @mikemorris and @tmpsantos to be potential reviewers.

kkaefer · 2016-10-13T11:31:23Z

@jfirebaugh @tmpsantos @brunoabinader @mikemorris This is finally in a good shape and ready for review. I implemented pretty much exactly what is described above.

In particular, the only remaining (new) method of a View object is bind(). It is called whenever the Map needs to bind that view because it wants to draw on it. When the system provides a default framebuffer (i.e. all UI platforms), we are changing the gl::Context's state to reflect the framebuffer and viewport binding. In particular, the bindFramebuffer and viewport state values in gl::Context have a special treatment because they are always used in conjunction in our code. We previously didn't have a way to explicitly bind the default view because we never bound other framebuffers so far, but we are going to in the future.

This pull request unlocks a few other tasks that aren't part of this PR yet:

Remove activate()/deactivate() from the Backend object(?)
Share Backend objects across multiple Map/View objects to prevent excessive context switching in the Node.js use case (where every Map object currently has its own GL context)
Move invalidate() and notifyMapChange elsewhere; they shouldn't be part of Backend.

jfirebaugh · 2016-10-14T18:09:29Z

Move invalidate() and notifyMapChange elsewhere; they shouldn't be part of Backend.

Refs #6383

jfirebaugh

Kudos, this looks like a great refactor. I like the fact that neither View nor Backend have a dependency on Map, as View did before. This eliminates a troublesome circular dependency.

Just some small nits and one TODO to finish up.

jfirebaugh · 2016-10-14T17:29:19Z

benchmark/api/query.benchmark.cpp

-#include <mbgl/platform/default/headless_display.hpp>
-#include <mbgl/platform/default/headless_view.hpp>
+#include <mbgl/platform/default/headless_backend.hpp>
+#include <mbgl/platform/default/offscreen_view.hpp>


Would it make sense as a followup to rename Headless ⇢ Offscreen for consistency?

I didn't rename this to have a difference in naming: Offscreen rendering doesn't necessarily imply headless rendering (and vice versa): We can use Offscreen views/texturs in regular rendering as well.

jfirebaugh · 2016-10-14T17:36:23Z

platform/android/src/style/layers/custom_layer.cpp

@@ -26,7 +26,8 @@ namespace android {
    void CustomLayer::update(jni::JNIEnv&) {
        Log::Debug(mbgl::Event::JNI, "Updating map");
        if (map) {
-            map->update(mbgl::Update::Repaint);
+            // TODO: trigger a rerender


Need this implemented before merge.

implemented

jfirebaugh · 2016-10-14T17:46:16Z

src/mbgl/gl/framebuffer.hpp

+
+class Framebuffer {
+public:
+    // Framebuffer(std::array<uint16_t, 2> size_, gl::UniqueFramebuffer framebuffer_)


jfirebaugh · 2016-10-14T18:00:10Z

include/mbgl/map/backend.hpp

+
+namespace mbgl {
+
+class Map;


jfirebaugh · 2016-10-14T18:01:24Z

include/mbgl/map/view.hpp

 #include <mbgl/util/image.hpp>

 #include <array>
-#include <functional>
-#include <memory>

 namespace mbgl {

 class Map;


jfirebaugh · 2016-10-14T18:01:51Z

include/mbgl/map/view.hpp

@@ -1,63 +1,22 @@
 #pragma once

-#include <mbgl/map/change.hpp>
-#include <mbgl/util/chrono.hpp>
 #include <mbgl/util/image.hpp>

 #include <array>


jfirebaugh · 2016-10-14T18:02:06Z

include/mbgl/map/view.hpp

@@ -1,63 +1,22 @@
 #pragma once

-#include <mbgl/map/change.hpp>
-#include <mbgl/util/chrono.hpp>
 #include <mbgl/util/image.hpp>


kkaefer · 2016-10-25T16:14:04Z

@jfirebaugh please review again

kkaefer force-pushed the 6596-refactor-gl-context-creation branch 9 times, most recently from 789daa1 to e307fdf Compare October 13, 2016 11:31

kkaefer force-pushed the 6596-refactor-gl-context-creation branch from e307fdf to 02e6151 Compare October 13, 2016 12:17

jfirebaugh suggested changes Oct 14, 2016

View reviewed changes

kkaefer mentioned this pull request Oct 24, 2016

Add root object for 3D opacity (and other properties) mapbox/mapbox-gl-style-spec#545

Closed

kkaefer added 2 commits October 24, 2016 16:09

[core] separate Backend from View for headless rendering

7ef73b2

[core] move gl::Context to Backend and refactor View

6a555ce

kkaefer force-pushed the 6596-refactor-gl-context-creation branch from 02e6151 to 6a555ce Compare October 25, 2016 00:13

jfirebaugh approved these changes Oct 25, 2016

View reviewed changes

kkaefer merged commit a4d259c into master Oct 25, 2016

kkaefer deleted the 6596-refactor-gl-context-creation branch October 25, 2016 20:52

This was referenced Oct 25, 2016

C++ bindings for OpenGL #6468

Merged

[linux] Overdraw debug is broken #6799

Closed

lucaswoj mentioned this pull request Feb 1, 2017

Correct depth rendering for multiple fill extrusions mapbox/mapbox-gl-js#4134

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor GL context creation and headless rendering #6596

Refactor GL context creation and headless rendering #6596

kkaefer commented Oct 7, 2016 •

edited

Loading

kkaefer commented Oct 5, 2016

mikemorris commented Oct 5, 2016 •

edited

Loading

mention-bot commented Oct 7, 2016

kkaefer commented Oct 13, 2016

jfirebaugh commented Oct 14, 2016

jfirebaugh left a comment

jfirebaugh Oct 14, 2016

kkaefer Oct 14, 2016

jfirebaugh Oct 14, 2016

kkaefer Oct 25, 2016

jfirebaugh Oct 14, 2016

jfirebaugh Oct 14, 2016

jfirebaugh Oct 14, 2016

jfirebaugh Oct 14, 2016

jfirebaugh Oct 14, 2016

kkaefer commented Oct 25, 2016

Refactor GL context creation and headless rendering #6596

Refactor GL context creation and headless rendering #6596

Conversation

kkaefer commented Oct 7, 2016 • edited Loading

kkaefer commented Oct 5, 2016

mikemorris commented Oct 5, 2016 • edited Loading

mention-bot commented Oct 7, 2016

kkaefer commented Oct 13, 2016

jfirebaugh commented Oct 14, 2016

jfirebaugh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkaefer commented Oct 25, 2016

kkaefer commented Oct 7, 2016 •

edited

Loading

mikemorris commented Oct 5, 2016 •

edited

Loading