Skip to content

Commit 355a469

Browse files
DemchaAVDemchaAV
committed
fix(engine): let an author say where Arabic must not join
The zero-width non-joiner and joiner are how Unicode lets an author override contextual joining — U+200C forbids a join the letters would otherwise make, U+200D forces one — and both were deleted before the shaper, the only thing that reads them, ever ran. They are category C like any other control, so the sanitizer that runs first removed them along with the rest, and a document could not express the distinction at all. The shaper needed almost nothing for the non-joiner: it is an opaque non-joining character, so the lookups that walk letters already came to the right answer once it survived. The joiner is the one that had to be taught, since it stands in for a letter that is not there. Both are consumed during shaping, because past that point they are code points no font can encode — measured as one thing and drawn as a substitution mark. Text with no Arabic in it never reaches the shaper, so the glyph seam drops them there instead. The joining controls get their own predicate rather than joining the bidirectional ones: these say nothing about direction, only about whether two letters connect. Also: - Auto-size measured a highlight run unshaped while the token it builds from that run is shaped, so a chip was sized against a string the layout never uses. Both paths shape now. This is not currently visible end to end, because auto-size does not shrink a paragraph holding a chip at all for any script, which is its own defect. - The degraded-shaping warning is keyed on the font rather than on (font, code point). A font that carries no presentation forms carries none of them, so the second letter said nothing the first had not, and a document turned one fact about the font into a warning per letter. - The changelog claimed mirroring per UAX #9 L4 without the scope the implementation has always documented: the punctuation that occurs in documents, not the whole mirroring table.
1 parent db29d54 commit 355a469

8 files changed

