Skip to content

Optional STB based Glyph Atlas Support#12

Merged
bdero merged 1 commit intobdero:masterfrom
johnoneil:user/joneil/stb-text-support
Jul 17, 2023
Merged

Optional STB based Glyph Atlas Support#12
bdero merged 1 commit intobdero:masterfrom
johnoneil:user/joneil/stb-text-support

Conversation

@johnoneil
Copy link

Overview

Here's the basic STB based Glyph Atlas support we've been working on.

The CMake footprint should keep the noop backend default unless IMPELLER_CMAKE_TYPOGRAPHER_BACKEND_STB is true. I do this for our builds by command line option -DIMPELLER_CMAKE_TYPOGRAPHER_BACKEND_STB=true.

If that's defined the typeface_stb and text_render_context_stb modules should be available for C++ clients instead of the current noop backend.

For the most part the text_render_context_stb is closely based on the Impeller text_render_context_skia.cc. I tried to make it as close as possible to ensure it behaves as similarly as possible so it's familiar and can be compared.

The only current test rigs we have are a couple of Rust based applications currently under review. This project forms the basis of Rust binding, so we exercise this module from Rust.

Any feedback or changes are appreciated.

Features:

  • Loading ttf and otf typefaces.
  • Dynamic atlas resizing
  • Efficient atlas updates with new glyphs that fit in the atlas of current size.

Deficiencies:

  • Color atlas support is disabled. The atlases build but we're seeing some kind of issue rendering Color atlas alpha correctly. We believe this issue is probably in our Rust binding. Not sure yet.

Additional Notes:

For our current target platform we have a texture size limitation of 2048x2048, so I've included a preprocessor definition to set this maximum size per build MAX_GLYPH_ATLAS_SIZE

Copy link
Owner

@bdero bdero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this impl is looks great! My comments are mainly around dependency management. Would be nice if we could pull the stb repo in as a submodule instead of versioning the headers inline here.

We're also missing a .clang-format in this repo, but we can take care of any Chromium-style deviations later once we've added a check for that.

Copy link
Owner

@bdero bdero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We recently how scaling works recently in the glyph atlas: flutter/engine#43695. I'll patch this backend with those changes next time we sync Flutter to HEAD in this repo.

@bdero bdero merged commit 44035a6 into bdero:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments