Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial integration of deck.gl with Google Maps #2179

Closed
wants to merge 2 commits into from

Conversation

MeTaNoV
Copy link

@MeTaNoV MeTaNoV commented Aug 9, 2018

For #1122 , provide an example to be updated of the integration of deck.gl with Google Maps.

@Pessimistress and @ibgreen , feel free to use your (mapping) expertise to correct this semi working example.

@CLAassistant
Copy link

CLAassistant commented Aug 9, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very promising.

I started playing around with the branch to see if I could fix the tearing.

Right now the main obstacle is that when but almost immediately ran out of quota on my Google API key.

Would need to find time to understand how to deal with that, if we need to set up a billable account just to do development on this, etc.

app: resolve('./app.js')
},

resolve: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we can drop this resolve clause.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

</div>
<script src='app.js'></script>
<!-- Replace GOOGLE_MAPS_API_KEY with your own key -->
<script async defer src="https://maps.googleapis.com/maps/api/js?key=GOOGLE_MAPS_API_KEY&libraries=visualization&callback=initMap"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about loading this script programmatically instead of using CSS, and using a webpack environment plugin to pick up the key from the environment, like we do in our mapbox examples.

That would allow us to continuously test this without constantly modifying the source.

function loadScript(url) {
  const script = document.createElement( 'script' );
  script.id = 'decoder_script';
  script.type = 'text/javascript';
  script.src = url;

  const head = document.getElementsByTagName( 'head' )[ 0 ];
  head.appendChild( script );

  return new Promise(resolve => {
    script.onload = resolve;
  });
};

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A much more flexible way, thanks for the hint!

- load Google Maps JS API programmatically
@MeTaNoV
Copy link
Author

MeTaNoV commented Aug 10, 2018

@ibgreen , I'll probably later add some controls to be able to change the pitch for validation purpose, since it seems to affect the whole rendering in some dramatic way.

@ibgreen
Copy link
Collaborator

ibgreen commented Aug 13, 2018

@MeTaNoV I am fine with landing this now. However until we have fixed the "tearing" let's put it in test/apps rather than examples. It will be confusing to users to have non-functional code in examples.

I'll take a look if I can find the time to understand how to get a working Google Map API key.

@donmccurdy
Copy link
Collaborator

I had some trouble running this example locally — ran into something like #2344, and wasn't sure how to get the release branch running when I'd just downloaded the example folder alone. Ended up fixing it by building with Browserify and a small hack. Just to make sure I get the same result, is this the "tearing" you mention?

d7998402-ec5d-4121-8b54-4d37f2372c9e-37075-000735573fa87caa

If so, it may be worth looking at Google Maps "new renderer" documentation:

OverlayViews can hook into the animation frame loop, and render at fractional-zoom levels. This allows arbitrary DOM elements to animate in sync with the map.

It appears that google.maps.OverlayView is the preferred way to hook into the animation frame loop. I tried implementing a version of this PR using OverlayView's draw() method to synchronize viewports, and ended up with this:

d42bb665-a36d-45e0-b7a6-bc6309fa5934-37075-000736284f01ef36

demo | source

The Google Maps and DeckGL viewports are in sync once animation finishes, but pretty clearly DeckGL is at least a frame behind during pan and zoom gestures. I assume Google Maps and DeckGL are both using rAF internally, but it's possible DeckGL is rendering before Google Maps has a chance to update it. It's also possible that the overlay.draw() function is not being invoked soon enough for DeckGL to apply it to the current frame. Is it possible to synchronously force DeckGL to render, if only to debug this and figure out what's going on? Or any other ideas?

@Pessimistress
Copy link
Collaborator

@donmccurdy In your code sample you are using google maps' interaction handling and setting deck.gl's view state on its callback.

Instead, you need to disable the interaction in google maps and use deck.gl's map controls. You can set google maps' map center in onViewStateChange.

The same technic is applied when using Mapbox. Basically deck.gl can be used as a stateless component, but Mapbox and google map cannot. When you get the draw callback, the base map has already rerendered.

@donmccurdy
Copy link
Collaborator

Thanks @Pessimistress — I think a core issue here is that Maps API does not support fractional zoom via .setZoom() or .setOptions(). It uses fractional zoom internally, but even in my example above I have to do some math to compute the fractional zoom, because .getZoom() returns integers throughout zoom transitions.

Going back to the original PR, you can see this clearly while zooming slowly using a trackpad. DeckGL zooms smoothly and then Maps API very suddenly jumps:

