Skip to content

Fix null default typeface on Android API 36; remove redundant C APIs#195

Merged
mattleibow merged 2 commits into
mono:skiasharpfrom
ramezgerges:dev/issue-3693-api36-null-typeface
Apr 25, 2026
Merged

Fix null default typeface on Android API 36; remove redundant C APIs#195
mattleibow merged 2 commits into
mono:skiasharpfrom
ramezgerges:dev/issue-3693-api36-null-typeface

Conversation

@ramezgerges

Copy link
Copy Markdown

Description of Change

SkiaSharp Issue

Related to https://github.com/mono/SkiaSharp/issues/

API Changes

Added

C API Purpose
sk_typeface_create_empty(void) Returns SkTypeface::MakeEmpty().release() — the minimal non-null primitive the managed binding uses as its fallback.

Removed

C API Replacement for external consumers
sk_typeface_create_default Call sk_fontmgr_match_family_style(sk_fontmgr_create_default(), nullptr, &Normal) with a null check + fallback to sk_typeface_create_empty.
sk_typeface_ref_default Same as above. Cache the result in your own storage.
sk_typeface_create_from_name sk_fontmgr_match_family_style on sk_fontmgr_create_default.
sk_typeface_create_from_file sk_fontmgr_create_from_file on sk_fontmgr_create_default.
sk_typeface_create_from_stream sk_fontmgr_create_from_stream on sk_fontmgr_create_default.
sk_typeface_create_from_data sk_fontmgr_create_from_data on sk_fontmgr_create_default.
sk_fontmgr_ref_default sk_fontmgr_create_default and cache.
sk_font_new sk_font_new_with_values(typeface, 12, 1, 0) with a non-null typeface.

