Skip to content

Add support for text highlight boxes + underlines #761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
bcamper merged 13 commits into from
Jul 21, 2020
Merged

Add support for text highlight boxes + underlines #761

bcamper merged 13 commits into from
Jul 21, 2020

Conversation

@bcamper
Copy link
Member

@bcamper bcamper commented Jul 5, 2020

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

  • Text box fill color
    • font.background.color
    • Optionally vary alpha independently: font.background.alpha
  • Text box stroke color
    • font.background.stroke.color
    • Optionally vary alpha independently: font.background.stroke.alpha
  • Text box stroke width:
    • font.background.stroke.width
    • Values in pixels, as with font.stroke.width
      • e.g. font.background.stroke.width: 2px (explicit) or font.background.stroke.width: 2 (implicit)
    • Defaults to 1px when font.background.stroke.color is not null
  • Text background fill and stroke are both incorporated into the label's collision box.

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

  • For terminology, I was unsure of the following choices:
    • I used font.background.color instead of font.background.fill. Generally, color is more common than fill in Tangram, and the latter always felt like a bit of an outlier. That said, there's an argument for using it here for consistency with font.fill.
    • I used font.background.stroke, with font.background.stroke.{color|width}, which does follow font.stroke.{color|width}. An alternative would be outline, to match shape outlines for lines and points.
  • I included support for alpha overrides, to match recently added functionality for all other color properties. Seems like potential overkill, but it's consistent.
  • There's no limit on font.background.stroke.width. There's some implementation-specific limit, but for instance stroke: 64px does work:
    tangram-1593919222805

@bcamper
Copy link
Member Author

bcamper commented Jul 5, 2020

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

@bcamper bcamper modified the milestones: v0.23.0, v0.21.0 Jul 5, 2020
@matteblair
Copy link
Member

matteblair commented Jul 5, 2020

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?

@nvkelso
Copy link
Member

nvkelso commented Jul 7, 2020 via email

@bdon
Copy link
Contributor

bdon commented Jul 10, 2020

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.

@nvkelso
Copy link
Member

nvkelso commented Jul 10, 2020 via email

@bcamper
Copy link
Member Author

bcamper commented Jul 11, 2020

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 Add support for text highlight boxes #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

  • 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.

  • 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, Add support for underline text style #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
    Screen Shot 2020-07-11 at 4 22 33 PM Optionally combined w/background box: Screen Shot 2020-07-11 at 4 23 06 PM

  • @bdon I believe this should have been addressed by e0d3bde, 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! 🚀

@bcamper bcamper changed the title Add support for text highlight boxes Add support for text highlight boxes + underlines Jul 11, 2020
@matteblair
Copy link
Member

matteblair commented Jul 12, 2020

Thanks for the clarifications 👍

A few more questions:

  • 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?
  • How are the elements in these formations blended together when multiple colors have transparency?
  • 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...
  • Is there a way to add an underline that inherits the font color? Would underline with a width but no color do that?

@nvkelso
Copy link
Member

nvkelso commented Jul 12, 2020 via email

@bcamper
Copy link
Member Author

bcamper commented Jul 13, 2020

  • 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
Copy link
Member

matteblair commented Jul 13, 2020

  • Those stroke metrics make sense to me. I think for points strokes do the inside/outside thing so I wanted to make sure.
  • Compositing makes sense too. In Tangram ES we'll probably implement this as two separate primitives, but I think as long as it's all using the same blending technique it should be equivalent..
  • I totally misinterpreted background.width haha. padding sounds good to me.
  • I feel like underlines inheriting text color is worth supporting - or at least that its absence might be frustrating. Seems like underlines with a default width and color would cover the vast majority of use cases. Maybe we can just start with underline: true?  

@bdon
Copy link
Contributor

bdon commented Jul 15, 2020

@bdon I believe this should have been addressed by e0d3bde

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

@bcamper
Copy link
Member Author

bcamper commented Jul 17, 2020

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
Copy link
Member

nvkelso commented Jul 17, 2020

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
Copy link
Contributor

bdon commented Jul 18, 2020

+1 to fixed values for underline color and width

@bcamper
Copy link
Member Author

bcamper commented Jul 21, 2020

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

@bcamper bcamper merged commit 7af097d into master Jul 21, 2020
@bcamper bcamper deleted the text-box branch July 31, 2020 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0.21.0

Development

Successfully merging this pull request may close these issues.

5 participants

@bcamper @matteblair @nvkelso @bdon