Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

text-max-size is applied to text-size, not just collision box #1333

Closed
friedbunny opened this issue Apr 23, 2015 · 5 comments
Closed

text-max-size is applied to text-size, not just collision box #1333

friedbunny opened this issue Apr 23, 2015 · 5 comments

Comments

@friedbunny
Copy link
Contributor

Per @peterqliu in chat, -native mistakenly applies the text-max-size styling property to the entire text and not just the collision box. Effectively, text-size can never be larger than text-max-size, even if it should be.

You can see this in Emerald's city labels. Set text-max-size: 35 and city labels scale as expected.

GL-JS appears to implement this properly (example, code), to spec.

Here is SymbolBucket::addFeature on -native.

The style spec says:

text-max-size
Optional number. Units in pixels. Defaults to 16. Requires text-field.
The maximum size text will be laid out, to calculate collisions with.

It seems like this property should be renamed to something like text-max-collision-size if it's not meant to constrain text-size.

@friedbunny
Copy link
Contributor Author

Workaround (and the way that Streets currently does it) is to always set text-max-size to your desired display maximum, which allows text to size properly but presumably also creates a large collision box.

@ansis
Copy link
Contributor

ansis commented Apr 23, 2015

I think this is the guilty line:

float fontSize = std::fmin(styleProperties.size, bucketProperties.max_size);

-js uses text-max-size only if text-size is missing, which I'm not sure actually happens ever: https://github.com/mapbox/mapbox-gl-js/blob/91285d64440a055cf4bc5d9e6b2d84baeb26c757/js/render/draw_symbol.js#L56

@friedbunny
Copy link
Contributor Author

@ansis So this is totally borked. I wonder, does the text of the spec represent an actual/desirable use-case, or should there also/only be a literal text-max-size?

@ansis
Copy link
Contributor

ansis commented Apr 23, 2015

Since text-size has a default, painter_symbol.cpp and draw_symbol.js should only use text-size.

Collision placement code should use text-max-size which it's already doing.

There is also an issue about getting rid of text-max-size.
mapbox/mapbox-gl-style-spec#255

@ansis
Copy link
Contributor

ansis commented Apr 24, 2015

Fixed in 17010bf

-js cleaned up in mapbox/mapbox-gl-js@98af024

@ansis ansis closed this as completed Apr 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants