mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.16k stars 2.21k forks source link

Limit SDF dowscaling #7652

Open brunoabinader opened 5 years ago

brunoabinader commented 5 years ago

Given the following style.json render test example:

{
  "version": 8,
  "metadata": {
    "test": { "height": 256, "width": 256 }
  },
  "center": [ 0, 0 ],
  "zoom": 0,
  "sources": {
    "point": {
      "type": "geojson",
      "data": {
        "type": "FeatureCollection",
        "features": [
          {
            "type": "Feature",
            "properties": { "name": "Napoli", "name_en": "Naples" },
            "geometry": {
              "type": "Point",
              "coordinates": [ 0, 0 ]
            }
          }
        ]
      }
    }
  },
  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
  "layers": [
    {
      "id": "text",
      "type": "symbol",
      "source": "point",
      "layout": {
        "text-field": ["format",
            ["get", "name_en"], { "font-scale": 48 },
            "Italy", { "font-scale": 16 } ,
            "\n", {},
            ["get", "name"], { "font-scale": 16, "text-font": ["literal", [ "NotoCJK" ]] }
        ],
        "text-font": [
          "Open Sans Semibold",
          "Arial Unicode MS Bold"
        ],
        "text-size": 1
      }
    }
  ]
}

In the example above, text-size is set to 1, while font-scale is set to different values. This generates downscaled SDF textures that produces unreadable results.

Rendered result: actual

/cc @ansis @ChrisLoer

ChrisLoer commented 5 years ago

From Slack, @ansis commented:

Hmm, although that is a slightly surprising usage it sounds like there is a bug caused by not adjusting the sdf blurring distance for the scaled size. I think this should be fixable. Can you open an issue and tag Chris?

Passing variable halo distance (or really, variable text-size) to the shaders would require us to do something like a paint property binder whenever we used format expressions. The current implementation takes advantage of being able to implement "font-scale" purely by changing the layout vertices. This has the advantage that no extra data has to get passed to the GPU to support multiple "scales" in a single draw call, but leads to these artifacts with large font-scales. See discussion at https://github.com/mapbox/mapbox-gl-js/pull/6994#issuecomment-407230046.

Putting an explicit limit on font-scale might help guide people in the right direction here. Maybe .25 <= font-scale <= 4?

ansis commented 5 years ago

I think text-size is already always data-driven so baking it in there wouldn't require any extra binding. I'm not familiar with the formatted text implementation. Would it be hard to implement it with this?

ChrisLoer commented 5 years ago

You're right, I think we can do it that way.