Skip to content

Commit 995ebba

Browse files
tthallosclaude
andcommitted
refactor(core,renderer): bake group scale at parse time
Resolve OOXML group chOff/chExt → ext scale into each descendant's absolute position and size during parsing, then normalize the group so childOffset=(0,0) and childExtent=size. Rendering becomes a pure pass-through: the group emits only its own translate/rotate/flip, and child elements render at their final stretched dimensions. Matches the model OnlyOffice uses in CGroupShape.getResultScaleCoefficients + Shape.recalculateTransform. Removes the AccumScale plumbing and foreignObject inverse-scale wrappers that were introduced to preserve glyph isotropy under a non-uniform group transform; neither is needed once scale no longer crosses the SVG tree. Verified against slide 10 of the group-test fixture: stretched frames render identically while glyphs stay isotropic, and all 148 unit tests pass. Refs #20, #21 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 40af0b3 commit 995ebba

5 files changed

Lines changed: 88 additions & 115 deletions

File tree

packages/core/src/parser/SlideParser.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,55 @@ function parseGroup(
636636
return group
637637
}
638638

639+
/**
640+
* Resolve a group's non-uniform child-coord scale into each descendant's
641+
* absolute geometry, then normalize the group so the renderer emits no
642+
* inner scale.
643+
*
644+
* OOXML groups map a child coord rect `[chOff, chOff+chExt]` onto the
645+
* outer rect `[off, off+ext]`. When `ext/chExt` isn't 1:1, descendants
646+
* should be *drawn* at the scaled size, but their text glyphs should
647+
* stay isotropic — PowerPoint stretches the frame, not the glyphs.
648+
*
649+
* Propagating the scale through an SVG group transform contaminates
650+
* every foreignObject underneath (text distorts). Instead we bake the
651+
* scale into each child's `position`/`size` here; stroke widths, font
652+
* sizes, and margins are left untouched, matching PowerPoint's
653+
* behavior. Nested groups recurse: their own `childExtent` stays in
654+
* their own child coord space, so re-baking compounds cleanly.
655+
*/
656+
function bakeGroupScale(group: GroupElement): void {
657+
const cx =
658+
group.childExtent.width > 0 ? group.size.width / group.childExtent.width : 1
659+
const cy =
660+
group.childExtent.height > 0 ? group.size.height / group.childExtent.height : 1
661+
const chOffX = group.childOffset.x
662+
const chOffY = group.childOffset.y
663+
664+
const hasTransform = cx !== 1 || cy !== 1 || chOffX !== 0 || chOffY !== 0
665+
666+
if (hasTransform) {
667+
for (const child of group.children) {
668+
child.position = {
669+
x: (child.position.x - chOffX) * cx,
670+
y: (child.position.y - chOffY) * cy,
671+
}
672+
child.size = {
673+
width: child.size.width * cx,
674+
height: child.size.height * cy,
675+
}
676+
}
677+
}
678+
679+
for (const child of group.children) {
680+
if (child.type === 'group') bakeGroupScale(child)
681+
}
682+
683+
// Renderer now sees an identity child space.
684+
group.childOffset = { x: 0, y: 0 }
685+
group.childExtent = { width: group.size.width, height: group.size.height }
686+
}
687+
639688
// ---- GraphicFrame (table or other) ----
640689

