Interaction of layer 'visible' flag with sublayers is unspecified

[matteblair]

https://github.com/tangrams/tangram-docs/blob/gh-pages/pages/layers.md

Example:

layers:
  parent-layer:
    visible: false
    child-layer:
      visible: true
      draw:
        polygons:
          order: 1
          color: blue

Is child-layer drawn? The current documentation is ambiguous on this, and we've recently discovered that tangram-js and tangram-es resolve this differently: tangrams/walkabout-style#154

cc @bcamper @nvkelso @meetar

[meetar]

I remember having this conversation – is `visible` a master switch or an override toggle? I think we settled on "override" because everything else works that way, though it means there's no easy way to turn an entire tree invisible at once.

…

On Wed, Mar 15, 2017 at 2:07 PM, Matt Blair ***@***.***> wrote: https://github.com/tangrams/tangram-docs/blob/gh-pages/pages/layers.md Example: layers: parent-layer: visible: false child-layer: visible: true draw: polygons: order: 1 color: blue Is child-layer drawn? The current documentation is ambiguous on this, and we've recently discovered that tangram-js and tangram-es resolve this differently: tangrams/walkabout-style#154 <tangrams/walkabout-style#154> cc @bcamper <https://github.com/bcamper> @nvkelso <https://github.com/nvkelso> @meetar <https://github.com/meetar> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#198>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAcEwtJ7Oq9ddB4FjcQkmqSWwTQHWPztks5rmCjMgaJpZM4MeUs_> .

[matteblair]

I wouldn't be surprised if this conversation has happened before :)

As I see it, the layer visible flag would be more useful as a "master switch" precisely because it behaves differently from draw rules (which are merged bottom-up). As you said, without it there is no way to confidently disable an entire branch of the layer tree. Also, it isn't really the case that "everything else" uses the bottom-up-merge behavior, it's only draw rules that do that. Filters, for example, are a layer property that act more-or-less like a "master switch" the way you're describing.

[nvkelso]

There are cases when both would be useful in the basemap styles ;) Something to just turn a whole layer and it's children off (enabled: true?) versus the useful visibility cascading – which is necessary for overlays and label variants.

[matteblair]

The "cascading" visibility is already available in a way with the visible draw parameter (which is distinct from the visible layer flag). It isn't exactly the same, but it's close enough that it seems almost redundant to have the layer visible flag behave the same way.

[matteblair]

(I am admittedly prejudiced towards the "master switch" behavior because that's how it is already implemented in tangram-es).

[meetar]

…You know what, I had totally conflated those two `visible` parameters. Given that I now agree that it makes more sense that the filter-level `visible` be a master switch – and maybe renamed? Do we have styles which rely on it there? And yes, `enabled` would be less confusing.

…

On Wed, Mar 15, 2017 at 4:15 PM, Matt Blair ***@***.***> wrote: (I am admittedly prejudiced towards the "master switch" behavior because that's how it is already implemented in tangram-es). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#198 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAcEwo4M0FHmAl6jLqBZEhrma4B6eAgUks5rmEbXgaJpZM4MeUs_> .

[tallytalwar]

+1 on enabled for layer property, defaulting to true.

[matteblair]

I think there's evidence now that the two parameters are easily confused, a different name would probably go a long way to prevent that.

I suspect that the layer visible flag is not frequently used in scenes, since it has had different behaviors in JS and ES and we didn't notice until now.

In the 250 usages of visible: in bubble-wrap, I found exactly one instance where it was used as a layer flag (and was redundant).

[bcamper]

I've come around to the idea of a "master layer" switch both because it provides a function we don't have now (a simple way to cull whole sections of the layer tree), and also because it can provide possibly significant run-time savings by allowing the engine to stop processing layers at that point (previously JS has had to keep descending into all sub-layers in case one of them sets visible back to true).

I get the argument for changing the property name to enabled -- would we make it backwards compatible with visible for at least one release cycle though? (Even if the behavior changes a bit.)

I would not like to have both behaviors (overridable and non-overridable) at the layer level (too much complexity); as pointed out, we still have the ability to override the visible parameter within draw groups for achieving this.

That said, I notice that the demo bundled with Tangram JS makes a lot of use of layer visible, and the pattern of turning it on only at the sub-layer level. Are these rendering differently in ES, but we haven't noticed yet? For example: https://github.com/tangrams/tangram/blob/master/demos/scene.yaml#L627-L651

[matteblair]

I agree that if we choose a new name for this parameter then we should maintain backward compatibility with visible for some amount of time.

It seems that the current demo scene bundled with Tangram JS does indeed render differently in Tangram ES (we use default scene that is similar but not identical, so we wouldn't have noticed before now).