tangrams / tangram

WebGL map rendering engine for creative cartography
https://tangram.city
MIT License
2.2k stars 290 forks source link

text:visible and outline:visible style parameters #327

Closed tallytalwar closed 8 years ago

tallytalwar commented 8 years ago

text:visible and outline:visible does not appear to work on tangram JS.

Based on the docs, outline parameters can have any parameters which a lines style have, similarly text under the points style can have any parameter which a texts style can have.

Refer tangram-es PR: https://github.com/tangrams/tangram-es/pull/791

We noticed this because outdoor-style started using text:visible parameter and we were getting a bunch of warning on ES.

bcamper commented 8 years ago

Clarification: text:visible does work for the standalone text style, but you are correct that it does not work for text nested under points, nor for outline (nested under lines).

I didn't necessarily intend for this to work (since visible is handled as generically either a draw group-level or style layer-level parameter), but I see that it follows from the way the docs are phrase, and is a reasonable commonsense interpretation.

So I'm open to adding that behavior, if there's consensus that it's desirable.

cc @blair1618 @nvkelso

tallytalwar commented 8 years ago

The reason we found out about this, was because it was in use in the outdoor-style. Example: https://github.com/tangrams/outdoor-style/blob/gh-pages/outdoor-style.yaml#L3245. But obviously had no affect, since its not implemented on either of the renderers.

nvkelso commented 8 years ago

You found me out! Yes, sometimes we might want to only show the icon part of the point label, but not it's icon. This junk style code is me trying to accomplish that.

bcamper commented 8 years ago
  • require icon & require text
    • @bcamper you said this is not currently supported?
    • if the icon can fit, but the text can't then whole thing should optionally fail
    • would require new optional property in point > text symbolizer.
    • Rational: There are a ton of orphan icons on the outdoor style today (and other house styles) because of this, usually related to the text portion colliding a tile boundary.

I'm looking at adding this in an upcoming release.

  • require icon & hide text
    • sounds like this is not currently supported, focus of this issue
    • icons: { visible: true, text: { visible: false } }
    • Rational: for some features like mountain peaks where the icon itself holds up well and the name is interesting but not essential, it's nice to sometimes hide the text for a few zooms.
    • Workaround? Maybe setting the text size to 0 for that zoom?

It shouldn't be difficult to make text nested in points respect the visible property (same for outline for consistency). @tallytalwar does this pose any issues for you to add?

  • no icon, require text
    • the only way I've found to do this so far is to disable the points block and reenable the old text symbolizer.
    • It's be better to optionally allow the icon of the point to be visible separate from the total combined symbolizer.

Ah, the intention was for the standalone text style to be used for that -- it wasn't meant to be old/deprecated so much as continue to be used for such cases where you are only rendering text (like road labels too). I'd have to think more about allowing the points portion to be "empty", because there are already several assumptions that the "parent" part of that relationship is renderable.

tallytalwar commented 8 years ago

@bcamper Already have the work done for ES. https://github.com/tangrams/tangram-es/pull/791. Was waiting on a decision to be taken for its official support :D.

bcamper commented 8 years ago

visible support for points > text and lines > outline is added in https://github.com/tangrams/tangram/commit/053451d76d34c8f9a08d78654e0883fbb25fa815 & https://github.com/tangrams/tangram/commit/345044906d7f4f007b410ee8a1036721c8561f8a, and will be in the next release, 0.8.2.

bcamper commented 8 years ago

Released in v0.8.2.