8b8140e2-52d0-4760-be1a-eeb50b4c846d-57905-00074448b1ae6893

The option of managing the DeckGL animation loop manually, similar to three.js' renderer.render(...), might address this. Whether that's possible, or even fits the deckgl usage model, I don't know.

That issue aside, I haven't actually seen any examples of Maps API synced correctly with a fixed-position overlay even since it (theoretically) became possible with the new renderer. If there isn't an easy way to test the idea with DeckGL, maybe a proof-of-concept with plain JS Canvas or WebGL would help to figure out whether the Maps API animation loop can be used this way.

@Pessimistress
Copy link
Collaborator

@donmccurdy You can try manually trigger redraw using deck._drawLayers(). If it works we can work out some public API.

@donmccurdy
Copy link
Collaborator

That's deck.layerManager.drawLayers({viewports: deck.getViewports()}), I assume?

Updated demo and source. Initial reactions:

  1. Zooming with UI buttons stays in sync (yay!)
  2. Zooming with trackpad and panning both still show noticeable lag.

I'm not sure what to make of (2). It's possible that draw() is invoked at a different time depending on the input method, or perhaps projection.fromLatLngToDivPixel behaves differently. :/

@Pessimistress
Copy link
Collaborator

Pessimistress commented Oct 23, 2018

The overlay matches the base map eventually, so the projection is calculated correctly. The timing issue will be very hard to work around though. You may have better luck requesting the fractional zoom feature from Google.

(Just a thought: does fitBounds() give you fractional zoom?)

@donmccurdy
Copy link
Collaborator

I spent a while debugging the demo above, using this hack to force DeckGL to render in the Maps API animation loop:

deck.setProps({viewState: { zoom, latitude, longitude }});
if (deck.layerManager) {
  deck.animationLoop._setupFrame();
  deck.animationLoop._updateCallbackData();
  deck.animationLoop.onRender(deck.animationLoop.animationProps);
}

Inspecting each render call, it looks very much like Maps API is behaving as documented — rendering happens in an rAF loop, and the code above is invoked at the same time. Despite that, the DeckGL overlay appears to lag behind during panning animation... would DeckGL apply any easing when I call deck.setProps({viewState})? If Maps API controls the camera, the fact that DeckGL lags slightly suggests something isn't fully synchronous here.

Unfortunately fitBounds() and the other available methods of setting Maps API state do not support fractional zoom. 😕

@Pessimistress
Copy link
Collaborator

I stepped through your code - it seems that when your overlay.draw callback is called, zoom is always an integer. Maybe you can call deck.setProp with a transition that matches the base map?

@donmccurdy
Copy link
Collaborator

I can't reproduce that, and do see floating-point zoom values. For example, setting a conditional breakpoint below:

screen shot 2018-11-28 at 10 53 41 am

Produces non-integers during a zoom transition:

screen shot 2018-11-28 at 10 53 33 am

Maps API does not allow the map to remain in a non-integer zoom level state outside these transitions, so scrolling with the mouse wheel for example will always end up with an integer zoom.

@Pessimistress
Copy link
Collaborator

Pessimistress commented Nov 28, 2018

@donmccurdy That was very helpful. I dig into getMatrix() and compared the projection returned by Maps API and the viewport passed to Deck. The adjustment you apply to map.getZoom() is correct, but map.getCenter() needs to be adjusted as well. Could you try this:

// const center = map.getCenter();  // this does not return the in-transition map center
const center = projection.fromDivPixelToLatLng({x: width / 2, y:height / 2});
const longitude = center.lng();
const latitude = center.lat();

This also explains why the lag is not shown when you use the navigation buttons to zoom.

@MeTaNoV
Copy link
Author

MeTaNoV commented Nov 30, 2018

@donmccurdy , thanks for your additional work. I noticed that your link to https://developers.google.com/maps/documentation/javascript/new-renderer is not working, didn't manage to find a good one after a quick search...
Also, in your source code, you give away a (valid?) google API Key, it might be wise to remove it... :)

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 1, 2018

I noticed that your link ... is not working

@MeTaNoV Oh, hm. I guess it is just the default renderer now.

Also, in your source code, you give away a (valid?) google API Key,

It's fine, the GCP API keys are typically visible in client code. I can add referrer or other API restrictions if someone starts abusing it. 😅

I dig into getMatrix() and compared the projection returned by Maps API and the viewport passed to Deck. The adjustment you apply to map.getZoom() is correct, but map.getCenter() needs to be adjusted as well.

