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

Asynchronous rendering #8820

Closed
24 of 28 tasks
ivovandongen opened this issue Apr 25, 2017 · 10 comments
Closed
24 of 28 tasks

Asynchronous rendering #8820

ivovandongen opened this issue Apr 25, 2017 · 10 comments
Assignees
Labels
Core The cross-platform C++ core, aka mbgl

Comments

@ivovandongen
Copy link
Contributor

ivovandongen commented Apr 25, 2017

We want to be able to offload rendering onto a separate thread to free up the main thread on some platforms (most notably Android and Qt). The main thread should, as much as possible, be used only for user input and drawing so that a high frame-rate can be maintained.

A lot of work is already delegated to dedicate workers, but in order to offer a smooth experience on somewhat more lightweight hardware we need to eliminate some of the processing currently done on the main thread. Most notably:

  • Rendering (OpenGL)
  • DDS (evaluation of source functions)

Not all platforms need/should use a separate thread for rendering, so we want to enable a model where either is possible. Also, as an additional hurdle, on some platforms we want to be able to integrate with existing platform specific solutions such as the GLSurfaceView which come with a dedicated render thread that prohibits the use of a run loop as it blocks on a mutex.

In order to support this, we probably need a model where we utilise three threads:

  • Main thread for integration with the UI framework / exposes our public API
  • Supporting thread for orchestration of the render data preparation and render thread wake-up
  • The render thread

Eg. Something like (very high-level):
staged_rendering 1

Preparation
Currently a couple of classes contain data that is needed on both the main thread (to offer our public, synchronous api) and on the would-be render thread. In preparation of supporting a render thread these classes need to be split up accordingly. In the case of Layers and Sources, we've decided to aim for a model where we have a parallel tree of Render items which have a reference to their counterpart's immutable implementation class.

data_structures 2

Related:

Currently underway

  • Define Renderer/Renderer::Impl pair:
    • SDK code implementing renderer thread will own std::unique_ptr<Renderer> provided by core
    • Renderer::Impl to hide details from public header
    • Public Renderer methods:
      • update(const UpdateParameters&) (UpdateParameters forward declared)
      • render(const RenderParameters&) (RenderParameters forward declared)
    • Probably merge Painter and RenderStyle into Renderer::Impl
  • Define interfaces to be implemented by SDKs to provide background rendering (or not). Methods:
    • setRenderer(std::unique_ptr<Renderer>) (might be null to "reset" renderer during shutdown)
    • update(std::unique_ptr<UpdateParameters>)
    • render(std::unique_ptr<RenderParameters>)
    • Latter two are expected to forward to Renderer either immediately (synchronously) or via sending message to rendering thread (asynchronously)
    • Also need some way for Renderer to trigger a re-render when needed (animation)
    • Also need some way to forward "Low Memory" notifications
    • Also need to deal with RenderStyleObserver/MapObserver notifications
    • Also need query APIs, w/ potential cross-thread synchronization

Next steps

  • Actually implement asynchronicity in the RenderFrontend for Android
  • Handle context loss on Android -- I (@jfirebaugh) vote for just recreating a totally fresh Renderer if we lose the context. The alternative is to keep a copy of all bucket data in main memory, so that we can recreate the GL buffers from it. This would ~2x the memory usage of maps, just to get somewhat faster reloads when the context comes back -- not worth it IMO.
  • TODO: can we refactor still rendering logic as an implementation of this new interface?

cc @mapbox/gl

@zugaldia
Copy link
Member

Related (follow-up) work on Android to implement GlSurfaceView to contain the map view: #5766.

@jfirebaugh
Copy link
Contributor

Summary of the task of splitting SpriteAtlas: we want to start treating the set of images specified by the style more similarly to how we treat layers and sources. I.e. Style should own a std::vector<std::unique_ptr<Image>>. The rest of SpriteAtlas is rendering related and will belong to the RenderStyle. We'll bridge access to Images between Style and the SpriteAtlas in RenderStyle using the same mechanism as for Layers and Sources (most likely via sharing references to immutable objects).

@tmpsantos
Copy link
Contributor

What is not clear from this design is how something like "query rendered features" is going to work synchronously. We need the "evaluated" state for that, right?

@ivovandongen
Copy link
Contributor Author

@tmpsantos Very good question. ATM I see two options:

  • Keep the synchronous versions of query*Features in place and make a blocking call from the main thread to whichever thread has the required data.
  • Make the calls a-synchronous, so not to block.

We can support both of course, but moving to the later would have my preference.

@jfirebaugh
Copy link
Contributor

jfirebaugh commented May 4, 2017