641690
function parseGraphicFrame(
@@ -1418,6 +1467,16 @@ export function parseSlide(
14181467
slide.elements = parseSpTreeChildren(spTree, slideCtx, tagIndex.top)
14191468
}
14201469

1470+
// Bake group scales into child geometry. OOXML groups declare a child
1471+
// coord space via chOff/chExt that differs from the group's outer ext,
1472+
// producing a (possibly non-uniform) scale for descendants. We resolve
1473+
// this scale into each child's absolute position/size at parse time
1474+
// (matching OnlyOffice's model) so the renderer doesn't need to carry
1475+
// a non-uniform scale through foreignObject boundaries.
1476+
for (const el of slide.elements) {
1477+
if (el.type === 'group') bakeGroupScale(el)
1478+
}
1479+
14211480
// Parse slide transition
14221481
const transition = parseTransition(sld)
14231482
if (transition) slide.transition = transition

packages/renderer/src/ShapeRenderer.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import type { ShapeElement, LineEnd } from '@pagus-kit/core'
66
import { generateShapePath } from '@pagus-kit/core'
77
import { renderFill } from './FillRenderer'
88
import { renderText } from './TextRenderer'
9-
import type { AccumScale } from './SlideRenderer'
109
import { fmt, ptToPx, dashArray } from './utils/svg'
1110

1211
const LINE_END_SIZE: Record<string, number> = { small: 3, medium: 5, large: 8 }
@@ -91,7 +90,6 @@ export function renderShape(
9190
genId: () => string,
9291
fontSubs?: Record<string, string>,
9392
hiddenParagraphs?: Set<number>,
94-
accumScale?: AccumScale,
9593
): string {
9694
const w = shape.size.width
9795
const h = shape.size.height
@@ -151,7 +149,7 @@ export function renderShape(
151149
// Text (foreignObject) — NOT flipped, stays readable
152150
let textEl = ''
153151
if (shape.textBody && shape.textBody.paragraphs.length > 0) {
154-
textEl = renderText(shape.textBody, shape.size, fontSubs, hiddenParagraphs, accumScale)
152+
textEl = renderText(shape.textBody, shape.size, fontSubs, hiddenParagraphs)
155153
}
156154

157155
if (textEl || isLine) {

packages/renderer/src/SlideRenderer.ts

Lines changed: 10 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -115,29 +115,13 @@ function buildShadowFilter(shadow: Shadow, defs: string[], genId: () => string):
115115

116116
// ---- Element rendering dispatch ----
117117

118-
/**
119-
* Accumulated group scale from the root down to this element.
120-
* When a group's outer `ext` differs from its inner `chExt`, the group applies
121-
* a non-uniform scale to its descendants. Text glyphs should NOT inherit that
122-
* non-uniformity (PowerPoint stretches the frame, not the glyphs), so we
123-
* thread the cumulative scale down and let text-emitting renderers cancel the
124-
* non-uniform component via an inverse transform.
125-
*/
126-
export interface AccumScale {
127-
x: number
128-
y: number
129-
}
130-
131-
const IDENTITY_SCALE: AccumScale = { x: 1, y: 1 }
132-
133118
function renderElement(
134119
el: SlideElement,
135120
defs: string[],
136121
genId: () => string,
137122
slideSize: Size,
138123
fontSubs?: Record<string, string>,
139124
hiddenIds?: string[],
140-
accumScale: AccumScale = IDENTITY_SCALE,
141125
): string {
142126
const transform = buildTransformAttr(el, slideSize.width, slideSize.height)
143127

@@ -160,7 +144,7 @@ function renderElement(
160144

161145
switch (el.type) {
162146
case 'shape':
163-
content = renderShape(el, defs, genId, fontSubs, hiddenParagraphs, accumScale)
147+
content = renderShape(el, defs, genId, fontSubs, hiddenParagraphs)
164148
break
165149
case 'image':
166150
content = renderImage(el, defs, genId)
@@ -169,13 +153,13 @@ function renderElement(
169153
content = renderMedia(el)
170154
break
171155
case 'table':
172-
content = renderTable(el, fontSubs, accumScale)
156+
content = renderTable(el, fontSubs)
173157
break
174158
case 'chart':
175159
content = renderChart(el)
176160
break
177161
case 'group':
178-
content = renderGroup(el, defs, genId, slideSize, fontSubs, hiddenIds, accumScale)
162+
content = renderGroup(el, defs, genId, slideSize, fontSubs, hiddenIds)
179163
break
180164
default:
181165
content = ''
@@ -202,46 +186,15 @@ function renderGroup(
202186
slideSize: Size,
203187
fontSubs?: Record<string, string>,
204188
hiddenIds?: string[],
205-
accumScale: AccumScale = IDENTITY_SCALE,
206189
): string {
207-
// Group applies a coordinate space transformation for its children
208-
// childOffset and childExtent define the source coordinate space
209-
// group's size defines the destination
210-
const scaleX =
211-
group.childExtent.width > 0
212-
? ptToPx(group.size.width) / ptToPx(group.childExtent.width)
213-
: 1
214-
const scaleY =
215-
group.childExtent.height > 0
216-
? ptToPx(group.size.height) / ptToPx(group.childExtent.height)
217-
: 1
218-
219-
const offsetX = fmt(-ptToPx(group.childOffset.x) * scaleX)
220-
const offsetY = fmt(-ptToPx(group.childOffset.y) * scaleY)
221-
222-
const hasScale = scaleX !== 1 || scaleY !== 1
223-
const hasOffset = group.childOffset.x !== 0 || group.childOffset.y !== 0
224-
225-
let innerTransform = ''
226-
if (hasScale) {
227-
innerTransform = `translate(${offsetX},${offsetY}) scale(${fmt(scaleX)},${fmt(scaleY)})`
228-
} else if (hasOffset) {
229-
innerTransform = `translate(${offsetX},${offsetY})`
230-
}
231-
232-
const childAccumScale: AccumScale = hasScale
233-
? { x: accumScale.x * scaleX, y: accumScale.y * scaleY }
234-
: accumScale
235-
236-
const childrenSvg = group.children
237-
.map((child) => renderElement(child, defs, genId, slideSize, fontSubs, hiddenIds, childAccumScale))
190+
// Child coord-space → outer rect scale is resolved at parse time
191+
// (see bakeGroupScale in core). Each child's position/size is already
192+
// expressed in the group's outer coord space, so this is a pure
193+
// container — only the group's own translate/rotate/flip (applied by
194+
// renderElement) are emitted.
195+
return group.children
196+
.map((child) => renderElement(child, defs, genId, slideSize, fontSubs, hiddenIds))
238197
.join('')
239-
240-
if (innerTransform) {
241-
return `<g transform="${innerTransform}">${childrenSvg}</g>`
242-
}
243-
244-
return childrenSvg
245198
}
246199

247200
// ---- Main render function ----

packages/renderer/src/TableRenderer.ts

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import type { TableElement, TableCell, BorderLine } from '@pagus-kit/core'
66
import { fillToCss } from './FillRenderer'
77
import { renderTextInline } from './TextRenderer'
8-
import type { AccumScale } from './SlideRenderer'
98
import { fmt, ptToPx, cssStyle } from './utils/svg'
109

1110
/** Mix a hex color toward white by the given ratio (0 = white, 1 = full color). */
@@ -87,7 +86,7 @@ function renderCell(cell: TableCell, colWidthPx: number, rowHeightPx: number, hi
8786
* @param table - The table element from the IR
8887
* @returns SVG foreignObject string containing an HTML table
8988
*/
90-
export function renderTable(table: TableElement, fontSubs?: Record<string, string>, accumScale?: AccumScale): string {
89+
export function renderTable(table: TableElement, fontSubs?: Record<string, string>): string {
9190
const wpx = ptToPx(table.size.width)
9291
const hpx = ptToPx(table.size.height)
9392

@@ -143,28 +142,14 @@ export function renderTable(table: TableElement, fontSubs?: Record<string, strin
143142
'table-layout': 'fixed',
144143
})
145144

146-
// Non-uniform group scale compensation (see TextRenderer for rationale).
147-
const sx = accumScale?.x ?? 1
148-
const sy = accumScale?.y ?? 1
149-
const needsCompensation = sx !== sy && sx > 0 && sy > 0
150-
const s = needsCompensation ? Math.min(sx, sy) : 1
151-
const invX = needsCompensation ? s / sx : 1
152-
const invY = needsCompensation ? s / sy : 1
153-
const expX = needsCompensation ? sx / s : 1
154-
const expY = needsCompensation ? sy / s : 1
155-
156-
const fo =
157-
`<foreignObject x="0" y="0" width="${fmt(wpx * expX)}" height="${fmt(hpx * expY)}">` +
158-
`<table xmlns="http://www.w3.org/1999/xhtml" style="${tableStyle}">` +
159-
colgroup +
160-
`<tbody>` +
161-
rowsHtml +
162-
`</tbody>` +
163-
`</table>` +
164-
`</foreignObject>`
165-
166-
if (needsCompensation) {
167-
return `<g transform="scale(${fmt(invX)},${fmt(invY)})">${fo}</g>`
168-
}
169-
return fo
145+
return [
146+
`<foreignObject x="0" y="0" width="${fmt(wpx)}" height="${fmt(hpx)}">`,
147+
`<table xmlns="http://www.w3.org/1999/xhtml" style="${tableStyle}">`,
148+
colgroup,
149+
`<tbody>`,
150+
rowsHtml,
151+
`</tbody>`,
152+
`</table>`,
153+
`</foreignObject>`,
154+
].join('')
170155
}

packages/renderer/src/TextRenderer.ts

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
*/
44

55
import type { TextBody, TextRun, Paragraph, Size, ResolvedColor, LineSpacing } from '@pagus-kit/core'
6-
import type { AccumScale } from './SlideRenderer'
76
import { escapeHtml, ptToPx, fmt, cssStyle } from './utils/svg'
87

98
function colorToCss(color: ResolvedColor): string {
@@ -367,7 +366,6 @@ export function renderText(
367366
containerSize: Size,
368367
fontSubs?: Record<string, string>,
369368
hiddenParagraphs?: Set<number>,
370-
accumScale?: AccumScale,
371369
): string {
372370
const { bodyProperties, paragraphs } = textBody
373371
const { anchor, margins } = bodyProperties
@@ -455,33 +453,13 @@ export function renderText(
455453

456454
const shrinkAttr = isShrinkNoScale ? ' data-pagus-shrink-fit="1"' : ''
457455

458-
// Non-uniform group scale compensation.
459-
// PowerPoint stretches the shape frame but keeps text glyphs isotropic. SVG
460-
// propagates a non-uniform parent scale into the foreignObject HTML, which
461-
// would horizontally (or vertically) distort text. To neutralize the
462-
// non-uniform component while preserving the overall size chosen by the
463-
// smaller axis, we wrap the foreignObject in an inverse scale and expand
464-
// its intrinsic dimensions so the post-wrap box still fills the frame.
465-
const sx = accumScale?.x ?? 1
466-
const sy = accumScale?.y ?? 1
467-
const needsCompensation = sx !== sy && sx > 0 && sy > 0
468-
const s = needsCompensation ? Math.min(sx, sy) : 1
469-
const invX = needsCompensation ? s / sx : 1
470-
const invY = needsCompensation ? s / sy : 1
471-
const expX = needsCompensation ? sx / s : 1
472-
const expY = needsCompensation ? sy / s : 1
473-
474-
const fo =
475-
`<foreignObject x="0" y="${fmt(foY * expY)}" width="${fmt(containerWPx * expX)}" height="${fmt(foHeight * expY)}" overflow="visible">` +
476-
`<div xmlns="http://www.w3.org/1999/xhtml"${shrinkAttr} style="${wrapperStyle}">` +
477-
html +
478-
`</div>` +
479-
`</foreignObject>`
480-
481-
if (needsCompensation) {
482-
return `<g transform="scale(${fmt(invX)},${fmt(invY)})">${fo}</g>`
483-
}
484-
return fo
456+
return [
457+
`<foreignObject x="0" y="${fmt(foY)}" width="${fmt(containerWPx)}" height="${fmt(foHeight)}" overflow="visible">`,
458+
`<div xmlns="http://www.w3.org/1999/xhtml"${shrinkAttr} style="${wrapperStyle}">`,
459+
html,
460+
`</div>`,
461+
`</foreignObject>`,
462+
].join('')
485463
}
486464

487465
/**

0 commit comments

Comments
 (0)