-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
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.
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: { |
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.
I assume we can drop this resolve clause.
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.
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> |
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.
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;
});
};
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.
A much more flexible way, thanks for the hint!
- load Google Maps JS API programmatically
@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. |
@MeTaNoV I am fine with landing this now. However until we have fixed the "tearing" let's put it in I'll take a look if I can find the time to understand how to get a working Google Map API key. |
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? If so, it may be worth looking at Google Maps "new renderer" documentation:
It appears that 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 |
@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 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 |
Thanks @Pessimistress — I think a core issue here is that Maps API does not support fractional zoom via 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: The option of managing the DeckGL animation loop manually, similar to three.js' 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. |
@donmccurdy You can try manually trigger redraw using |
That's Updated demo and source. Initial reactions:
I'm not sure what to make of (2). It's possible that |
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 |
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 Unfortunately |
I stepped through your code - it seems that when your |
I can't reproduce that, and do see floating-point Produces non-integers during a zoom transition: 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. |
@donmccurdy That was very helpful. I dig into // 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. |
@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... |
@MeTaNoV Oh, hm. I guess it is just the default renderer now.
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. 😅
Great insight, thank you! We need to use |
@donmccurdy That looks like it's working! @MeTaNoV Maybe you guys can work together to update this PR? |
@donmccurdy 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. |
Ok, in that case, I think it's best if the Deck class offers a public API to do this, something like |
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 |
Sure, as soon as you and @donmccurdy are finished with the last bit of technical detail. |
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:
If I wait for the map to initialize and then move the DeckGL 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. |
There is an experimental prop In the Mapbox integration, we tell Mapbox to repaint: And Mapbox will tell all of its custom layers to draw, at which point we just call If there's a Maps API that you can use to force redraw, it has to call the Edit: one caveat of |
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 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. |
You need 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 ...
deck.setProps({viewState: { zoom, latitude, longitude }});
// These two lines can turn into a public API `deck.draw()`
deck._drawLayers();
deck.needsRedraw({clearRedrawFlags: true}); |
@Pessimistress is this what you're suggesting?
In that scenario, we're using two private APIs of DeckGL ( |
API details aside, we've tested the last open question I know of — picking. Picking appears to be working just fine. :) |
@donmccurdy I gave this some more thought - it doesn't make sense for your Google Maps integration to use 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 |
Ok that makes sense, thank you. 👍 Would the proposed 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? |
Yes that's the intention. |
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. 😇 |
Spent a while trying to get
// Renders a frame outside the render loop and clears dirty flags
redraw() {
this._setupFrame();
this._updateCallbackData();
this.onRender(this.animationProps);
this._clearNeedsRedraw();
}
redraw() {
this.animationLoop.redraw();
}
Does this seem right? |
@donmccurdy the synchronous redraw API is published in [email protected]. You should be able to call |
Closing this now that we're rolling this solution into an official 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 |
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.