Skip to content

Commit 5689bc1

Browse files
authored
fix(textkit): fix glyphIndexAt regression (#3369)
* fix(textkit): advanceWidthBetween regression with glyphIndices Adapt advanceWidthBetween to new glyphIndexAt semantics from #3358. The refactored glyphIndexAt uses reverse-loop search instead of direct array lookup, requiring callers to use `end - 1` (as slice.ts was updated). advanceWidthBetween was missed, causing width=0 for characters at run boundaries when scriptItemizer splits CJK text by script (Han/Hiragana/ Katakana). This broke text wrapping for any multi-script text. * add changeset * fix(textkit): handle past-the-end index in glyphIndexAt glyphIndexAt now returns glyphIndices.length when the string index is beyond all glyph mappings, restoring the old fallback behavior. This removes the need for callers to pass end - 1 as a workaround. advanceWidthBetween is reverted to its original calling convention.
1 parent fa06da6 commit 5689bc1

8 files changed

Lines changed: 164 additions & 0 deletions

.changeset/wise-clocks-drop.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@react-pdf/textkit": minor
3+
---
4+
5+
fix(textkit): advanceWidthBetween regression with glyphIndices
2.65 KB
Loading
3.41 KB
Loading

packages/renderer/tests/text.test.jsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import {
1111
} from '@react-pdf/renderer';
1212
import renderToImage from './renderComponent';
1313

14+
Font.register({
15+
family: 'NotoSansJP',
16+
src: 'https://fonts.gstatic.com/s/notosansjp/v52/-F6jfjtqLzI2JPCgQBnw7HFyzSD-AsregP8VFBEj75s.ttf',
17+
});
18+
1419
const styles = StyleSheet.create({
1520
title: {
1621
margin: 20,
@@ -155,4 +160,36 @@ describe('text', () => {
155160

156161
expect(image).toMatchImageSnapshot();
157162
});
163+
164+
test('should wrap CJK text at character boundaries', async () => {
165+
const image = await renderToImage(
166+
<Document>
167+
<Page size={[200, 120]} style={{ padding: 10 }}>
168+
<View style={{ width: 80, padding: 5, border: '1px solid #ccc' }}>
169+
<Text style={{ fontFamily: 'NotoSansJP', fontSize: 12 }}>
170+
本当に長いテキスト
171+
</Text>
172+
</View>
173+
</Page>
174+
</Document>,
175+
);
176+
177+
expect(image).toMatchImageSnapshot();
178+
});
179+
180+
test('should wrap mixed CJK and Latin text', async () => {
181+
const image = await renderToImage(
182+
<Document>
183+
<Page size={[200, 120]} style={{ padding: 10 }}>
184+
<View style={{ width: 100, padding: 5, border: '1px solid #ccc' }}>
185+
<Text style={{ fontFamily: 'NotoSansJP', fontSize: 12 }}>
186+
Hello世界!これはテストです。
187+
</Text>
188+
</View>
189+
</Page>
190+
</Document>,
191+
);
192+
193+
expect(image).toMatchImageSnapshot();
194+
});
158195
});

packages/textkit/src/run/glyphIndexAt.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ const glyphIndexAt = (index: number, run: Run) => {
1515
const glyphIndices = run?.glyphIndices;
1616
if (!glyphIndices) return index;
1717

18+
// If index is past all glyph mappings, return length (one past end)
19+
if (
20+
glyphIndices.length > 0 &&
21+
index > glyphIndices[glyphIndices.length - 1]
22+
) {
23+
return glyphIndices.length;
24+
}
25+
1826
let result = index;
1927

2028
for (let i = glyphIndices.length - 1; i >= 0; i -= 1) {

packages/textkit/tests/attributedString/advanceWidthBetween.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,55 @@ describe('attributeString advanceWidthBetween operator', () => {
228228

229229
expect(advanceWidthBetween(1, 2, string)).toBe(10);
230230
});
231+
232+
test('should return correct per-character width for CJK runs split by script', () => {
233+
// Simulates scriptItemizer splitting "本当にテキスト" into:
234+
// run 0: "本当" (Han)
235+
// run 1: "に" (Hiragana)
236+
// run 2: "テキスト" (Katakana)
237+
const pos = (w: number) => ({
238+
xAdvance: w,
239+
yAdvance: 0,
240+
xOffset: 0,
241+
yOffset: 0,
242+
});
243+
const string = {
244+
string: '本当にテキスト',
245+
runs: [
246+
{
247+
start: 0,
248+
end: 2,
249+
attributes: {},
250+
glyphIndices: [0, 1],
251+
positions: [pos(12), pos(12)],
252+
},
253+
{
254+
start: 2,
255+
end: 3,
256+
attributes: {},
257+
glyphIndices: [0],
258+
positions: [pos(12)],
259+
},
260+
{
261+
start: 3,
262+
end: 7,
263+
attributes: {},
264+
glyphIndices: [0, 1, 2, 3],
265+
positions: [pos(12), pos(11), pos(12), pos(12)],
266+
},
267+
],
268+
};
269+
270+
// Each character should return its own width, not 0
271+
expect(advanceWidthBetween(0, 1, string)).toBe(12); // 本
272+
expect(advanceWidthBetween(1, 2, string)).toBe(12); // 当
273+
expect(advanceWidthBetween(2, 3, string)).toBe(12); // に
274+
expect(advanceWidthBetween(3, 4, string)).toBe(12); // テ
275+
expect(advanceWidthBetween(4, 5, string)).toBe(11); // キ
276+
expect(advanceWidthBetween(5, 6, string)).toBe(12); // ス
277+
expect(advanceWidthBetween(6, 7, string)).toBe(12); // ト
278+
279+
// Cross-run range
280+
expect(advanceWidthBetween(0, 7, string)).toBe(83);
281+
});
231282
});

packages/textkit/tests/run/advanceWidthBetween.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,43 @@ describe('run advanceWidthBetween operator', () => {
129129

130130
expect(advanceWidthBetween(7, 9, run)).toBe(32);
131131
});
132+
133+
test('should return correct width for each character with glyphIndices', () => {
134+
// Simulates a CJK run where scriptItemizer splits by script
135+
// e.g., "本当" as a Han run with 1:1 glyph mapping
136+
const run = {
137+
start: 0,
138+
end: 2,
139+
attributes: {},
140+
glyphIndices: [0, 1],
141+
positions: [
142+
{ xAdvance: 12, yAdvance: 0, xOffset: 0, yOffset: 0 },
143+
{ xAdvance: 12, yAdvance: 0, xOffset: 0, yOffset: 0 },
144+
],
145+
};
146+
147+
expect(advanceWidthBetween(0, 1, run)).toBe(12);
148+
expect(advanceWidthBetween(1, 2, run)).toBe(12);
149+
expect(advanceWidthBetween(0, 2, run)).toBe(24);
150+
});
151+
152+
test('should return correct width for last character with glyphIndices and offset start', () => {
153+
// Run starting at offset, like a Katakana run after Han+Hiragana runs
154+
const run = {
155+
start: 3,
156+
end: 7,
157+
attributes: {},
158+
glyphIndices: [0, 1, 2, 3],
159+
positions: [
160+
{ xAdvance: 12, yAdvance: 0, xOffset: 0, yOffset: 0 },
161+
{ xAdvance: 11, yAdvance: 0, xOffset: 0, yOffset: 0 },
162+
{ xAdvance: 12, yAdvance: 0, xOffset: 0, yOffset: 0 },
163+
{ xAdvance: 12, yAdvance: 0, xOffset: 0, yOffset: 0 },
164+
],
165+
};
166+
167+
expect(advanceWidthBetween(3, 4, run)).toBe(12);
168+
expect(advanceWidthBetween(6, 7, run)).toBe(12);
169+
expect(advanceWidthBetween(3, 7, run)).toBe(47);
170+
});
132171
});

packages/textkit/tests/run/glyphIndexAt.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,28 @@ describe('run glyphIndexAt operator', () => {
8181
expect(glyphIndexAt(0, run)).toBe(0);
8282
expect(glyphIndexAt(1, run)).toBe(2);
8383
});
84+
85+
test('should return length for index past all glyph mappings', () => {
86+
const run = {
87+
start: 0,
88+
end: 2,
89+
attributes: {},
90+
glyphIndices: [0, 1],
91+
};
92+
93+
expect(glyphIndexAt(2, run)).toBe(2);
94+
expect(glyphIndexAt(5, run)).toBe(2);
95+
});
96+
97+
test('should return length for index past ligature mappings', () => {
98+
const run = {
99+
start: 0,
100+
end: 5,
101+
attributes: {},
102+
glyphIndices: [0, 1, 2, 4],
103+
};
104+
105+
expect(glyphIndexAt(5, run)).toBe(4);
106+
expect(glyphIndexAt(10, run)).toBe(4);
107+
});
84108
});

0 commit comments

Comments
 (0)