tangrams / bubble-wrap

Bubble Wrap basemap style
http://tangrams.github.io/bubble-wrap/
MIT License
27 stars 21 forks source link

Don't rely on default font size #242

Open bcamper opened 7 years ago

bcamper commented 7 years ago

I've been noticing some odd road labels, and I think that the problem is relying on the default font.size parameter. For example, check out these big road labels for 14th Ave, 18th Ave, etc. on Tangram ES:

native

They look more normal in Tangram JS, but in both cases I think it's falling back on the default font size, which must be smaller in JS (12px) - @tallytalwar or @karimnaaji?

web

This happens on (at least?) a few layers, including secondary and tertiary roads at some zooms. In this example, at z12 we're matching to labels-secondary-default, which has no font block but inherits some other font params from roads.

Starting at z13, the font size is explicitly set:

                labels-secondary-default:
                    draw:
                        text-blend-order:
                            text_source: global.ux_language_text_source_road_ref_and_name_short
                            visible: global.text_visible_secondary
                labels-secondary-z13:
                    filter:
                        $zoom: [13]
                    draw:
                        text-blend-order:
                            priority: 56
                            visible: global.text_visible_secondary_e
                            #text_source: global.ux_language_text_source_road_ref_and_name_short
                            font:
                                fill: [0.5,0.5,0.5]
                                size: 11px
                                stroke: { color: global.text_stroke, width: 2 }

This also begs the question of whether we should even have a default size, or whether we should be requiring this to be set explicitly for labels to render? cc @matteblair

nvkelso commented 7 years ago

I had noticed the same thing using the iOS demo app, thanks for the sleuthing!

From https://mapzen.com/documentation/tangram/draw/#font-parameters:

The font object has a number of unique parameters.
None are required, but least one must be specified for a text style to be drawn.

I've summarized the defaults below (different than the example in docs):

draw:
    text:
        font:
            family: Helvetica
            size: 12px               # with px being the default units
            weight: normal        # is this `400`?
            fill: white                 # arguably this should be black
            #style: italic            # no default (effectively "normal")
            #stroke: { color: white, width: 2 }    # no default
            # transform: uppercase                  # no default

My preference is for ES and JS to have the same default text font size (12px).

nvkelso commented 7 years ago

(Which would make this a Tangram ES issue, with no change for Bubble Wrap scene file.)

matteblair commented 7 years ago

A cursory glance at the relevant code in tangram-es leads me to believe that we currently do not default to a font size of 12 px: https://github.com/tangrams/tangram-es/blob/master/core/src/style/textStyle.h#L33

@karimnaaji or @hjanetzek can you clarify whether I'm looking at the right value and what the units for this are?

karimnaaji commented 7 years ago

Yes it seems that there is a mismatch on the default value here between JS and ES, we use 16pixels.

@matteblair this is the right place to look at, the value is defined in the stylesheet default units (pixels).

tallytalwar commented 7 years ago

Correct.

screen shot 2017-04-03 at 4 38 05 pm

Left is the default on ES, right is setting default to 12px.