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: https://github.com/tangrams/walkabout-style/pull/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 notifications@github.com 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 https://github.com/tangrams/walkabout-style/pull/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 https://github.com/tangrams/tangram-docs/issues/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 notifications@github.com 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 https://github.com/tangrams/tangram-docs/issues/198#issuecomment-286865056, 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).