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

map.setLayoutProperty('layerID', 'visibility', 'none') will trigger duplicate http request #7459

Open
yangdonglai opened this issue Oct 22, 2018 · 6 comments
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@yangdonglai
Copy link
Contributor

mapbox-gl-js version: 0.49.0

Question

simple hidden a raster layer map.setLayoutProperty('layerID', 'visibility', 'none') will duplicate http request`

Is it a special feature or something need improved?

step

  1. add a raster layer with google map source.and clear network console
    image

  2. hidden raster layer by map.setLayoutProperty('layerID', 'visibility', 'none'),and you will find it trigger duplicate http
    image

@yangdonglai
Copy link
Contributor Author

Is it a special feature or something need improved?

I think visibility should not trigger source reload. Could somebody identify which ones layout property should trigger sourceCache.reload like paint property.
image

@ChrisLoer ChrisLoer added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Oct 22, 2018
@ChrisLoer
Copy link
Contributor

visibility is a layout property and source reloads are generally necessary for layout property changes. However, the case you describe is a little bit special because by setting the visibility to none, you're removing the last layer that uses the source. The code in Style#update triggers a reload of the source first, and then after triggering the reload it does the calculation to determine which sources are used, at which point it clears out the tile set for the hidden source. Although the tile set is cleared out almost immediately after the reload is triggered, the first messages get sent to the workers and they trigger the network activity you see (although it should be pretty cheap since the tiles are already loaded and they haven't expired).

update(parameters: EvaluationParameters) {
if (!this._loaded) {
return;
}
const changed = this._changed;
if (this._changed) {
const updatedIds = Object.keys(this._updatedLayers);
const removedIds = Object.keys(this._removedLayers);
if (updatedIds.length || removedIds.length) {
this._updateWorkerLayers(updatedIds, removedIds);
}
for (const id in this._updatedSources) {
const action = this._updatedSources[id];
assert(action === 'reload' || action === 'clear');
if (action === 'reload') {
this._reloadSource(id);
} else if (action === 'clear') {
this._clearSource(id);
}
}
for (const id in this._updatedPaintProps) {
this._layers[id].updateTransitions(parameters);
}
this.light.updateTransitions(parameters);
this._resetUpdates();
}
for (const sourceId in this.sourceCaches) {
this.sourceCaches[sourceId].used = false;
}
for (const layerId of this._order) {
const layer = this._layers[layerId];
layer.recalculate(parameters);
if (!layer.isHidden(parameters.zoom) && layer.source) {
this.sourceCaches[layer.source].used = true;
}
}
this.light.recalculate(parameters);
this.z = parameters.zoom;
if (changed) {
this.fire(new Event('data', {dataType: 'style'}));
}
}

@Niumeng1
Copy link

I found that the branch of 0.20.0 version of the same code, the request is not made, the new code on request

@mourner
Copy link
Member

mourner commented Mar 21, 2019

This was likely fixed by #7459. I'll reopen if it's not the case.

@igor-dv
Copy link

igor-dv commented Jul 11, 2019

Hey, we have the same issue with the latest version (1.1.0), I see it was mentioned this was fixed by #7459, but #7459 is actually this issue...

@mourner
Copy link
Member

mourner commented Jul 11, 2019

Oh, I meant #8005. Let's reopen and check again then.

@mourner mourner reopened this Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

5 participants