Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ndk: Add bindings for AFont #397

Merged
merged 53 commits into from
Aug 16, 2023
Merged

ndk: Add bindings for AFont #397

merged 53 commits into from
Aug 16, 2023

Conversation

paxbun
Copy link
Contributor

@paxbun paxbun commented Jun 15, 2023

This PR provides bindings for AFont, AFontMatcher, and ASystemFontIterator and some helper structs for the better abstraction of some parts of the OpenType specification.

Example:

for font in ndk::font::SystemFontIterator::new().unwrap() {
  log::debug!("Font: {}", font.path().display());
  for idx in 0..font.axis_count() {
    log::debug!("  Axis {idx} ({}): {}", font.axis_tag_at(idx), font.axis_value_at(idx));
  }
  log::debug!("  Collection Idx: {}", font.collection_index());
  log::debug!("  Locale: {:?}", font.locale());
  log::debug!("  Weight: {}", font.weight());
  log::debug!("  Is Italic: {}", font.is_italic());
}

Result (Tested on Galaxy Tab S8):
image

@paxbun
Copy link
Contributor Author

paxbun commented Jun 15, 2023

TODOs

  • Reflect PR number to CHANGELOG.md
  • Reflect NDK version (I used 25.1.8937393) to CHANGELOG.md and Cargo.toml or re-generate ndk-sys bindings

@MarijnS95
Copy link
Member

Nice! I'll pull in the bindings regeneration via #370, and then have a look at this.

ndk-sys/CHANGELOG.md Outdated Show resolved Hide resolved
ndk/CHANGELOG.md Outdated Show resolved Hide resolved
ndk-sys/wrapper.h Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Show resolved Hide resolved
ndk/src/font.rs Show resolved Hide resolved
@MarijnS95 MarijnS95 mentioned this pull request Aug 9, 2023
@MarijnS95
Copy link
Member

@paxbun any updates or should I apply the remaining comments/remarks/suggestions and merge this? I'd like to get the "breaking" changes imposed by the ndk-sys regen in and publish all these crates soon-ish™.

@paxbun
Copy link
Contributor Author

paxbun commented Aug 16, 2023

@MarijnS95 I think I mostly applied your suggestion, and I also checked whether there are any ffi::* functions not wrapped. I also added constructors that constly panic to AxisTag.

@MarijnS95
Copy link
Member

Note that I reported the header issues at android/ndk#1920, because surface_control.h (which I need and am writing a wrapper for) has the same (and many more C++-like) issues.

Hopefully we can drop the workarounds when upstream picks them up.

ndk/src/font.rs Outdated Show resolved Hide resolved
Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

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

Marked most of the conversations as resolved, nice job clearing it all up!

I only have nits now, and if you fix the conflicts to make the CI run it might have a complaint or two, but after that I'll merge this in!

ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Outdated Show resolved Hide resolved
ndk/src/font.rs Show resolved Hide resolved
@paxbun
Copy link
Contributor Author

paxbun commented Aug 16, 2023

I applied all the suggestions you mentioned! (except for the axis tag one)

@MarijnS95 MarijnS95 merged commit 9b94b63 into rust-mobile:master Aug 16, 2023
18 checks passed
@paxbun paxbun deleted the feat/afont branch December 14, 2023 04:58
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