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

Make it so things don't flash so much when you change the tile source URL #5364

Merged
merged 10 commits into from May 19, 2016
Merged

Make it so things don't flash so much when you change the tile source URL #5364

merged 10 commits into from May 19, 2016

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented May 19, 2016

This generalizes the work in #4389 (originally #3660) and #4527 so that all tile sources can enjoy flash-free rendering when things change. Instead of a WMTS or WMS specific property, all tile sources have a key that is assigned to tiles created by the source (as the tile.key property). The WMTS source uses its dimensions property to generate the key (if dimensions change, the key changes). The WMS source uses its params property to generate the key (if params change, the key changes). The URL sources (e.g. ol.source.XYZ) use the url property to generate the key.

As a result, when you call source.setUrl(), the currently rendered tiles are not cleared. Instead, new tiles are loaded and rendered in place of existing tiles after the load.

This means that source.setUrl() can (and should) be used for "smooth rendering" on changes (as was the original intent with #3660. In cases where people want to completely clear currently rendered tiles, source.refresh() can be called. Alternatively, layer.setSource() can be used to erase currently rendered tiles and render new tiles as they load.

I'm still interested to see if the perceived performance of interim tile loading/rendering can be improved (see #4455). But this is a separate issue. This change only makes it so all ol.source.Tile can use the same mechanism.

@tschaub tschaub changed the title Make it so things don't flash so much when you change the tile source URL (WIP) Make it so things don't flash so much when you change the tile source URL May 19, 2016
@tschaub tschaub changed the title (WIP) Make it so things don't flash so much when you change the tile source URL Make it so things don't flash so much when you change the tile source URL May 19, 2016
@bjornharrtell
Copy link
Contributor

This got me wondering if source.refresh() could/should not have the same (optional) feature if possible. I've used vector tiles recently and modify style of the layer dynamically. To redraw the vector tiles with the new style I use source.refresh() and in that case I think it would be nice to avoid the clear.

@bartvde
Copy link
Member

bartvde commented May 19, 2016

Nice to see this improvement @tschaub !

@bartvde
Copy link
Member

bartvde commented May 19, 2016

@tschaub should setUrl be used here as well then? https://github.com/openlayers/ol3/blob/master/examples/wms-time.js#L40

or am I taking your words too strictly "This means that source.setUrl() can (and should) be used for "smooth rendering" on changes" ?

I guess I'm misinterpreting that sentence ;-)

@ahocevar
Copy link
Member

ahocevar commented May 19, 2016

@bjornharrtell:

I've used vector tiles recently and modify style of the layer dynamically

When you call setStyle() on an ol.layer.VectorTile, it will redraw. I think (but haven't tried) that you could achieve the same when you call layer.changed(). In both cases, the source won't be cleared. Note that a style change does not mean the source has changed, it's only the layer. So the source should not be involved in changing a style.

@ahocevar
Copy link
Member

ahocevar commented May 19, 2016

Very nice generalisation @tschaub. To me, this looks great. Please merge.

@bartvde: For WMS sources, updateParams results in a url change, but is a nicer API because you do not have to build the URL from the params manually. So yes, I think you are taking @tschaub's words too strictly, because he meant the generic case of ol.source.Tile, where there is no params handling.

@bjornharrtell
Copy link
Contributor

Thank you @ahocevar for clarifying and sorry for the noise.

@tschaub
Copy link
Member Author

tschaub commented May 19, 2016

Thanks for the reviews. I've added tests and upgrade notes. I'll merge when green.

@bartvde - I meant that source.setUrl() should be used to achieve smooth transitions rather than some other method (we have a few applications that do a ton of extra work maintaining a "loader" source, registering tile load listeners, trying to guess when loading is done, and finally changing the URL of another source - extra code, unnecessary overhead, and sometimes buggy results). I agree with @ahocevar that source.updateParams() should be used for WMS sources to achieve the same.

@bartvde
Copy link
Member

bartvde commented May 19, 2016

cool thanks for the clarification and thanks for the PR @tschaub

@tschaub tschaub merged commit 50408d7 into openlayers:master May 19, 2016
@tschaub tschaub deleted the less-flashy branch May 19, 2016 15:26
@tschaub
Copy link
Member Author

tschaub commented May 19, 2016

In case the effect of this is not clear, here's the example in action:

less-flashy

And the old behavior:

more-flashy

(The second gif doesn't capture all the flashes you see even with a full cache.)

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

4 participants