Skip to content

feat: apply vrtr to rotated glyphs in vertical mixed layout - #59

Open
jafin wants to merge 1 commit into
mainfrom
feat/vertical-vrtr
Open

jafin wants to merge 1 commit into
mainfrom
feat/vertical-vrtr

Conversation

@jafin

@jafin jafin commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Closes #51

Summary

Feature planning per glyph in DefaultShaper:

 for each glyph
   if upright (IsVerticalGlyph)
     add vert
   else
     add calt, clig, liga, rclt, curs, kern
+    if layout is VerticalMixed*
+      add vrtr            # still rotated synthetically
 for each user feature
-  add to every glyph
+  if feature is vrt2 and layout is VerticalMixed*
+    add to upright glyphs only   # vrt2 glyphs are pre-rotated
+  else
+    add to every glyph
Mode vert vrtr Synthetic rotation
Horizontal* – – –
Vertical* all glyphs – none
VerticalMixed* upright glyphs rotated glyphs rotated glyphs

vrt2 is never added by default. TextOptions.FeatureTags documents the vrt2 rule. CHANGELOG has entries under Unreleased.

Evidence

  • Before: UserVrt2DoesNotRotateRotatedGlyphTwiceInVerticalMixedLayout fails. NotoSansKR, VerticalMixedRightLeft, FeatureTags = [vrt2], text あ…い: U+2026 (orientation R) becomes vrt2 glyph 21602 and is then rotated as well.
    Expected to be equivalent to [24543, 735, 24545]
    but collection does not contain expected item: 735
    
    RotatedGlyphIsShapedWithVrtrInVerticalMixedLayout fails, because the planned features have no vrtr.
  • After: both pass. The full suite passes on net8.0, net10.0 and net11.0 (12,369 tests, including the existing image-reference tests).

Gap (as the brief requires): no test font has a vrtr feature, so there is no end-to-end vrtr substitution test. vrtr is tested at the shaper-plan seam: rotated glyphs get it, upright glyphs and non-mixed modes do not. Synthetic rotation of vrtr glyphs is unchanged by construction, because the rotation path is not touched.

Merge Danger

Door: two-way

No public API change. A revert restores the old behaviour.

Blast Radius: narrow

Only vertical mixed layouts change. For fonts with a vrtr feature, rotated glyphs can change to the font's vrtr forms. Users who request vrt2 in vertical mixed layout no longer get pre-rotated glyphs on rotated runs. Output for every other font and mode is unchanged.

Rotated glyphs in VerticalMixed layouts now get the OpenType vrtr feature.
Its glyphs are designed to be rotated by the layout engine, so they are
still rotated synthetically.

vrt2 is still never applied by default. A vrt2 requested in
TextOptions.FeatureTags is now skipped for glyphs that are rotated in
vertical mixed layout, because its pre-rotated glyphs were rotated twice.

Closes #51
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0ff11f15-f36b-418b-a9c9-4d39fd9168af

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the enhancement New feature or request label Oct 26, 2026
@sonarqubecloud

Copy link
Copy Markdown

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

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vertical mixed layout: apply vrtr to rotated glyphs; don't apply vrt2

1 participant