search

tangrams / tangram

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

Add support for text highlight boxes + underlines #761

Closed bcamper closed 4 years ago

bcamper commented 4 years ago

See #724. Adds support for an optional box behind text labels, which adds the following parameters, in a new font.background block:

Examples

Fill only:

draw:
  text:
    ...
    font:
      ...
      background:
        color: red

tangram-1593906832593

Fill and stroke:

draw:
  text:
    ...
    font:
      ...
      background:
        color: red
        stroke:
          color: black
          width: 2px

tangram-1593906900223

Stroke only:

draw:
  text:
    ...
    font:
      ...
      background:
        stroke:
          color: black

tangram-1593907009480

All parameters can accept JS functions (alpha and stroke width shown here):

draw:
  text:
    ...
    font:
      ...
      background:
        color: red
        alpha: function() { return Math.min(Math.max(feature.kind_tile_rank - 4, 0), 20) / 8 + .35 }
        stroke:
          color: black
          width: function() { return Math.min(Math.max(feature.kind_tile_rank - 4, 1), 8) }

tangram-1593907059790

Open Questions

bcamper commented 4 years ago

cc @nvkelso @bdon @meetar @matteblair @tallytalwar thoughts on functionality, syntax, and naming appreciated!

matteblair commented 4 years ago

Nice! I'm excited to use this to make some sick Supreme-branded maps.

nvkelso commented 4 years ago

Most excellent!

On Sat, Jul 4, 2020 at 23:51 Matt Blair notifications@github.com wrote:

Nice! I'm excited to use this to make some sick Supreme-branded maps.

  • How are the dimensions of the background box determined from the text layout?
  • What happens for curved labels?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/tangrams/tangram/pull/761#issuecomment-653849883, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQIOY5BULQ2FXPLGYWE7LR2APFPANCNFSM4OQVSTGQ .

bdon commented 4 years ago

Looking good!

nvkelso commented 4 years ago

Padding of background box?

For curved text, I agree can be for later. Usually there is option to stop the box behind whole string, words, and characters when a threshold of letter space or word space is reached.

On Thu, Jul 9, 2020 at 20:44 Brandon Liu notifications@github.com wrote:

Looking good!

  • IMHO curved labels can be excluded from this
  • I'm getting a WebGL: INVALID_VALUE: texImage2D: no canvas when there is no stroke key defined in background
  • I think there should be a way to control the padding of the background box.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/tangrams/tangram/pull/761#issuecomment-656465207, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQIO5UFTR2ARELGOU5L4TR22FBDANCNFSM4OQVSTGQ .

bcamper commented 4 years ago

OK, getting back to this again and trying to wrap up...

As always, we can make further enhancements or adjustments in the future, but given the scope of this PR, I'd love to wrap it up and proceed. There looks to be general consensus. If you have any further comments on default background box sizing, text underlines, or anything else, please speak up now and then we can get this into Tangram v0.21.0! 🚀

matteblair commented 4 years ago

Thanks for the clarifications 👍

A few more questions:

nvkelso commented 4 years ago

Fantastic!

On Sat, Jul 11, 2020 at 16:28 Brett Camper notifications@github.com wrote:

OK, getting back to this again and trying to wrap up...

-

Which labels apply: Sorry if this was lost here, but based on discussion in original issue #724 https://github.com/tangrams/tangram/issues/724, the approach here was to limit to point labels only, e.g. exclude both curved and angled straight line labels. Curved labels present the implementation difficulty here, so angled straight labels (e.g. usually straight road labels) were excluded largely to match the omission of curved labels, since they are usually labelling the same set of features, and it seemed it would be weird to have it on some but not others. This could be revisited in the future, but I wanted to manage this initial implementation complexity, and believe point labels are the primary use case. cc @matteblair https://github.com/matteblair

Background box size: The background box dimensions are currently calculated as 4px on each side of the text, but in practice as you can see from screenshots, it creates a bit more padding vertically than horizontally, because font heights extend above. This size was mostly arbitrary, we can adjust the default if people have other suggestions.

Background box size configurability: I was initially going to suggest we defer making the background box dimension configurable, because there's already a lot going on here and I don't want to re-create the CSS box model ;) However, it was only a few lines of code now that everything else is setup, so... here you go, e.g. font.background.width = 8px 6cf2090 https://github.com/tangrams/tangram/commit/6cf20902e7829bb5a025e7148ceb82507ebf95f6 .

Text underlines: I know I said I wanted to manage complexity... I do, but I was already in process of adding the companion feature to this,

723 https://github.com/tangrams/tangram/issues/723. I've added that

here now, with similar syntax. As with the text box, a lot these calculations for positioning and spacing have just been manually tweaked here until they more or less look right under a variety of conditions, because canvas text font metrics are not complex and/or widely available enough to support all these cases. Example:

draw: text: font: underline: color: blue

alpha: 0.5 # optional alpha override

       width: 2px # defaults to 1px if only `color` is defined

[image: Screen Shot 2020-07-11 at 4 22 33 PM] https://urldefense.proofpoint.com/v2/url?u=https-3A__user-2Dimages.githubusercontent.com_16733_87235623-2Df8d8c080-2Dc3ab-2D11ea-2D8ed4-2Da6c3819b22d2.png&d=DwMFaQ&c=ncDTmphkJTvjIDPh0hpF_w&r=d__e7RyVjCBMVyq0q3TP4Q&m=GVe_5g2_6cAWRqk8ybBTvieqxPXpn5saMwFe02kwLKU&s=aO4z-juOf1NPJeko6KbjNsJ1l_Xdx2eQnn7lKLcHxK8&e= Optionally combined w/background box: [image: Screen Shot 2020-07-11 at 4 23 06 PM] https://urldefense.proofpoint.com/v2/url?u=https-3A__user-2Dimages.githubusercontent.com_16733_87235629-2D03935580-2Dc3ac-2D11ea-2D816a-2D13fcb2431792.png&d=DwMFaQ&c=ncDTmphkJTvjIDPh0hpF_w&r=d__e7RyVjCBMVyq0q3TP4Q&m=GVe_5g2_6cAWRqk8ybBTvieqxPXpn5saMwFe02kwLKU&s=kALre10McwPTYQnNQKRAayXAkNhMV0Fbv0Xiinm2ecI&e=

@bdon https://github.com/bdon I believe this should have been addressed by e0d3bde https://github.com/tangrams/tangram/commit/e0d3bdeb3f47f931a2c19732f789f4ef9e82eff1, if you're still seeing on this branch, could you please send me a scene YAML reduction example I can work off of.

I'm getting a WebGL: INVALID_VALUE: texImage2D: no canvas when there is no stroke key defined in background

As always, we can make further enhancements or adjustments in the future, but given the scope of this PR, I'd love to wrap it up and proceed. There looks to be general consensus. If you have any further comments on default background box sizing, text underlines, or anything else, please speak up now and then we can get this into Tangram v0.21.0! 🚀

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/tangrams/tangram/pull/761#issuecomment-657146343, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQIO6IOYXKEWHZQYIRFE3R3DYQXANCNFSM4OQVSTGQ .

bcamper commented 4 years ago
  • The stroke on the background boxes: Does this follow the SVG model of "stroke" where the line extends both into and out of the shape? If so, is the specified width just the exterior extent or the full width?

In this case, the full stroke is drawn "outside of"/beyond the background box edge, so the stroke width does not affect the visible portion of the background box fill, and the visible portion of the stroke around the background box should be the full width. So the full width of the box should be:

stroke_width + background_width + text_width + background_width + stroke_width = text_width + 2 * (stroke_width + background_width)

Where text_width is implementation-specific between Tangram JS/ES, with some small variations due to different text rendering technologies. (And same applies for height as text_height + 2 * (stroke_width + background_width).)

  • How are the elements in these formations blended together when multiple colors have transparency?

Since for Tangram JS these elements are all drawn as part of the label texture, they are composited using Canvas default source-over blending with "simple alpha compositing", which if I'm reading right looks equivalent to glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA). That texture is then composited with the Tangram scene, with the same blending when the default Tangram overlay blend mode is used.

  • So font.background.width and font.background.height specify the padding that's added to both sides of each axis? Should we call it padding_width? Maybe padding by itself to set both? Hm. The possibilities start to get out of hand pretty quickly...

Only font.background.width is currently defined (no font.background.height), because it was intended as stroke width of the line being drawn -- so it applies to both the sides and top/bottom of the box (see notes on box dimensions above). However, this misunderstanding confirms my discomfort with width in this context.

I like padding instead, it's a new term for Tangram but fits the CSS definition. I also don't think we need support for specifying different padding for width vs. height at this point, so I would suggest we stick with a single scalar for now, e.g. padding: 8px. If we wanted to expand this to a 2D value in the future, we would simply start accepting array values as we do for similar props like size, offset, etc.

Similarly, I haven't loved the use of stroke here overall. outline would be another Tangram term that fits, e.g. from line.outline. Or, if we use padding, the CSS equivalent for this would be border. I don't have strong feelings on this though.

  • Is there a way to add an underline that inherits the font color? Would underline with a width but no color do that?

There is not but that makes sense, I think that's a better default, to have underline.color inherit from font.fill.

In this case, would there then not be a default for underline.width (currently 1px)? Or would we continue to have a default width but require the underline block to specify either width or color? Otherwise, you get an empty underline: {} object drawing an underline, and that feels a little too implicit to be intuitive. Maybe I'm overcomplicating things though? I don't have strong opinions on this one either.

matteblair commented 4 years ago
bdon commented 4 years ago

@bdon I believe this should have been addressed by e0d3bde

Yes, the background boxes are fixed now for my scene.yaml.

bcamper commented 4 years ago

Makes sense to me to use underline: true to indicate defaults for inherited font color and default stroke width (1px? may be a bit too narrow), and follows similar Tangram conventions. I had been thinking of it as a shortcut, rather than removing the option to set the color and width properties.

@nvkelso @bdon see above, @matteblair is proposing we start with fixed values for text underline color and width, to manage complexity (especially as they are not 100% sure yet how they would implement this in Tangram ES). Does this make sense as an initial limitation to you? @matteblair pointed out that even most text editing software doesn't offer that level of customization. The code is already written on the Tangram JS side, so not a big difference for me either way :)

nvkelso commented 4 years ago

I like underline: true with some sensible default for the line weight (1, 1.5, 2?) and inheriting the line color from the text's color. I think that boolean matches what we might see for bold or italic.

bdon commented 4 years ago

+1 to fixed values for underline color and width

bcamper commented 4 years ago

Sounds good! I set the underline width at 1.5px which I think gives good results across device pixel ratios. We can adjust if needed.

Screen Shot 2020-07-20 at 9 10 59 PM