Internal helpers removed

  • sk_get_default_typeface — the null-safety logic lives in managed now.
  • sk_get_default_fontmgr — the managed binding is the sole cache.
  • sk_create_default_fontmgr — its body (the #if cascade selecting a platform fontmgr) is now inlined into sk_fontmgr_create_default.
  • src/c/sk_default_fontmgr.h and src/c/sk_default_fontmgr.cpp deleted.

Contract changes

  • sk_font_new_with_values now requires a non-null typeface argument. Passing nullptr is undefined behavior.
  • SkCompatPaint's default constructor now constructs its backing SkFont with SkTypeface::MakeEmpty() instead of the platform default. SkCompatPaint exists only to back the deprecated SKPaint text-rendering surface in the managed binding; modern callers should use SKFont directly, which is always constructed with a real typeface.

Behavioral Changes

None.

Required SkiaSharp PR

Requires mono/SkiaSharp#3730

PR Checklist

  • Rebased on top of skiasharp at time of PR
  • Changes adhere to coding standard
  • Updated documentation

ramezgerges and others added 2 commits April 24, 2026 14:54
sk_get_default_typeface previously called legacyMakeTypeface("", Normal),
which returns null on Android API 36's SkFontMgr_Android. The subsequent
->ref() in sk_typeface_ref_default crashed with SIGSEGV at offset 0x8
(fault addr 0x8 — the sk_sp internal pointer).

- Drop sk_typeface_ref_default, sk_typeface_create_default,
  sk_fontmgr_ref_default, sk_font_new, and the four
  sk_typeface_create_from_* functions. The managed binding now owns
  default-typeface resolution via SKFontManager.Default.MatchFamily
  with a fallback to the new minimal primitive sk_typeface_create_empty.
  ABI break — requires a SkiaSharp major version bump.

- Add sk_typeface_create_empty: a thin C API exposing
  SkTypeface::MakeEmpty() for managed callers that need a non-null
  fallback typeface.

- SkCompatPaint's default constructor now uses SkTypeface::MakeEmpty()
  directly. SKPaint's text-via-font surface is already [Obsolete] in
  managed — modern callers should use SKFont, which managed always
  instantiates with a real typeface handle.

- sk_font_new_with_values now requires a non-null typeface handle
  (managed always passes one via SKFont(SKTypeface.Default)).

- Remove the sk_default_fontmgr.{h,cpp} helper module entirely. The
  platform-fontmgr selection is now a single #if cascade inside
  sk_fontmgr_create_default in sk_typeface.cpp. Managed
  SKFontManager.Default is the sole cache of the default font manager.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SkCompatPaint's default constructor initialized its SkFont with
SkTypeface::MakeEmpty, which has zero glyphs. The managed binding
compensated by overwriting the font immediately after construction;
that dance served no purpose. Require every SkCompatPaint to be
constructed with an explicit font, so managed can pass a pinned
default SKFont once and rely on SkCompatPaint to copy from it.

- Remove SkCompatPaint::SkCompatPaint(). SkCompatPaint(const SkFont*)
  is now the only forward constructor; copy ctor is unchanged.
- SkCompatPaint::reset() becomes reset(const SkFont*): callers provide
  the reset-to-default font.
- Remove sk_compatpaint_new; sk_compatpaint_reset's signature gains a
  sk_font_t* argument. Update SkiaKeeper to reference
  sk_compatpaint_new_with_font instead of the removed sk_compatpaint_new.
- ABI break — pairs with a managed-side change in SkiaSharp.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ramezgerges
ramezgerges force-pushed the dev/issue-3693-api36-null-typeface branch from 1ec11cd to 4db5aa5 Compare April 24, 2026 14:57
@mattleibow
mattleibow merged commit fb76f3d into mono:skiasharp Apr 25, 2026
1 check passed
@mattleibow

Copy link
Copy Markdown
Collaborator

⚠️ Process error: The sk_fontmgr_legacy_create_typeface function that this PR was named after was not included in the squash merge.

During review, the function was added in commit e6736d19f but it was pushed to a separate branch (dev/issue-3693-legacy-typeface) on mono/skia instead of being pushed to the PR author's fork branch. When this PR was squash-merged, only the commits on the fork's branch were included — the separately pushed commit was left behind.

This was my mistake — I should have pushed the commit to the PR branch (the author's fork) before merging, not to a separate branch on the upstream repo.

Fix: #198 cherry-picks the missing commit onto skiasharp.

mattleibow added a commit that referenced this pull request Apr 25, 2026
## Context

- mono/SkiaSharp#3693
- #195

## What happened

During the review of PR #195, the `sk_fontmgr_legacy_create_typeface` function was added
in a separate commit (`e6736d19f`) pushed to a local branch (`dev/issue-3693-legacy-typeface`)
on mono/skia — but this commit was **not pushed to the PR author's fork branch**. When PR #195
was squash-merged, only the commits on the fork's branch were included. The separately pushed
commit was left behind, so the function that the PR was named after was never actually merged.

This was a process error: the fix should have been pushed to the PR branch (on the author's
fork) before merging, not to a separate branch on the upstream repo.

## Fix

This PR cherry-picks commit `e6736d19f23da0746c4291ed7223e69ffbce3ac1` onto `skiasharp` to
add the missing function.

## What the function does

`sk_fontmgr_legacy_create_typeface` exposes `SkFontMgr::legacyMakeTypeface()` to the C API.
It is needed by SkiaSharp's `SKTypeface.Default` to reliably resolve the platform default
typeface on Android, where `matchFamilyStyle(null)` returns null because `onMatchFamily(null)`
rejects null family names. `legacyMakeTypeface(null)` bypasses this by using `fDefaultStyleSet`
(which searches "sans-serif", "Roboto", then falls back to style set 0).

Without this function, `SKTypeface.Default` falls back to the empty typeface on Android API 36,
causing all text measurement and drawing to produce zero/nothing.
mattleibow added a commit to mono/SkiaSharp that referenced this pull request Apr 25, 2026
Updates submodule from fb76f3dd84 to f5345469d1 (mono/skia#198).
Adds the sk_fontmgr_legacy_create_typeface C API that was missing
from the squash merge of mono/skia#195.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ramezgerges added a commit to ramezgerges/skia that referenced this pull request Apr 26, 2026
Brings in the latest mono/skia 'skiasharp' branch updates on top of the
m147 work-in-progress:

  * Add variable font C API for typeface variation queries and cloning (mono#185)
  * Add sk_fontmgr_legacy_create_typeface C API (mono#198, mono#195) — replaces
    the old sk_default_fontmgr.{cpp,h} with a single C entry point that
    forwards to SkFontMgr::legacyMakeTypeface(). Fixes the Android
    default-typeface SIGSEGV.
  * Use int32_t instead of long for SkManagedStream move offset
    (mono#186) — fail-loud on overflow instead of silent truncation
    of platform-dependent C long.
  * Update libpng to 1.6.58
  * Update libexpat to 2.7.5

Conflict resolutions:
  - DEPS: keep m147's freetype roll, take origin/skiasharp's libexpat 2.7.5
    (and libpng 1.6.58 auto-merged the same way).
  - src/c/sk_default_fontmgr.cpp: accept origin/skiasharp's deletion;
    their legacyMakeTypeface refactor obviates our m147 carry of that file.
  - src/c/sk_typeface.cpp: re-apply our m139 SkFontScanner fix to the
    Android and FontConfig SkFontMgr_New_* call sites, which moved into
    this file as part of their refactor.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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