Great insight, thank you! We need to use fromContainerPixelToLatLng, not fromDivPixelToLatLng, but that seems to solve the remaining disconnect with panning/zooming. 🎉

Here's an updated demo and source.

3da286a0-b0e4-4f62-97e4-bb3d37c58615-14345-00004ec9497495b2

@Pessimistress
Copy link
Collaborator

@donmccurdy That looks like it's working!

@MeTaNoV Maybe you guys can work together to update this PR?

@Pessimistress
Copy link
Collaborator

@donmccurdy Is the manual re-render still necessary with this change?

@donmccurdy
Copy link
Collaborator

Is the manual re-render still necessary with this change?

I still need it, yeah. The DeckGL rAF is being invoked before the Maps API rAF, so it ends up rendering with the previous frame's state otherwise. If there were some way to reorder the draws that would probably do the trick, but seems fragile.

@Pessimistress
Copy link
Collaborator

Ok, in that case, I think it's best if the Deck class offers a public API to do this, something like deck.redraw()

@donmccurdy
Copy link
Collaborator

Note that an rAF animation loop is still running for both DeckGL and Maps API in my demo. In each frame, (1) the DeckGL rAF fires first, sees that no state has changed, and exits early. (2) the Maps API rAF fires, modifies DeckGL state, and tells DeckGL to draw. So the actual WebGL draw happens only once, happily.

Will that be OK for examples like TripsLayer or WindLayer, where there's animation apart from the viewport state? I'm assuming that as long as we update the timestamp during the Maps API rAF, DeckGL will continue to do nothing in its own animation loop, so we don't double-draw. If that's an unsafe assumption, perhaps we need a way to disable the default animation loop, in addition to the .redraw() method.

@MeTaNoV
Copy link
Author

MeTaNoV commented Dec 4, 2018

@MeTaNoV Maybe you guys can work together to update this PR?

Sure, as soon as you and @donmccurdy are finished with the last bit of technical detail.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 5, 2018

Other than working out an API to DeckGL, there are a couple other TODOs that would be good to cover in this example if possible:

  • Z-order. The DeckGL overlay currently entirely covers the Maps API zoom, fullscreen, and streetview controls. (solved below)
  • Picking. I've set pointer-events: none on the <canvas/> element so it doesn't interfere with map navigation, but that also prevents picking and event handling.

If I wait for the map to initialize and then move the DeckGL <canvas/> inside the maps api wrapper, and then style the maps API controls with something like .gmnoprint{ z-index: 999 !important; }, the controls appear on top of the layer. Not sure if that will have any side effects, or if there's a more elegant solution? One alternative would be attaching the DeckGL layer to one of the DOM elements returned by overlay.getPanes() (example) but those elements are managed/positioned in ways that we don't necessarily want, and it can be difficult to work around that.

For the second issue, I guess users could invoke DeckGL picking programmatically when Maps API detects a click event? That looks straightforward but I haven't tested it.

@Pessimistress
Copy link
Collaborator

Pessimistress commented Dec 5, 2018

perhaps we need a way to disable the default animation loop

There is an experimental prop _customRender to Deck. If you supply this prop, the animation loop will run and and update layers/highlight/transitions etc., but skip rendering to the WebGL context, and call _customRender instead (only if redraw is necessary).

In the Mapbox integration, we tell Mapbox to repaint:
https://github.com/uber/deck.gl/blob/master/modules/mapbox/src/deck-utils.js#L11

And Mapbox will tell all of its custom layers to draw, at which point we just call
https://github.com/uber/deck.gl/blob/master/modules/mapbox/src/deck-utils.js#L64

If there's a Maps API that you can use to force redraw, it has to call the draw callbacks for all overlays, and the same technique will apply.

Edit: one caveat of _customRender is that it also skips clearing the WebGLContext (in case you need to draw other things into it outside of deck). You will need to manually call gl.clear(gl.DEPTH_BUFFER_BIT | gl.COLOR_BUFFER_BIT); in this case.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 21, 2018

There is an experimental prop _customRender to Deck. If you supply this prop, the animation loop will run and and update layers/highlight/transitions etc., but skip rendering to the WebGL context, and call _customRender instead (only if redraw is necessary).

I'm not sure I understand how to apply this. If I do...

deck.setProps({_customRender: () => console.log('_customRender()')});

... it doesn't seem like that logging method is being called? Assuming I've just done something wrong and it works fine, how would it affect this use with Maps API? Would we use an empty _customRender function to ensure DeckGL doesn't render except during the Maps API draw callbacks?

