Integrate Harffbuzz freetype to render Khmer Burmese and hindi

[alanchenboy]

refactor for #1289

This is a demo of how to use harfbuzz to render complex text.

Example style: https://alanchenboy.github.io/harfbuzzResource/hindi.json.

Screenshots: .

https://github.com/maplibre/maplibre-native/assets/5135389/f8f294ec-1d20-4106-b745-51d5f4a1af1d.

https://github.com/maplibre/maplibre-native/assets/5135389/7e3857e9-74c4-405e-b12c-e5722cf76919.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 93.22382% with 33 lines in your changes are missing coverage. Please review.

Project coverage is 85.78%. Comparing base (04be6ec) to head (6b02cba).

:exclamation: Current head 6b02cba differs from pull request most recent head d65c81d

Please upload reports for the commit d65c81d to get more accurate results.

Files	Patch %	Lines
src/mbgl/text/shaping.cpp	85.56%	14 Missing :warning:
src/mbgl/text/harfbuzz.cpp	78.94%	8 Missing :warning:
src/mbgl/text/glyph_manager.cpp	92.59%	4 Missing :warning:
src/mbgl/text/glyph.cpp	91.66%	2 Missing :warning:
src/mbgl/layout/layout.hpp	50.00%	1 Missing :warning:
src/mbgl/layout/symbol_layout.cpp	98.90%	1 Missing :warning:
src/mbgl/storage/resource.cpp	87.50%	1 Missing :warning:
src/mbgl/tile/geometry_tile_worker.cpp	96.77%	1 Missing :warning:
src/mbgl/util/i18n.cpp	91.66%	1 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1439       +/-   ##
===========================================
+ Coverage   59.30%   85.78%   +26.48%     
===========================================
  Files         580      566       -14     
  Lines       28674    28188      -486     
  Branches    11276        0    -11276     
===========================================
+ Hits        17006    24182     +7176     
+ Misses       4139     4006      -133     
+ Partials     7529        0     -7529     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[alanchenboy]

Swipe out macos end is such a pain for me, I have to use my personal computer to fix unit tests

[ntadej]

I wonder if we want to make this an optional feature? This would allow us to fix Qt later for example.

Also what is the binary size increase? (@louwers, didn't we have a CI test for that in the past? I guess the rest of the actions need to pass.)

[alanchenboy]

https://github.com/maplibre/maplibre-native/actions/runs/5747452827/job/15578480347 does anybody known what is the reason for this failure?

[alanchenboy]

I didn't have linux end, will fix linux CI in next weeks.

[louwers]

@alanchenboy That means the iOS render tests are failing. Did you run them locally?

[alanchenboy]

This looks quite hard to pass all unit tests. I will investigate why these changes impact some render tests like pure fill render results.

[louwers]

@alanchenboy CMake has been updated.

[alanchenboy]

No cmake in this job? https://github.com/maplibre/maplibre-native/actions/runs/6464095713/job/17551288992?pr=1439

[github-actions[bot]]

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +11% +15.3Mi  +7.0% +2.09Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1439-compared-to-main.txt

Compared to d38709084a9865fe0bb8300aec70ebf8243b3d43 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +32% +37.4Mi  +436% +26.0Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1439-compared-to-legacy.txt

[alanchenboy]

I think opt-out/in is feasible. let me take a try

[alanchenboy]

@louwers add a flag to turn off harfbuzz freetype default, can use bazel to enable it in the iOS platform example:

bazel run //platform/ios:xcodeproj --@rules_xcodeproj//xcodeproj:extra_common_flags="--//:renderer=legacy --//:maplibre_platform=ios --//:shaping=harfbuzz"

[wipfli]

I would love to try out this pull request with the linux renderer. @louwers is this possible and are there some compilation guidelines?

[louwers]

@wipfli Yes, follow the Linux link in the main README.

Check out this branch (gh pr checkout 1439 or manyally add Alan's remote) and don't forget to

git submodule update --init

To pull in Harfbuzz as a submodule.

[alanchenboy]

@wipfli Linux to turn on harfbuzz can add cmake parameter: -DMLN_TEXT_SHAPING_HARFBUZZ=ON

thanks for test and review.

[wipfli]

Looking at your code and the example style sheet, the only change to the style specification is the new font property, right?

"fonts": "https://alanchenboy.github.io/harfbuzzResource/fonts/{fontstack}/{language}.ttf",

[wipfli]

I think it would be best if the user could indicate what unicode range a font supports in the style sheet. Something similar to how browsers do it in css @font-face would be ideal I think.

https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/unicode-range

[alanchenboy]

@wipfli Sound reasonable, Let me redesign the style when I am free. Thanks for the advice.

[wipfli]

We can use the geojson from https://github.com/wipfli/i18n-testsuite-maplibre to test if this pull request works with most relevant scripts...

[wipfli]

Should the -DMLN_TEXT_SHAPING_HARFBUZZ=ON flag be in the github workflow?

https://github.com/maplibre/maplibre-native/blob/9e7f1cc3a579e096fb2f750c14078c823b2fe545/.github/workflows/linux-ci.yml#L96 ?

[wipfli]

When I compile without the -DMLN_TEXT_SHAPING_HARFBUZZ=ON and run ./build/bin/mbgl-render --style https://alanchenboy.github.io/harfbuzzResource/hindi.json --output out.png I get this image:

without the flag:

And when I compile it with the flag I get: with the flag:

In the first image, the Hindi labels are not rendered correct which is what one expects without harfbuzz. But actually I thought that MapLibre Native just shows nothing if it encounters Hindi by default, no?

[louwers]

Should the -DMLN_TEXT_SHAPING_HARFBUZZ=ON flag be in the github workflow?

https://github.com/maplibre/maplibre-native/blob/9e7f1cc3a579e096fb2f750c14078c823b2fe545/.github/workflows/linux-ci.yml#L96 ?

This brings up an interesting point. In the current state, if we decide to compile with Harfbuzz support on CI, builds without Harfbuzz will be untested. In fact, it not even sure they will even compile.

Ideally we don't rely on #ifdef to choose a code path. We should have a specializations of SymbolLayout, GlyphRange, GlyphRequestor etc. that use HarfBuzz and specializations that don't. It should be possible to compile both versions at the same time (so we can at least compile both on CI without a proliferation of build config permutations).

Our binary distribution for Android and iOS should probably contain the 'kitchen sink' of all features available. Binary size is probably only important to big players who will make their own builds anyway (feel free to correct me here). We could consider introducing a plugin system, but I doubt that will be worth the effort.

[alanchenboy]

Keeping both versions for render tests is challenging, some render tests will get different download results between PBF file and the font file. This project is not like metal, Basically, it aims for the same result, but changing the shaping will make many tests turn out differently, such as the render test containing the Khmer text.

[alanchenboy]

In the first image, the Hindi labels are not rendered correct which is what one expects without harfbuzz. But actually I thought that MapLibre Native just shows nothing if it encounters Hindi by default, no?

Hindi could be drawn incorrectly with PBF, like your first screenshot @wipfli, but these text is not readable in many complex languages.

[alanchenboy]

use font face to to define font files, example: https://alanchenboy.github.io/harfbuzzResource/hindi.json

    "font-faces": [
        {
            "text-font": ["Noto Sans Regular"],
            "font-files": [
                {
                    "url": "https://alanchenboy.github.io/harfbuzzResource/fonts/Noto%20Sans%20Regular/khmer.ttf",
                    "unicode-range": [
                        "U+1780-17FF"
                    ]
                },
                {
                    "url":"https://alanchenboy.github.io/harfbuzzResource/fonts/Noto%20Sans%20Regular/myanmar.ttf",
                    "unicode-range": [
                        "U+1000-109F", "U+A9E0-0xA9FF", "U+AA60-0xAA7F"
                    ]
                },
                {
                    "url":"https://alanchenboy.github.io/harfbuzzResource/fonts/Noto%20Sans%20Regular/devanagari.ttf",
                    "unicode-range": [
                        "U+0900-097F", "U+A8E0-0xA8FF"
                    ]
                }
                
            ]
        }
    ],

[louwers]

!benchmark android

[keith]

phew! I think we might still have to have the empty WORKSPACE file tho

[louwers]

@keith Seems to work without.

@alanchenboy I am enabling Harfbuzz by default to run some benchmarks.

[wipfli]

I think this is coming together nicely. Thanks for keeping working on this pull request @alanchenboy.

We want to be able to compile without HarfBuzz because of binary size. To reduce the number of ifdefs we could just have the ifdef only inside the function which calls HarfBuzz.

If you compile with HarfBuzz, this function will return the glyph vector and metrics vector filled with text shaping results. If you compile without HarfBuzz, this function will return empty vectors.

What do you think?

[alanchenboy]

Sorry for the late response, I was a little bit occupied these days. If without harfbuzz, we can simply ignore the try shaping intent at the layout thread,

I may spare some time this week, let me figure this out .