I did a rough dry run at splitting Style and RenderStyle just to see what will need to change to allow that to happen. Here's my list. Items marked with ℹ️ depend on having infrastructure in place for immutable objects.

  • Promote add/remove/getImage and the style::Image map itself to Style, and make Style responsible for loading the sprite PNG and JSON, parceling out the individual images, and filling up the map.
  • GlyphAtlas will belong to RenderStyle, which won't be able to propagate onGlyphsLoaded/Error messages to a parent observer. Investigate whether this is necessary/how to eliminate. (Likely do the same for onSpriteLoaded/Error for consistency, even though it could be preserved.)
  • What's lastError used for? Should it belong to Style or RenderStyle exclusively, or does it need to be split?
  • Both Style and RenderStyle need isLoaded and dumpDebugLogs methods. In the places that call Style::isLoaded, add a call to RenderStyle::isLoaded(); likewise for dumpDebugLogs.
  • ℹ️ TileParameters will have a RenderStyle reference rather than a Style reference. GeometryTile will need to iterate over RenderLayers rather than Layers, obtain an immutable Impl for each, and pass them to the worker rather than a copy of the Layer.
  • ℹ️ GlyphAtlas will need access to a glyph URL (by immutable reference).
  • ℹ️ SpriteAtlas will need access to images (by immutable reference).
  • ℹ️ Replace in-place updates of renderSources and renderLayers (in setJSON and {add,remove}{Source,Layer}) with update-time updates based on diffing immutables.
  • ℹ️ Replace updateBatch mechanism with immutable layer diffing.
  • ℹ️ update will move to RenderStyle. Replace direct access to sources, layers, images, transition options, classes, and light with immutable references passed via UpdateParameters.

I'm continuing to work on prerequisites for the ℹ️ items, but I could use help on the others.

@ivovandongen
Copy link
Contributor Author

@jfirebaugh

Promote add/remove/getImage and the style::Image map itself to Style, and make Style responsible for loading the sprite PNG and JSON, parceling out the individual images, and filling up the map.

I will take care of that while looking at the SpriteAtlas

GlyphAtlas will belong to RenderStyle, which won't be able to propagate onGlyphsLoaded/Error messages to a parent observer. Investigate whether this is necessary/how to eliminate

Depending on the concurrency model / necessity, we could retain this. I was imagining re-using the actor model for communication between Style/RenderStyle. In this case we can propagate these messages back from RenderStyle to Style.

I'm finishing up the Light changes you requested now and then will start to pick up the items you list one by one.

@ivovandongen
Copy link
Contributor Author

screen shot 2017-05-05 at 17 09 45

Promote add/remove/getImage and the style::Image map itself to Style, and make Style responsible for loading the sprite PNG and JSON, parceling out the individual images, and filling up the map.

Looking into cutting up the SpriteAtlas; it seems that we would need a way for the RenderStyle-to-be to communicate back to the Style to request images since the GeometryTiles (loaded by RenderStyle) require it. Doesn't seem to be a way around this if we coordinate loading the images on the main thread as we need access to both images added through the api and images loaded through the sprite url for {add/remove/get}Image.

We should pass the RenderStyle an interface (like ImageProvider) to use when requesting images, which we could implement differently later on to abstract over the threads Style and RenderStyle live on.

@jfirebaugh
Copy link
Contributor

jfirebaugh commented May 5, 2017

we would need a way for the RenderStyle-to-be to communicate back to the Style to request images

No, communication is one way; there's no communication in RenderStyle direction. All data will be provided in the StyleRender direction via immutable references, including for images. This means we probably need to split style::Image and style::Image::Impl, so we can provide Immutable<style::Image::Impl>s to the RenderStyle.

Like it's responsible for loading source TileJSON, Style will be responsible for loading the sprite. (In a future revision of the style specification I hope that both sprite images and source TileJSON will be mandatorily inline.)

@kkaefer
Copy link
Contributor

kkaefer commented May 9, 2017

I added a write-up of lifetimes and object ownership in GLSurfaceView over in #5766 (comment)

@jfirebaugh
Copy link
Contributor

jfirebaugh commented Jun 8, 2017

I've been keeping a list of potential followup changes. These are things that I've noticed when working on this that would be nice to do at some point but so far aren't directly necessary:

  • Reorganize src directory structure
    • move more files to renderer directory
    • organize primarily by source/layer subtype, rather than functional division (e.g. put foo_bucket.cpp, foo_render_layer.cpp, foo_program.cpp, foo_shaders.cpp all in the same directory)
  • Rename/refactor Transform/TransformStateCamera/Immutable<Camera::Impl>
  • Combine RenderLayer::{transition,evaluate,setImpl} into a single update method (like RenderSource)
  • Combine RenderStyle::update and RenderStyle::getRenderData?
  • Rename MapModeRenderMode
  • Move SourceType enum to source_type.hpp (like layer_type.hpp)
  • const correct getRenderSource
  • Guard pointer dereferences in RenderStyle::queryRenderedFeatures
  • Disallow use of addImage to update existing images -- should be a separate API
  • Extend diff algorithm to detect moved layers
  • Remove order-sensitivity in source and image diffing (order doesn't matter)
  • Could removing a symbol layer cause other symbol layers to need placement?
  • Switch to shared pointers for Layer/Source/Image (related to Crash when mutating sources and layers that were added to a previously loaded style #8882)
  • Make Style a public API, remove runtime styling APIs from Map (related to Initializers for MGLStyle #6386)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

No branches or pull requests

5 participants