fix text-halo-width units

[ansis]

The units for text-halo-width are completely arbitrary and unclear. What should they be, pixels?

[kkaefer]

The values depend on how the signed distance field is generated. Currently, we're using 24px fonts to generate the glyphs and compute the distance field. In the generation code, we store values from -2 pixels to +6 pixels (256 / 32 = 8). So currently, a value of 0 corresponds to a halo width of 6px on a 24px font, and a value of 0.5 corresponds to a halo width of 2px on a 24px font. Since we shift all values by 64 (= 2px on a 24px font), a halo width of 0 has the value 0.75.

We should allow users to specify halo-width in pixels and do the math in the renderer using (6 - halo-width / (font-size / 24)) / 8, and clamp to values between 0 and 1. This means that the maximum halo width depends on the font size; it's 6px for fonts rendering at 24px, 4px for fonts rendering at 16px and 3px for fonts rendering at 12px.

[ansis]

@nickidlugash would it be more useful to have halo-width be in pixels or em?

[nickidlugash]

@ansis what is the unit for text-halo-blur? I think it would be useful for the units of these two properties to match. Pixels might be more useful for both, since I think it's preferable to adjust the halo size independent of the text size. @ajashton do you have any preference?

[lbud]

Per current v3 style spec:

    "text-halo-blur": {
      "type": "number",
      "default": 1,
      "function": true,
      "transition": true,
      "doc": "Fade out the halo towards the outside. 1 means no fade out. Higher values mean a higher fade out."

I'm not sure how text-halo-blur actually renders -- it seems like it should render relative to text-halo-width? In which case maybe it wouldn't make sense for them both to be in pixels? But it does seem like maybe a weird and arbitrary scale right now...?

[lbud]

So if text-halo-width is specified in pixels and based on text-size, what about cases like this (from outdoors-v3.json):

    "style": {
      "text-color": "#333",
      "text-halo-width": 0.4,
      "text-halo-color": "rgba(244,239,225,0.8)",
      "text-size": {
        "fn": "stops",
        "stops": [[3.99, 0], [4, 10], [9.99, 16], [10, 0]]
      }

If text-halo-width was pixel-based, you'd have to specify different pixel widths per stop in a stop function, which seems kind of crazy. ...?

[lbud]

That almost makes me want to say maybe instead we should just do something to translate the scale to be more intuitive, like where 0 is min (no) halo and 1 is max halo, so what's currently .75 would be specified by "text-halo-width": 0, and what's currently 0 would be specified by "text-halo-width": 1. But then @jfirebaugh brought up the fact that if we ever wanted to change this arbitrary max halo size to something greater than 6px (out of 24), we'd be supporting values > 1, which makes it once again a weird arbitrary scale...

[ansis]

I think pixels should work fine. Migrating them is tricky because it's a switch from units relative to the font size to pixels. I'd suggest migrating in an easier but inexact way, based on layer.render[text-max-size]. Inverting kkaefer's equation gets us:

newHaloWidth = (6 - 8 * oldHaloWidth) * (fontSize / 24);

[lbud]

Right, right. Hm...I've got migration code working for functions that writes functions inside of text-halo-width based on size functions. But another issue is cases like

      "text-color": "#585042",
      "text-halo-color": "@land",
      "text-halo-width": 0.6,
      "text-size": "@road_label_1_size"

-- should text-halo-width read road_label_1_size and calculate there? Or find that code and create a road_label_1_halo_width variable? Except that's a bad idea because there's a place in outdoors-v3.json that goes

    "style": {
      "text-color": "#585042",
      "text-halo-color": "@land",
      "text-halo-width": 0.6,
      "text-size": "@road_label_1_size"
    },
    "style.night": {
      "text-color": "@text_night",
      "text-halo-color": "@text2_stroke_night",
      "text-halo-width": 0.5,
      "text-size": "@road_label_1_size"
    }

o_O?

[ansis]

Hmm, reading the constant and adding it directly on the layer might be better than creating a new constant that is used once

[lbud]

I'm just reading migrated code and want to do a gut check here. This is what a migrated text-halo-width function looks like and I'm just worried this seems like maybe a major pain to write:

    "style": {
      "text-color": "#635644",
      "text-halo-width": {
        "fn": "stops",
        "stops": [[9, 1.2], [12, 1.5], [14, 2.1], [16, 2.4], [17, 3]]
      },
      "text-halo-color": "@text_stroke",
      "text-size": {
        "fn": "stops",
        "stops": [[9, 8], [12, 10], [14, 14], [16, 16], [17, 20]]
      }

(The same code before was

    "style": {
      "text-color": "#635644",
      "text-halo-width": 0.3,
      "text-halo-color": "@text_stroke",
      "text-size": {
        "fn": "stops",
        "stops": [[9, 8], [12, 10], [14, 14], [16, 16], [17, 20]]
      }

[nickidlugash]

@lbud IMO the reason for using px units for text-halo-width instead of a unit tied to the text-size is that you often wouldn't want your halo to increase proportionally to your text size, i.e. you wouldn't often want to write a style that looks like the migrated styles anyway. There's different strategies for using halos, but since it's often not a design feature per se (but rather a stylistic device to help make the text more legible over different backgrounds), often I want to make the halo as small as possible while still achieving this function. In this scenario, I wouldn't expect to write nearly as many stops as a function that is proportional to text size.

[kkaefer]

Thanks @nickidlugash, that's very helpful input. Based on that, the proposed design of making the halo width a pixel unit that is independent of the font size seems okay.

[lbud]

Awesome, thanks @nickidlugash -- very helpful.

Next question: in a case like

{
    "id": "waterway_label",
    "source": "mapbox.mapbox-terrain-v1,mapbox.mapbox-streets-v5",
    "source-layer": "waterway_label",
    "filter": { "$type": "line" },
    "render": {
      "type": "text",
      "text-field": "{{name_en}}",
      "text-path": "curve",
      "text-font": "Open Sans Semibold Italic, Arial Unicode MS Bold",
      "text-max-size": 12,
      "text-max-angle": 0.5
    },
    "style": {
      "text-color": "@water_dark",
      "text-halo-width": 0.4,
      "text-halo-color": "@text_stroke"
    },
    "style.night": {
      "text-color": "@text_water_night",
      "text-halo-color": "@water_night"
    }
}

(found here) where no size is specified -- where does it inherit text-size from?

[edenh]

@lbud it inherits from text-max-size

[lbud]

@edenh thanks. Would there ever be a case where text-halo-width is specified but not text-max-size or text-size?

[edenh]

text-max-size used to be a required property for the text to be rendered, so no. I'm not sure if this will change with the upcoming symbol work (@ansis).

[lbud]

text-halo-width is now converting in v3 > v3.1 migrations here ... what should we do about text-halo-blur?

[ansis]

halo blur should also be in pixels. It's currently an even weirder value (multiples of some arbitrary constant value, with blur=1 being the default). I think this should convert it to pixels where 0 is the default

newHaloBlur = (oldHaloBlur - 1) * 2.5 / 24 * 8

[lbud]

Hm, ok. Weird thing about halo blur is that there is none committed anywhere in our repos, so ...do we even need to write migration code?

[ansis]

nope, no migration needed then

[lbud]

I'm trying to figure out where this comes from

newHaloBlur = (oldHaloWidth - 1) * 2.5 / 24 * 8

right now drawtext.js says

gl.uniform1f(shader.u_gamma, layerStyle['text-halo-blur'] * 2.5 / fontSize / window.devicePixelRatio);

do you mean to say that layerStyle['text-halo-blur'] * 2.5 / fontSize / window.devicePixelRatio) is oldHaloWidth? or...I'm confused as to what code this comes from

[ansis]

woops, oldHaloWidth was supposed to be oldHaloBlur (the value of the blur in the old stylesheet). But since no styles need to be converted, that equation can be forgotten. I'm not completely sure if it was even right.

The halo blur first needs to be converted from pixel units to sdf units.

sdfDistance = pixelDistance / (fontSize / 24) / 8
24 is the font size the sdf is rendered in
8 is the number of pixels from 0 to 1 values in the sdf

This is the same as what you changed for halo-width, except width has the 6 - ... offset. mapbox/mapbox-gl-js#514

Text is antialiased by always having at least a little blur. The halo blur equation is also going to need an offset so that there is some blur even when text-halo-blur is 0. The offset should be whatever value produces the sharpest text without looking pixelated (I think around + 1.6). The offset should be scaled by devicePixelRatio to avoid boing overly blurry on retina, so I think it would all look something like this:

gl.uniform1f(shader.u_gamma, layerStyle['text-halo-blur'] / (fontSize / 24) / 8 + 1.6 / window.devicePixelRatio)

[lbud]

I don't know if maybe I'm missing something but wouldn't it maybe look more like

gl.uniform1f(shader.u_gamma, (layerStyle['text-halo-blur'] / (fontSize / 24) + 1.6) / 8 * 2.5 / fontSize / window.devicePixelRatio);

(or that, and then simplified)...so the 2.5 (still not sure what that does) is preserved?
(Since the regular text gamma is

gl.uniform1f(shader.u_gamma, 2.5 / fontSize / window.devicePixelRatio);

)
And how is it working now without the 1.6 offset? brain melting 😕