Unfortunately there is no method to tell Maps API when to render. 😕


I've resolved the z-order issue by adding the DeckGL canvas to the OverlayView's panes (which are moved in CSS as the map pans) and repositioning the canvas each frame. That works fine unless you zoom out far enough that the map wraps multiple times in the viewport, which seems acceptable for now. As a result (1) there is no need to disable pointer events, and (2) the deckgl overlay is no longer on top of the zoom/satellite/fullscreen UI buttons.

@Pessimistress
Copy link
Collaborator

You need deck.gl@^6.3.0 to use _customRender.

When it works, you'd want to force Google Map to redraw (maybe like this?), which will in turn invoke the draw call for each overlay. Then in overlay.draw:

...
deck.setProps({viewState: { zoom, latitude, longitude }});
// These two lines can turn into a public API `deck.draw()`
deck._drawLayers();
deck.needsRedraw({clearRedrawFlags: true});

@donmccurdy
Copy link
Collaborator

donmccurdy commented Jan 15, 2019

@Pessimistress is this what you're suggesting?

  1. DeckGL _customRender callback triggers resize on Maps API, forcing redraw
  2. Maps API draw() callback sets view state on DeckGL
  3. Maps API draw() calls _drawLayers() (or new public API) on DeckGL

In that scenario, we're using two private APIs of DeckGL (_customRender and _drawLayers) or creating public APIs for them, and also relying on resize events to trigger drawing in Maps API... that seems more fragile than letting Maps API run the animation loop, eliminating (1) above. How does _customRender help in this case? If Maps API is going to set view state on DeckGL, it seems like Maps API should manage the animation cycle, but maybe I've missed something.

@donmccurdy
Copy link
Collaborator

API details aside, we've tested the last open question I know of — picking. Picking appears to be working just fine. :)

@Pessimistress
Copy link
Collaborator

@donmccurdy I gave this some more thought - it doesn't make sense for your Google Maps integration to use _customRender because deck is not sharing the same GL context with the base map (which is the case for Mapbox integration). We don't need to redraw the base map in order to refresh the deck layers.

On your concern about Deck's own animation loop - it is still necessary for auto highlight and transitions to work. In order to eliminate the double-draw, I propose we add a public API deck.redraw() which synchronously updates the canvas and clears the dirty flags.

@donmccurdy
Copy link
Collaborator

Ok that makes sense, thank you. 👍

Would the proposed deck.redraw() method do a proper version of what my hack (below) is currently doing then?

deck.animationLoop._setupFrame();
deck.animationLoop._updateCallbackData(); // call callback
deck.animationLoop.onRender(deck.animationLoop.animationProps); // end callback

More specifically – it will always redraw synchronously, and allows the next iteration of Deck's animation loop to be skipped if state hasn't changed?

@Pessimistress
Copy link
Collaborator

Yes that's the intention.

@donmccurdy
Copy link
Collaborator

Sounds great to me. I can try a PR if that would be helpful, although I don't really know what's required other than the code above, and I suspect there's more to it. 😇

@donmccurdy
Copy link
Collaborator

donmccurdy commented Jan 31, 2019

Spent a while trying to get local demo -> local deck.gl -> local luma.gl working and couldn't get the npm link incantations right, so I'll post here instead lol. I'm assuming the steps should be:

  1. add a redraw() method on luma.gl's AnimationLoop object:
  // Renders a frame outside the render loop and clears dirty flags
  redraw() {
    this._setupFrame();
    this._updateCallbackData();
    this.onRender(this.animationProps);
    this._clearNeedsRedraw();
  }
  1. add a redraw() method on DeckGL's Deck object:
  redraw() {
    this.animationLoop.redraw();
  }
  1. documentation

Does this seem right?

@donmccurdy
Copy link
Collaborator

/cc visgl/luma.gl#898

@Pessimistress
Copy link
Collaborator

@donmccurdy the synchronous redraw API is published in luma.gl@6.4.1. You should be able to call deck.animationLoop.redraw() in your code. (We will add a deck.redraw() method which does just that.)

@Pessimistress
Copy link
Collaborator

Closing this now that we're rolling this solution into an official @deck.gl/google-maps module. Thank you @MeTaNoV and @donmccurdy for the prototyping efforts and great discussions.

For anyone who's curious how this will work, check out https://github.com/uber/deck.gl/tree/master/examples/get-started/pure-js/google-maps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants