Skip to content

Conversation

@batpad
Copy link
Member

@batpad batpad commented Mar 30, 2024

Created a new PR toward #112, essentially copying over the code from #113, but applying it against main.

@kylebarron this seems to work!! It does seem to make the map a bit slow for me, and we should investigate that, but in principle, this for me works as expected.

On the javascript, I can move and zoom the map around and then in python call:

m._view_state

Where m is a Map instance, and I get the current viewState as it is on the map.

Conversely, to modify the view state on the map, in Python I can do something like:

new_state = m._view_state.copy()
new_state['zoom'] = 8
m._view_state = new_state

The above code works to set the zoom on the map to 8.

I think it will still be great to get eyes on this from React folks, and maybe think about if we could upstream the useModelStateDebounced method or so, but in principle, this seems to work!

src/index.tsx Outdated
Comment on lines 173 to 177
viewState={
Object.keys(viewState).length === 0
? DEFAULT_INITIAL_VIEW_STATE
: viewState
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd guess we don't want to check this on every render

@kylebarron
Copy link
Member

Closing in favor of #448, which is based off this pr

@kylebarron kylebarron closed this Apr 1, 2024
kylebarron added a commit that referenced this pull request Apr 2, 2024
Closes #112



In #444 @batpad
mentioned

> It does seem to make the map a bit slow for me, and we should
investigate that, but in principle, this for me works as expected

While playing around with that PR, I also saw some view state lagging at
low zooms with lots of data rendered. In this screencast I'm smoothly
zooming in, but the rendering is very jagged, and sometimes zooms back
out slightly. **This screencast is of the previous PR**


https://github.com/developmentseed/lonboard/assets/15164633/bbe0a168-b76e-4563-bc30-828d648210db

My hypothesis of what's happening is that when rendering lots of data
there's a slight lag for deck to call the `onViewStateChange` handler.
And so deck will occasionally call the handler with "old" data and
that's where you sometimes get the "snap back" in the scrolling.

I think this is a really bad UX, and it seems not to happen when deck
manages the view state internally. So I think potentially the best UX
and performance solution is to decouple the view state from the model,
while still syncing from deck to Python in the debounced model handler.

This PR tries to implement this.

---------

Co-authored-by: Sanjay Bhangar <sanjay@developmentseed.org>
roger120981 pushed a commit to roger120981/lonboard that referenced this pull request Jul 14, 2025
Closes developmentseed#112



In developmentseed#444 @batpad
mentioned

> It does seem to make the map a bit slow for me, and we should
investigate that, but in principle, this for me works as expected

While playing around with that PR, I also saw some view state lagging at
low zooms with lots of data rendered. In this screencast I'm smoothly
zooming in, but the rendering is very jagged, and sometimes zooms back
out slightly. **This screencast is of the previous PR**


https://github.com/developmentseed/lonboard/assets/15164633/bbe0a168-b76e-4563-bc30-828d648210db

My hypothesis of what's happening is that when rendering lots of data
there's a slight lag for deck to call the `onViewStateChange` handler.
And so deck will occasionally call the handler with "old" data and
that's where you sometimes get the "snap back" in the scrolling.

I think this is a really bad UX, and it seems not to happen when deck
manages the view state internally. So I think potentially the best UX
and performance solution is to decouple the view state from the model,
while still syncing from deck to Python in the debounced model handler.

This PR tries to implement this.

---------

Co-authored-by: Sanjay Bhangar <sanjay@developmentseed.org>
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.

3 participants