Avoid loading style source if its layer(s) are not shown

[brunoabinader]

Given https://github.com/mapbox/mapbox-gl-styles/blob/master/styles/satellite-hybrid-v8.json as example of a style containing 2+ sources, if all layers related to a specific source are either disabled (e.g. due to class selections) or invisible, we should avoid loading the style source entirely. This saves processing time and network bandwidth.

When the layers related to a specific style source are re-enabled, the burden of loading the style source could be reduced by using a cache file source.

/cc @mapbox/gl @mapbox/mobile

[jfirebaugh]

Rename this to isVisible and reverse the boolean, for consistency with "visibility" as from the style spec.

[jfirebaugh]

Instead of introducing more state, can we use Source::enabled for this?

[brunoabinader]

Instead of introducing more state, can we use Source::enabled for this?

Indeed, enabling the source only if the layer is visible in Style::recalculate() is nicer than comparing strings in Style::loadSources(). However, we still require Source::isLoading() to avoid double requests and StyleLayer::isVisible() because each StyleLayer-derived class calculates its visibility in a different way. I'll update the code accordingly.

[jfirebaugh]

There are some corner cases where a layer with a zero-opacity color still renders something, e.g. symbols with halo but transparent fill, background-pattern, outline-only fill layers. Can we limit the check to opacity > 0? Also we should add checks of the *-visibility layout property, which is the most common way to hide a layer completely.

[jfirebaugh]

Can we make layer.isVisible() equivalent to layer.passes != RenderPass::None?

[jfirebaugh]

Should be a one liner: return !loaded && req.operator bool();

[brunoabinader]

Can we limit the check to opacity > 0?
Also we should add checks of the *-visibility layout property, which is the most common way to hide a layer completely.

This is used in Style::recalculate() now:

bool StyleLayer::needsRendering() const {
    return passes != RenderPass::None && visibility != VisibilityType::None;
}

I had to fix/modify some style unit tests to adapt to the new scheme for source loading - instead of immediately load sources upon style JSON parsing, we wait to check whether a layer actually needs it.

[jfirebaugh]

Is this change necessary? I would expect Source:isLoaded() to return true in the case that at some zN, it was enabled and loaded, and then at zM the only layer that used it becomes hidden and the source was therefore disabled.

[brunoabinader]

Agreed about enabled, but I believe we also should check if there is a pending request, which indicates the source is in loading state.

[jfirebaugh]

Doesn't req imply !loaded though? loaded only gets set to true after all the requests are successful.

[brunoabinader]

Yes 🎯 Good catch, indeed!