Lines changed: 201 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ follow semantic versioning; release dates are ISO 8601.
5555
that carries the letters but not the forms (the `GSUB`-only families) now degrades
5656
to unjoined base letters instead of `?`, which costs the joining rather than the
5757
text. In right-to-left runs, paired punctuation is mirrored at the PDF seam
58-
(UAX #9 L4), so a parenthesis in Hebrew faces what it encloses.
58+
(UAX #9 L4), so a parenthesis in Hebrew faces what it encloses. The mirrored set is
59+
the punctuation that occurs in documents — parentheses, brackets, braces, angle
60+
brackets, guillemets — rather than the whole Unicode mirroring table; a mathematical
61+
operator passes through drawn as written.
5962

6063
### Fonts
6164

‎core/src/main/java/com/demcha/compose/document/layout/TextFlowSupport.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,16 @@ private static boolean paragraphFitsSingleLine(ParagraphNode node,
619619
} else if (run instanceof InlineSvgRun svgRun) {
620620
width += svgRun.width();
621621
} else if (run instanceof InlineHighlightRun highlight) {
622-
width += measurement.textWidth(engineStyle, highlight.text())
622+
// Shaped, like the token this run becomes. An unshaped measurement
623+
// sizes the text off one string and lays it out from another: the
624+
// presentation forms carry their own advance widths, so the probe
625+
// was answering about a string the layout never sees. Not currently
626+
// observable end to end — auto-size does not shrink a paragraph
627+
// holding a chip at all, for any script — but the two paths have to
628+
// agree regardless of which of them is reached first.
629+
// Shaped, like the token this run becomes: an unshaped measurement
630+
// sizes the text off one string and lays it out from another.
631+
width += measurement.textWidth(engineStyle, ArabicShaper.shape(highlight.text()))
623632
+ highlight.background().padding().horizontal();
624633
}
625634
}

‎core/src/main/java/com/demcha/compose/engine/text/TextControlSanitizer.java‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ public static String replace(String text, String replacement) {
4444
* anything had a chance to act on it.</p>
4545
*
4646
* @param text source text
47-
* @return sanitized text keeping the bidirectional formatting characters, never {@code null}
47+
* <p>The Arabic joining controls (U+200C, U+200D) are kept for the same reason and
48+
* read by the same kind of later seam: the shaper consumes them to decide whether
49+
* two letters connect.</p>
50+
*
51+
* @return sanitized text keeping the formatting controls, never {@code null}
4852
* @since 2.2.0
4953
*/
5054
public static String removeExceptDirectionMarks(String text) {
@@ -56,7 +60,7 @@ public static String removeExceptDirectionMarks(String text) {
5660
for (int inputIndex = 0; inputIndex < text.length(); ) {
5761
int codePoint = text.codePointAt(inputIndex);
5862
int charCount = Character.charCount(codePoint);
59-
if (isCategoryC(codePoint) && !isBidiControl(codePoint)) {
63+
if (isCategoryC(codePoint) && !isFormattingControl(codePoint)) {
6064
if (sanitized == null) {
6165
sanitized = new StringBuilder(text.length());
6266
sanitized.append(text, 0, inputIndex);
@@ -88,7 +92,7 @@ public static String removeDirectionMarks(String text) {
8892
StringBuilder sanitized = null;
8993
for (int index = 0; index < text.length(); index++) {
9094
char character = text.charAt(index);
91-
if (isBidiControl(character)) {
95+
if (isFormattingControl(character)) {
9296
if (sanitized == null) {
9397
sanitized = new StringBuilder(text.length());
9498
sanitized.append(text, 0, index);
@@ -109,6 +113,37 @@ public static String removeDirectionMarks(String text) {
109113
* @return {@code true} for a bidirectional formatting character
110114
* @since 2.2.0
111115
*/
116+
/**
117+
* Returns whether a code point is an Arabic joining control — the zero-width
118+
* non-joiner or joiner.
119+
*
120+
* <p>These are not bidirectional controls and are kept separate from them: they say
121+
* nothing about direction, they say whether two letters may connect. U+200C forbids
122+
* a join that would otherwise happen and U+200D forces one, which is the author's
123+
* only way to write a form the contextual rules would not produce. Both draw
124+
* nothing, and both are category C, so the plain control sanitizer deletes them —
125+
* before the shaper that is their only reader has run.</p>
126+
*
127+
* @param codePoint code point to test
128+
* @return {@code true} for U+200C or U+200D
129+
* @since 2.2.0
130+
*/
131+
public static boolean isJoiningControl(int codePoint) {
132+
return codePoint == 0x200C || codePoint == 0x200D;
133+
}
134+
135+
/**
136+
* Returns whether a code point steers the text pipeline and draws nothing — a
137+
* bidirectional formatting character or an Arabic joining control.
138+
*
139+
* @param codePoint code point to test
140+
* @return {@code true} for a formatting control the pipeline reads and then drops
141+
* @since 2.2.0
142+
*/
143+
public static boolean isFormattingControl(int codePoint) {
144+
return isBidiControl(codePoint) || isJoiningControl(codePoint);
145+
}
146+
112147
public static boolean isBidiControl(int codePoint) {
113148
return codePoint == 0x061C
114149
|| codePoint == 0x200E

‎core/src/main/java/com/demcha/compose/engine/text/bidi/ArabicShaper.java‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,19 @@ private ArabicShaper() {
3636
/** Last code point of the table: U+064A, yeh. */
3737
private static final int BASE_LAST = 0x064A;
3838

39+
/**
40+
* U+200D, joining type C: stands in for a letter that would have joined, so the
41+
* neighbour takes a connected form even with nothing to connect to.
42+
*/
43+
private static final char ZERO_WIDTH_JOINER = 0x200D;
44+
/**
45+
* U+200C, joining type U: an opaque non-joining character. It needs no special case
46+
* in the lookups that walk letters — it is simply not a letter — but the two that
47+
* ask about an adjacent character directly have to see it as a break rather than as
48+
* an absence.
49+
*/
50+
private static final char ZERO_WIDTH_NON_JOINER = 0x200C;
51+
3952
/** Joining behaviour of a letter. */
4053
private static final int NON_JOINING = 0;
4154
/** Joins only to the letter before it (isolated and final forms exist). */
@@ -126,6 +139,13 @@ public static String shape(String text) {
126139
StringBuilder shaped = new StringBuilder(text.length());
127140
for (int index = 0; index < text.length(); index++) {
128141
char character = text.charAt(index);
142+
if (TextControlSanitizer.isJoiningControl(character)) {
143+
// Read above as context, dropped here: the control has done its work by
144+
// the time the neighbouring letters have their forms, and carrying it
145+
// further would hand the font a code point it cannot encode — measured
146+
// as one thing and drawn as a substitution mark.
147+
continue;
148+
}
129149
int[] letter = letterAt(character);
130150
if (letter == null) {
131151
shaped.append(character);
@@ -266,6 +286,12 @@ private static boolean joinsPrevious(String text, int index) {
266286
if (isTransparent(character)) {
267287
continue;
268288
}
289+
if (character == ZERO_WIDTH_JOINER) {
290+
return true;
291+
}
292+
if (character == ZERO_WIDTH_NON_JOINER) {
293+
return false;
294+
}
269295
int[] letter = letterAt(character);
270296
return letter != null && letter[0] == DUAL_JOINING;
271297
}
@@ -282,7 +308,14 @@ private static boolean joinsNext(String text, int index) {
282308
if (after < 0) {
283309
return false;
284310
}
285-
int[] next = letterAt(text.charAt(after));
311+
char following = text.charAt(after);
312+
if (following == ZERO_WIDTH_JOINER) {
313+
return true;
314+
}
315+
if (following == ZERO_WIDTH_NON_JOINER) {
316+
return false;
317+
}
318+
int[] next = letterAt(following);
286319
return next != null && next[0] != NON_JOINING;
287320
}
288321

‎core/src/test/java/com/demcha/compose/engine/text/TextControlSanitizerTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,26 @@ void isBidiControlCoversTheMarksEmbeddingsOverridesAndIsolates() {
6060
assertThat(TextControlSanitizer.isBidiControl(0x200B)).isFalse();
6161
assertThat(TextControlSanitizer.isBidiControl(0xFEFF)).isFalse();
6262
}
63+
64+
@Test
65+
void theJoiningControlsSurviveLayoutSanitizingBecauseTheShaperIsTheirReader() {
66+
// Written as escapes: a raw zero-width control is invisible in review.
67+
String zwnj = "‌";
68+
String zwj = "‍";
69+
70+
assertThat(TextControlSanitizer.removeExceptDirectionMarks("ب" + zwnj + "ه"))
71+
.describedAs("U+200C is category C like any other control, but deleting it "
72+
+ "here removes the author's instruction before the shaper — the "
73+
+ "only thing that reads it — has run")
74+
.isEqualTo("ب" + zwnj + "ه");
75+
assertThat(TextControlSanitizer.removeExceptDirectionMarks("ب" + zwj))
76+
.isEqualTo("ب" + zwj);
77+
}
78+
79+
@Test
80+
void theJoiningControlsAreDroppedAtTheSeamThatHandsTextToABackend() {
81+
assertThat(TextControlSanitizer.removeDirectionMarks("ب‌ه"))
82+
.describedAs("no font can encode them, so they must not reach one")
83+
.isEqualTo("به");
84+
}
6385
}

‎core/src/test/java/com/demcha/compose/engine/text/bidi/ArabicShaperTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,56 @@ void everyProducedFormMapsBackToItsBaseLetters() {
118118
assertThat(ArabicShaper.baseLettersOf('A')).isNull();
119119
assertThat(ArabicShaper.baseLettersOf(0x05D0)).isNull();
120120
}
121+
122+
// U+200C and U+200D are written as escapes on purpose: a raw zero-width control in a
123+
// test source is invisible in review, and this file is about exactly those characters.
124+
private static final String ZWNJ = "‌";
125+
private static final String ZWJ = "‍";
126+
127+
@Test
128+
void aZeroWidthNonJoinerBreaksAJoinTheLettersWouldOtherwiseMake() {
129+
// beh + heh join; with ZWNJ between them the author is saying they must not.
130+
String joined = ArabicShaper.shape("به");
131+
String broken = ArabicShaper.shape("ب" + ZWNJ + "ه");
132+
133+
assertThat(joined).isNotEqualTo(broken);
134+
assertThat(broken)
135+
.describedAs("beh takes its isolated form and heh its own, as if they were "
136+
+ "not adjacent; the control itself draws nothing and is consumed")
137+
.isEqualTo("ﺏﻩ");
138+
assertThat(broken).doesNotContain(ZWNJ);
139+
}
140+
141+
@Test
142+
void aZeroWidthJoinerMakesALetterTakeAConnectedFormWithNothingToConnectTo() {
143+
String isolated = ArabicShaper.shape("ب");
144+
String joinedForward = ArabicShaper.shape("ب" + ZWJ);
145+
146+
assertThat(isolated).isEqualTo("ﺏ");
147+
assertThat(joinedForward)
148+
.describedAs("the joiner stands in for a following letter, so beh takes its "
149+
+ "initial form — the way an author quotes a connected form on its own")
150+
.isEqualTo("ﺑ");
151+
assertThat(joinedForward).doesNotContain(ZWJ);
152+
}
153+
154+
@Test
155+
void aJoiningControlDoesNotSurviveIntoTheShapedText() {
156+
// Nothing downstream can encode them: a font has no glyph for either, so a control
157+
// that survived shaping would be measured as one thing and drawn as '?'.
158+
assertThat(ArabicShaper.shape("م" + ZWNJ + "ر" + ZWJ + "ح"))
159+
.doesNotContain(ZWNJ)
160+
.doesNotContain(ZWJ);
161+
}
162+
163+
@Test
164+
void aJoiningControlBetweenLamAndAlefKeepsThemOutOfTheLigature() {
165+
String ligated = ArabicShaper.shape("لا");
166+
String separated = ArabicShaper.shape("ل" + ZWNJ + "ا");
167+
168+
assertThat(ligated).hasSize(1);
169+
assertThat(separated)
170+
.describedAs("an author who broke the join must not get the ligature back")
171+
.hasSize(2);
172+
}
121173
}

‎render-pdf/src/main/java/com/demcha/compose/engine/render/pdf/GlyphFallbackLogger.java‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ public final class GlyphFallbackLogger {
4444
*/
4545
private static final Set<Long> SEEN = ConcurrentHashMap.newKeySet();
4646

47+
/**
48+
* Fonts already reported as lacking the Arabic presentation forms. Separate from
49+
* {@link #SEEN} because this fact is per font, not per code point.
50+
*/
51+
private static final Set<String> DEGRADED_FONTS = ConcurrentHashMap.newKeySet();
52+
4753
/**
4854
* Per-font glyph-coverage memo: a font's PostScript base name to the set of
4955
* code points it can ({@code true}) or cannot ({@code false}) encode.
@@ -110,11 +116,15 @@ public static String sanitize(PDFont font, String text) {
110116
for (int offset = 0; offset < length; ) {
111117
int codePoint = text.codePointAt(offset);
112118
offset += Character.charCount(codePoint);
113-
if (codePoint == '\n' || codePoint == '\r' || TextControlSanitizer.isBidiControl(codePoint)) {
114-
// A bidirectional formatting character steers the layout and draws
115-
// nothing. Substituting it would put a visible '?' on the page and give
116-
// a zero-width character a width, which the wrapping already committed
117-
// to not having. Dropping it keeps measurement and rendering in step.
119+
if (codePoint == '\n' || codePoint == '\r'
120+
|| TextControlSanitizer.isFormattingControl(codePoint)) {
121+
// A formatting character steers the layout and draws nothing —
122+
// bidirectional controls steer the direction, joining controls the
123+
// shaping. The shaper consumes its own, but only when it runs at all,
124+
// so text with no Arabic in it still arrives carrying them.
125+
// Substituting either would put a visible '?' on the page and give a
126+
// zero-width character a width, which the wrapping already committed to
127+
// not having. Dropping it keeps measurement and rendering in step.
118128
continue;
119129
}
120130
if (isEncodable(font, coverage, codePoint)) {
@@ -150,10 +160,14 @@ private static boolean appendBaseLetters(PDFont font,
150160
}
151161
sb.append(base);
152162
String fontName = fontKey(font);
153-
long key = ((long) fontName.hashCode() << 32) | (codePoint & 0xFFFFFFFFL);
154-
if (SEEN.add(key)) {
155-
LOG.warn("glyph.degraded font={} codePoint=U+{} joined form unavailable, "
156-
+ "drawing base letters — Arabic renders unjoined in this font",
163+
// Keyed on the font alone rather than on (font, code point): a font that carries
164+
// no presentation forms carries none of them, so the second form says nothing the
165+
// first did not. Per-form keying turned one fact about the font into a warning for
166+
// every distinct letter the document happened to use.
167+
if (DEGRADED_FONTS.add(fontName)) {
168+
LOG.warn("glyph.degraded font={} joined forms unavailable, drawing base "
169+
+ "letters — Arabic renders unjoined in this font (first at "
170+
+ "U+{})",
157171
fontName, String.format("%04X", codePoint));
158172
}
159173
return true;

‎render-pdf/src/test/java/com/demcha/compose/document/backend/fixed/pdf/RtlGlyphOrderTest.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,4 +245,22 @@ protected void writeString(String text, List<TextPosition> textPositions) {
245245
}
246246
return drawn.toString().trim();
247247
}
248+
249+
@Test
250+
void aZeroWidthNonJoinerWrittenIntoAParagraphReachesTheShaper() throws Exception {
251+
// The end-to-end half of the joining-control guard. The shaper handles U+200C
252+
// correctly on its own, but it only ever saw it once control sanitizing stopped
253+
// deleting it first — a gap no unit test of either piece could show.
254+
String joined = glyphsInDrawingOrder(renderArabic("به", TextDirection.RTL));
255+
String broken = glyphsInDrawingOrder(
256+
renderArabic("ب‌ه", TextDirection.RTL));
257+
258+
assertThat(joined)
259+
.describedAs("beh and heh connect when nothing says otherwise")
260+
.isNotEqualTo(broken);
261+
assertThat(broken)
262+
.describedAs("the author's non-joiner has to survive every seam between "
263+
+ "the builder and the content stream")
264+
.doesNotContain("‌");
265+
}
248266
}

0 commit comments

Comments
 (0)