Skip to content

Add text: Korean with an embedder-supplied face - #5158

Draft
taxjung85 wants to merge 1 commit into
storytold:mainfrom
taxjung85:korean-added-text
Draft

taxjung85 wants to merge 1 commit into
storytold:mainfrom
taxjung85:korean-added-text

Conversation

@taxjung85

Copy link
Copy Markdown

Korean in Add text, drawn with an embedder-supplied face

We embed PdfCraft's PDF window in a Korean web app (the wasm build). Add text currently refuses Hangul ("The PDF text fonts can't draw “ㅇ” (U+3147)"), so Korean users can't add a single word to a PDF.

This draft adds a Korean face slot that follows the Thai / Arabic / Hebrew path exactly:

  • pdfcraft_fonts::set_document_korean_font(&'static [u8]) lets the embedding app supply a face once (first one wins; unreadable bytes are refused). Nothing is built in and no font file is committed, so AGENTS.md section 1.4 stays intact. We pass an OFL-1.1 face (Pretendard) from our own app.
  • ShapedScript::Korean, korean_has, shape_korean (harfrust, script::HANGUL, grapheme clusters).
  • In Add text, a character the standard fonts can't draw and the supplied face can (korean_drawn) is shaped with that face and drawn as Type 3 outlines with a ToUnicode map, like the other shaped scripts. The face file is never embedded; only the glyphs used are drawn. Line wrapping, first_undrawable and the apply-time check use the same rule.
  • Without a supplied face nothing changes: such text is refused as before.

Test

crates/edit/tests/korean_face.rs runs in its own binary because the face is process-wide. Inter stands in for a Korean face (U+0150 takes the same path as Hangul): refused before, drawable after, written as Type 3, and read back by list_added with the same text. cargo test -p pdfcraft-edit -p pdfcraft-fonts passes locally.

We have shipped the same change on the 64e0868 base in our app, and checked a saved PDF: the text renders, and copying it returns the original Korean.

Questions for you

  • Would you rather have a craft-fonts face (e.g. a Kore script face) as the built-in source, with this setter only as an embedder override? We're happy to rework it that way.
  • If you want this reachable headlessly (automation tool, parity entry), tell us where you'd like it and we'll add it.

A character the standard fonts can't draw and the face supplied through
pdfcraft_fonts::set_document_korean_font can is shaped with that face and
drawn as Type 3 outlines with a ToUnicode map, like Thai, Arabic and Hebrew.
No font is built in or committed; without a supplied face nothing changes.
@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@Harrison-Booth

Copy link
Copy Markdown
Contributor

I checked the added test and the Korean routing/shaping path. The test uses Inter with U+0150, so it covers non-WinAnsi routing through the supplied face, but not Hangul; its list_added assertion reads the stored Add-text item and does not verify ToUnicode extraction from the saved PDF. Could you add a test using an already pinned craft-fonts Korean face, covering a precomposed syllable, compatibility Jamo, and mixed Latin/Hangul, and check Type 3 output plus copied/extracted text and the no-face refusal? No craft-fonts checkout was available here, so I did not run a Korean-face test or MuPDF/Poppler.

One API/headless point: set_document_korean_font is a first-set process-wide OnceLock, while page_add_text has no way to select/provide a face. That leaves a host with one fixed face per process and no corresponding face selection through the CLI/MCP tool. Is that intended for the WASM embedder?

For comparison, the open #2560 already adds craft-fonts-backed Korean Add-text tests, including a saved-PDF ToUnicode assertion, so it may be useful to compare the coverage and coordinate overlapping edit/fonts changes.

@croquete1

Copy link
Copy Markdown

Revisão

LGTM — Add text passa a shapear Coreano (Hangul) quando os fonts padrão não desenham, como já faz com árabe/hebraico/tailandês.

  • Proveniência limpa: o PR não commita fonte alguma (nenhum .ttf/.otf, nada em assets/fonts, sem ATTRIBUTION). A face coreana é fornecida em runtime via pdfcraft_fonts::set_document_korean_font — a própria mensagem de GlyphError::NoFont diz "Korean text needs a Korean face supplied with pdfcraft_fonts::set_document_korean_font". É o mesmo mecanismo de face suprida (como o craft-fonts), não fonte embutida no repo.
  • Never-panic: korean_has(c)/win_ansi_encodable decidem quando usar a face; o caminho sem fonte retorna erro acionável em vez de panicar.
  • Só código (crates/edit, crates/fonts) + teste korean_face.rs. Nada a corrigir.

This branch has not been deployed

No deployments
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.

3 participants