diff --git a/devtools/visual-testing/pnpm-lock.yaml b/devtools/visual-testing/pnpm-lock.yaml index ffff9b5947..bd42bdc85f 100644 --- a/devtools/visual-testing/pnpm-lock.yaml +++ b/devtools/visual-testing/pnpm-lock.yaml @@ -2006,13 +2006,13 @@ packages: resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} superdoc@file:../../packages/superdoc/superdoc.tgz: - resolution: {integrity: sha512-H+zXVDvf1OPb6XJtcZbgrGFpyeEv9nlC3XJG9taawK2/0iYrtLQ2xZ3qc5WStZd5kf3AxSa4gKCGB5UgTl3uxQ==, tarball: file:../../packages/superdoc/superdoc.tgz} - version: 1.20.0 + resolution: {integrity: sha512-VSxckZlguegAfx9nYqI4aDDbix9Kw6Qr9aB5GPYlaOBsx3EDtMDHlRJGnkXHNg1QAEUkQtayHMCx54uZSYcDvg==, tarball: file:../../packages/superdoc/superdoc.tgz} + version: 1.23.0 peerDependencies: '@hocuspocus/provider': ^2.13.6 pdfjs-dist: ^5.4.296 y-prosemirror: ^1.3.7 - yjs: 13.6.19 + yjs: ^13.6.19 supports-color@7.2.0: resolution: {integrity: sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==} diff --git a/packages/layout-engine/contracts/src/cell-spacing.ts b/packages/layout-engine/contracts/src/cell-spacing.ts new file mode 100644 index 0000000000..251c46e002 --- /dev/null +++ b/packages/layout-engine/contracts/src/cell-spacing.ts @@ -0,0 +1,25 @@ +import type { CellSpacing } from './index.js'; + +/** 15 twips per pixel (1440 twips/inch ÷ 96 px/inch). */ +const TWIPS_PER_PX = 15; + +/** + * Resolves table cell spacing to pixels (for border-spacing). + * + * Handles number (px) or `{ type, value }`. The editor/DOCX decoder often stores + * value already in pixels, so we use value as px. If value is in twips (raw OOXML), + * type is `'dxa'` and we convert; otherwise value is treated as px. + * + * @param cellSpacing - Cell spacing value from block attrs + * @returns Cell spacing in pixels (always >= 0) + */ +export function getCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { + if (cellSpacing == null) return 0; + if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); + const v = cellSpacing.value; + if (typeof v !== 'number' || !Number.isFinite(v)) return 0; + const t = (cellSpacing.type ?? '').toLowerCase(); + // Editor/store often has value already in px; raw OOXML has twips (dxa). Only convert when value looks like twips (large). + const asPx = t === 'dxa' && v >= 20 ? v / TWIPS_PER_PX : v; + return Math.max(0, asPx); +} diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 352471a3c4..294c4581b5 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -9,6 +9,15 @@ export { OOXML_PCT_DIVISOR, type TableWidthAttr, type TableColumnSpec } from './ export { effectiveTableCellSpacing } from './table-cell-spacing.js'; +// Table column rescaling (moved from layout-engine for cross-stage use) +export { rescaleColumnWidths } from './table-column-rescale.js'; + +// Cell spacing resolution (moved from measuring-dom for cross-stage use) +export { getCellSpacingPx } from './cell-spacing.js'; + +// OOXML z-index normalization (moved from pm-adapter for cross-stage use) +export { normalizeZIndex, coerceRelativeHeight, isPlainObject, OOXML_Z_INDEX_BASE } from './ooxml-z-index.js'; + // Export justify utilities export { shouldApplyJustify, @@ -1975,6 +1984,10 @@ export type { ResolvedTextLineItem, ResolvedDropCapItem, ResolvedListMarkerItem, + ResolvedTableItem, + ResolvedImageItem, + ResolvedDrawingItem, } from './resolved-layout.js'; +export { isResolvedTableItem, isResolvedImageItem, isResolvedDrawingItem } from './resolved-layout.js'; export * as Engines from './engines/index.js'; diff --git a/packages/layout-engine/contracts/src/ooxml-z-index.ts b/packages/layout-engine/contracts/src/ooxml-z-index.ts new file mode 100644 index 0000000000..1f3764a67a --- /dev/null +++ b/packages/layout-engine/contracts/src/ooxml-z-index.ts @@ -0,0 +1,59 @@ +/** + * OOXML z-index normalization utilities. + * + * OOXML stores z-order as large relativeHeight numbers (base ~251658240). + * These helpers convert to small positive CSS z-index values. + */ + +/** Checks whether `value` is a non-null, non-array object. */ +export const isPlainObject = (value: unknown): value is Record => + value !== null && typeof value === 'object' && !Array.isArray(value); + +/** + * Base value for OOXML relativeHeight z-ordering. + * + * @example + * - 251658240 → 0 (base/background) + * - 251658242 → 2 (slightly above base) + * - 251658291 → 51 (further above) + */ +export const OOXML_Z_INDEX_BASE = 251658240; + +/** + * Coerces relativeHeight from OOXML (number or string) to a finite number. + */ +export function coerceRelativeHeight(raw: unknown): number | undefined { + if (typeof raw === 'number' && Number.isFinite(raw)) return raw; + if (typeof raw === 'string' && raw.trim() !== '') { + const n = Number(raw); + if (Number.isFinite(n)) return n; + } + return undefined; +} + +/** + * Normalizes z-index from OOXML relativeHeight value. + * + * OOXML uses large numbers starting around 251658240. To preserve the relative + * stacking order, we subtract the base value to get a small positive number + * suitable for CSS z-index. This ensures elements with close relativeHeight + * values maintain their correct stacking order. + * + * @param originalAttributes - The originalAttributes object from ProseMirror node attrs + * @returns Normalized z-index number or undefined if no relativeHeight + * + * @example + * ```typescript + * normalizeZIndex({ relativeHeight: 251658240 }); // 0 (background) + * normalizeZIndex({ relativeHeight: 251658242 }); // 2 (above background) + * normalizeZIndex({ relativeHeight: 251658291 }); // 51 (further above) + * normalizeZIndex({}); // undefined + * normalizeZIndex(null); // undefined + * ``` + */ +export function normalizeZIndex(originalAttributes: unknown): number | undefined { + if (!isPlainObject(originalAttributes)) return undefined; + const relativeHeight = coerceRelativeHeight(originalAttributes.relativeHeight); + if (relativeHeight === undefined) return undefined; + return Math.max(0, relativeHeight - OOXML_Z_INDEX_BASE); +} diff --git a/packages/layout-engine/contracts/src/resolved-layout.ts b/packages/layout-engine/contracts/src/resolved-layout.ts index 46d13e7391..9170e1e202 100644 --- a/packages/layout-engine/contracts/src/resolved-layout.ts +++ b/packages/layout-engine/contracts/src/resolved-layout.ts @@ -1,4 +1,4 @@ -import type { FlowMode, Fragment, Line } from './index.js'; +import type { DrawingBlock, FlowMode, Fragment, ImageBlock, Line, TableBlock, TableMeasure } from './index.js'; /** A fully resolved layout ready for the next-generation paint pipeline. */ export type ResolvedLayout = { @@ -29,7 +29,12 @@ export type ResolvedPage = { }; /** Union of all resolved paint item kinds. */ -export type ResolvedPaintItem = ResolvedGroupItem | ResolvedFragmentItem; +export type ResolvedPaintItem = + | ResolvedGroupItem + | ResolvedFragmentItem + | ResolvedTableItem + | ResolvedImageItem + | ResolvedDrawingItem; /** A group of nested resolved paint items (for future use). */ export type ResolvedGroupItem = { @@ -139,6 +144,121 @@ export type ResolvedDropCapItem = { height?: number; }; +// ============================================================================ +// Kind-specific resolved items (PR7: table, image, drawing) +// ============================================================================ + +/** + * A resolved table fragment with pre-extracted block/measure data. + * Replaces blockLookup.get() in the table render path. + */ +export type ResolvedTableItem = { + kind: 'fragment'; + /** Discriminant for table fragments. */ + fragmentKind: 'table'; + /** Stable identifier matching fragmentKey() semantics from the painter. */ + id: string; + /** 0-based page index this item belongs to. */ + pageIndex: number; + /** Left position in pixels. */ + x: number; + /** Top position in pixels. */ + y: number; + /** Width in pixels. */ + width: number; + /** Height in pixels (from fragment.height). */ + height: number; + /** Stacking order (tables typically don't have zIndex at fragment level). */ + zIndex?: number; + /** Block ID — written to data-block-id. */ + blockId: string; + /** Index within page.fragments — bridge to legacy rendering. */ + fragmentIndex: number; + /** Pre-extracted TableBlock (replaces blockLookup.get()). */ + block: TableBlock; + /** Pre-extracted TableMeasure (replaces blockLookup.get()). */ + measure: TableMeasure; + /** Pre-computed cell spacing: measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing). */ + cellSpacingPx: number; + /** Pre-computed effective column widths: fragment.columnWidths ?? measure.columnWidths. */ + effectiveColumnWidths: number[]; +}; + +/** + * A resolved image fragment with pre-extracted block data. + * Replaces blockLookup.get() in the image render path. + */ +export type ResolvedImageItem = { + kind: 'fragment'; + /** Discriminant for image fragments. */ + fragmentKind: 'image'; + /** Stable identifier matching fragmentKey() semantics from the painter. */ + id: string; + /** 0-based page index this item belongs to. */ + pageIndex: number; + /** Left position in pixels. */ + x: number; + /** Top position in pixels. */ + y: number; + /** Width in pixels. */ + width: number; + /** Height in pixels. */ + height: number; + /** Stacking order for anchored images. */ + zIndex?: number; + /** Block ID — written to data-block-id. */ + blockId: string; + /** Index within page.fragments — bridge to legacy rendering. */ + fragmentIndex: number; + /** Pre-extracted ImageBlock (replaces blockLookup.get()). */ + block: ImageBlock; +}; + +/** + * A resolved drawing fragment with pre-extracted block data. + * Replaces blockLookup.get() in the drawing render path. + */ +export type ResolvedDrawingItem = { + kind: 'fragment'; + /** Discriminant for drawing fragments. */ + fragmentKind: 'drawing'; + /** Stable identifier matching fragmentKey() semantics from the painter. */ + id: string; + /** 0-based page index this item belongs to. */ + pageIndex: number; + /** Left position in pixels. */ + x: number; + /** Top position in pixels. */ + y: number; + /** Width in pixels. */ + width: number; + /** Height in pixels. */ + height: number; + /** Stacking order for anchored drawings. */ + zIndex?: number; + /** Block ID — written to data-block-id. */ + blockId: string; + /** Index within page.fragments — bridge to legacy rendering. */ + fragmentIndex: number; + /** Pre-extracted DrawingBlock (replaces blockLookup.get()). */ + block: DrawingBlock; +}; + +/** Type guard: checks whether a resolved paint item is a ResolvedTableItem. */ +export function isResolvedTableItem(item: ResolvedPaintItem): item is ResolvedTableItem { + return item.kind === 'fragment' && 'fragmentKind' in item && item.fragmentKind === 'table' && 'measure' in item; +} + +/** Type guard: checks whether a resolved paint item is a ResolvedImageItem. */ +export function isResolvedImageItem(item: ResolvedPaintItem): item is ResolvedImageItem { + return item.kind === 'fragment' && 'fragmentKind' in item && item.fragmentKind === 'image' && 'block' in item; +} + +/** Type guard: checks whether a resolved paint item is a ResolvedDrawingItem. */ +export function isResolvedDrawingItem(item: ResolvedPaintItem): item is ResolvedDrawingItem { + return item.kind === 'fragment' && 'fragmentKind' in item && item.fragmentKind === 'drawing' && 'block' in item; +} + /** Resolved list marker rendering data with pre-computed positioning. */ export type ResolvedListMarkerItem = { /** Marker text content (e.g., "1.", "a)", bullet). */ diff --git a/packages/layout-engine/contracts/src/table-column-rescale.ts b/packages/layout-engine/contracts/src/table-column-rescale.ts new file mode 100644 index 0000000000..8b704334b2 --- /dev/null +++ b/packages/layout-engine/contracts/src/table-column-rescale.ts @@ -0,0 +1,34 @@ +/** + * Proportionally rescales table column widths when the measured total width + * exceeds the available fragment width. + * + * Returns `undefined` when no rescaling is needed (total fits within fragment). + * Each column is guaranteed at least 1px; the last column absorbs rounding drift. + * + * @param measureColumnWidths - Measured widths per column (or undefined) + * @param measureTotalWidth - Sum of measured widths plus borders/spacing + * @param fragmentWidth - Available render width for the table + * @returns Rescaled widths array, or undefined if no scaling needed + */ +export function rescaleColumnWidths( + measureColumnWidths: number[] | undefined, + measureTotalWidth: number, + fragmentWidth: number, +): number[] | undefined { + if ( + !measureColumnWidths || + measureColumnWidths.length === 0 || + measureTotalWidth <= fragmentWidth || + measureTotalWidth <= 0 + ) { + return undefined; + } + const scale = fragmentWidth / measureTotalWidth; + const scaled = measureColumnWidths.map((w) => Math.max(1, Math.round(w * scale))); + const scaledSum = scaled.reduce((a, b) => a + b, 0); + const target = Math.round(fragmentWidth); + if (scaledSum !== target && scaled.length > 0) { + scaled[scaled.length - 1] = Math.max(1, scaled[scaled.length - 1] + (target - scaledSum)); + } + return scaled; +} diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 941232a7f3..3a51aed0ea 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -172,28 +172,9 @@ function resolveTableFrame( * * @returns Rescaled column widths if clamping occurred, undefined otherwise. */ -export function rescaleColumnWidths( - measureColumnWidths: number[] | undefined, - measureTotalWidth: number, - fragmentWidth: number, -): number[] | undefined { - if ( - !measureColumnWidths || - measureColumnWidths.length === 0 || - measureTotalWidth <= fragmentWidth || - measureTotalWidth <= 0 - ) { - return undefined; - } - const scale = fragmentWidth / measureTotalWidth; - const scaled = measureColumnWidths.map((w) => Math.max(1, Math.round(w * scale))); - const scaledSum = scaled.reduce((a, b) => a + b, 0); - const target = Math.round(fragmentWidth); - if (scaledSum !== target && scaled.length > 0) { - scaled[scaled.length - 1] = Math.max(1, scaled[scaled.length - 1] + (target - scaledSum)); - } - return scaled; -} +// Canonical implementation moved to @superdoc/contracts; re-imported for local use and re-exported. +export { rescaleColumnWidths } from '@superdoc/contracts'; +import { rescaleColumnWidths } from '@superdoc/contracts'; const COLUMN_MIN_WIDTH_PX = 25; const COLUMN_MAX_WIDTH_PX = 200; diff --git a/packages/layout-engine/layout-resolved/src/resolveDrawing.ts b/packages/layout-engine/layout-resolved/src/resolveDrawing.ts new file mode 100644 index 0000000000..de0db6741d --- /dev/null +++ b/packages/layout-engine/layout-resolved/src/resolveDrawing.ts @@ -0,0 +1,34 @@ +import type { DrawingFragment, ResolvedDrawingItem } from '@superdoc/contracts'; +import { requireResolvedBlockAndMeasure, type BlockMapEntry } from './resolvedBlockLookup.js'; + +/** Mirrors fragmentKey() for drawing fragments. */ +function resolveDrawingFragmentId(fragment: DrawingFragment): string { + return `drawing:${fragment.blockId}:${fragment.x}:${fragment.y}`; +} + +/** + * Resolves a drawing fragment into a ResolvedDrawingItem with the pre-extracted DrawingBlock. + */ +export function resolveDrawingItem( + fragment: DrawingFragment, + fragmentIndex: number, + pageIndex: number, + blockMap: Map, +): ResolvedDrawingItem { + const { block } = requireResolvedBlockAndMeasure(blockMap, fragment.blockId, 'drawing', 'drawing', 'drawing'); + + return { + kind: 'fragment', + fragmentKind: 'drawing', + id: resolveDrawingFragmentId(fragment), + pageIndex, + x: fragment.x, + y: fragment.y, + width: fragment.width, + height: fragment.height, + zIndex: fragment.isAnchored ? fragment.zIndex : undefined, + blockId: fragment.blockId, + fragmentIndex, + block, + }; +} diff --git a/packages/layout-engine/layout-resolved/src/resolveImage.ts b/packages/layout-engine/layout-resolved/src/resolveImage.ts new file mode 100644 index 0000000000..d1747585f9 --- /dev/null +++ b/packages/layout-engine/layout-resolved/src/resolveImage.ts @@ -0,0 +1,34 @@ +import type { ImageFragment, ResolvedImageItem } from '@superdoc/contracts'; +import { requireResolvedBlockAndMeasure, type BlockMapEntry } from './resolvedBlockLookup.js'; + +/** Mirrors fragmentKey() for image fragments. */ +function resolveImageFragmentId(fragment: ImageFragment): string { + return `image:${fragment.blockId}:${fragment.x}:${fragment.y}`; +} + +/** + * Resolves an image fragment into a ResolvedImageItem with the pre-extracted ImageBlock. + */ +export function resolveImageItem( + fragment: ImageFragment, + fragmentIndex: number, + pageIndex: number, + blockMap: Map, +): ResolvedImageItem { + const { block } = requireResolvedBlockAndMeasure(blockMap, fragment.blockId, 'image', 'image', 'image'); + + return { + kind: 'fragment', + fragmentKind: 'image', + id: resolveImageFragmentId(fragment), + pageIndex, + x: fragment.x, + y: fragment.y, + width: fragment.width, + height: fragment.height, + zIndex: fragment.isAnchored ? fragment.zIndex : undefined, + blockId: fragment.blockId, + fragmentIndex, + block, + }; +} diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts index d1894625ed..1466689c3d 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.test.ts @@ -179,7 +179,7 @@ describe('resolveLayout', () => { expect(result.pages[0].items[0].height).toBe(54); }); - it('resolves an image fragment with height and zIndex', () => { + it('resolves an image fragment with height, zIndex, and pre-extracted block', () => { const imageFragment: ImageFragment = { kind: 'image', blockId: 'img1', @@ -194,11 +194,12 @@ describe('resolveLayout', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [imageFragment] }], }; - const blocks: FlowBlock[] = [{ kind: 'image', id: 'img1', src: 'test.png', width: 300, height: 250 }]; + const imageBlock = { kind: 'image' as const, id: 'img1', src: 'test.png', width: 300, height: 250 }; + const blocks: FlowBlock[] = [imageBlock]; const measures: Measure[] = [{ kind: 'image', width: 300, height: 250 }]; const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); - const item = result.pages[0].items[0]; + const item = result.pages[0].items[0] as import('@superdoc/contracts').ResolvedImageItem; expect(item).toMatchObject({ kind: 'fragment', id: 'image:img1:100:200', @@ -206,9 +207,11 @@ describe('resolveLayout', () => { height: 250, zIndex: 5, }); + // PR7: verify pre-extracted block + expect(item.block).toBe(imageBlock); }); - it('resolves a drawing fragment with zIndex', () => { + it('resolves a drawing fragment with zIndex and pre-extracted block', () => { const drawingFragment: DrawingFragment = { kind: 'drawing', drawingKind: 'vectorShape', @@ -226,15 +229,25 @@ describe('resolveLayout', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [drawingFragment] }], }; + const drawingBlock = { + kind: 'drawing' as const, + id: 'dr1', + drawingKind: 'vectorShape' as const, + geometry: { width: 200, height: 150 }, + }; + const blocks: FlowBlock[] = [drawingBlock as any]; + const measures: Measure[] = [{ kind: 'drawing', width: 200, height: 150 }]; - const result = resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] }); - const item = result.pages[0].items[0]; + const result = resolveLayout({ layout, flowMode: 'paginated', blocks, measures }); + const item = result.pages[0].items[0] as import('@superdoc/contracts').ResolvedDrawingItem; expect(item).toMatchObject({ id: 'drawing:dr1:50:60', fragmentKind: 'drawing', height: 150, zIndex: 3, }); + // PR7: verify pre-extracted block + expect(item.block).toBe(drawingBlock); }); it('omits zIndex for non-anchored drawing fragments even when the fragment carries one', () => { @@ -254,8 +267,20 @@ describe('resolveLayout', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [drawingFragment] }], }; + const drawingBlock = { + kind: 'drawing' as const, + id: 'dr-inline', + drawingKind: 'vectorShape' as const, + geometry: { width: 200, height: 150 }, + }; + const drawingMeasure = { kind: 'drawing' as const, width: 200, height: 150 }; - const result = resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] }); + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [drawingBlock as any], + measures: [drawingMeasure as any], + }); expect(result.pages[0].items[0].zIndex).toBeUndefined(); }); @@ -282,14 +307,186 @@ describe('resolveLayout', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [tableFragment] }], }; + const tableBlock = { kind: 'table' as const, id: 'tbl1', rows: [] }; + const tableMeasure = { kind: 'table' as const, rows: [], columnWidths: [], totalWidth: 0, totalHeight: 0 }; - const result = resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] }); + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [tableBlock as any], + measures: [tableMeasure as any], + }); const item = result.pages[0].items[0]; expect(item.id).toBe('table:tbl1:0:3:0,0,1-2,3,-1'); expect(item.height).toBe(300); expect(item.fragmentKind).toBe('table'); }); + it('resolves a table fragment with pre-extracted block, measure, and computed fields', () => { + const tableBlock = { + kind: 'table' as const, + id: 'tbl1', + rows: [{ cells: [{ content: [] }] }], + attrs: { cellSpacing: { type: 'px' as const, value: 4 } }, + }; + const tableMeasure = { + kind: 'table' as const, + rows: [{ height: 30, cells: [{ width: 200 }] }], + columnWidths: [200, 268], + totalWidth: 468, + totalHeight: 30, + }; + const tableFragment: TableFragment = { + kind: 'table', + blockId: 'tbl1', + fromRow: 0, + toRow: 1, + x: 72, + y: 100, + width: 468, + height: 30, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [tableFragment] }], + }; + + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [tableBlock as any], + measures: [tableMeasure as any], + }); + const item = result.pages[0].items[0] as import('@superdoc/contracts').ResolvedTableItem; + expect(item.fragmentKind).toBe('table'); + expect(item.block).toBe(tableBlock); + expect(item.measure).toBe(tableMeasure); + // cellSpacingPx: measure has no cellSpacingPx, falls back to getCellSpacingPx(block.attrs.cellSpacing) + expect(item.cellSpacingPx).toBe(4); + // effectiveColumnWidths: no fragment.columnWidths, so uses measure.columnWidths + expect(item.effectiveColumnWidths).toEqual([200, 268]); + }); + + it('uses measure.cellSpacingPx when present on the table measure', () => { + const tableBlock = { + kind: 'table' as const, + id: 'tbl2', + rows: [], + attrs: { cellSpacing: { type: 'px' as const, value: 10 } }, + }; + const tableMeasure = { + kind: 'table' as const, + rows: [], + columnWidths: [100], + totalWidth: 100, + totalHeight: 0, + cellSpacingPx: 7, + }; + const tableFragment: TableFragment = { + kind: 'table', + blockId: 'tbl2', + fromRow: 0, + toRow: 0, + x: 0, + y: 0, + width: 100, + height: 0, + }; + const layout: Layout = { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [tableFragment] }] }; + + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [tableBlock as any], + measures: [tableMeasure as any], + }); + const item = result.pages[0].items[0] as import('@superdoc/contracts').ResolvedTableItem; + // measure.cellSpacingPx (7) takes precedence over block.attrs.cellSpacing (10) + expect(item.cellSpacingPx).toBe(7); + }); + + it('uses fragment.columnWidths over measure.columnWidths when present', () => { + const tableBlock = { kind: 'table' as const, id: 'tbl3', rows: [] }; + const tableMeasure = { + kind: 'table' as const, + rows: [], + columnWidths: [200, 300], + totalWidth: 500, + totalHeight: 0, + }; + const rescaledWidths = [160, 240]; + const tableFragment: TableFragment = { + kind: 'table', + blockId: 'tbl3', + fromRow: 0, + toRow: 0, + x: 0, + y: 0, + width: 400, + height: 0, + columnWidths: rescaledWidths, + }; + const layout: Layout = { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments: [tableFragment] }] }; + + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [tableBlock as any], + measures: [tableMeasure as any], + }); + const item = result.pages[0].items[0] as import('@superdoc/contracts').ResolvedTableItem; + expect(item.effectiveColumnWidths).toEqual(rescaledWidths); + }); + + it('throws when a resolved table fragment is missing its block-map entry', () => { + const tableFragment: TableFragment = { + kind: 'table', + blockId: 'missing-table', + fromRow: 0, + toRow: 1, + x: 0, + y: 0, + width: 100, + height: 40, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [tableFragment] }], + }; + + expect(() => resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] })).toThrow( + '[layout-resolved] Missing block/measure entry for table fragment "missing-table".', + ); + }); + + it('throws when a resolved image fragment points at the wrong block kinds', () => { + const imageFragment: ImageFragment = { + kind: 'image', + blockId: 'img-wrong', + x: 10, + y: 20, + width: 100, + height: 80, + }; + const layout: Layout = { + pageSize: { w: 612, h: 792 }, + pages: [{ number: 1, fragments: [imageFragment] }], + }; + const wrongBlock = { kind: 'paragraph' as const, id: 'img-wrong', runs: [] }; + const wrongMeasure = { kind: 'paragraph' as const, lines: [], totalHeight: 0 }; + + expect(() => + resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [wrongBlock as any], + measures: [wrongMeasure as any], + }), + ).toThrow( + '[layout-resolved] Expected image fragment "img-wrong" to resolve to image/image, got paragraph/paragraph.', + ); + }); + it('resolves a list-item fragment with computed height', () => { const listItemFragment: ListItemFragment = { kind: 'list-item', @@ -363,8 +560,15 @@ describe('resolveLayout', () => { pageSize: { w: 612, h: 792 }, pages: [{ number: 1, fragments }], }; + const imageBlock = { kind: 'image' as const, id: 'img1', src: 'ordered.png', width: 100, height: 80 }; + const imageMeasure = { kind: 'image' as const, width: 100, height: 80 }; - const result = resolveLayout({ layout, flowMode: 'paginated', blocks: [], measures: [] }); + const result = resolveLayout({ + layout, + flowMode: 'paginated', + blocks: [imageBlock as any], + measures: [imageMeasure as any], + }); expect(result.pages[0].items.map((i) => i.id)).toEqual(['para:p1:0:1', 'para:p2:0:1', 'image:img1:200:0']); expect(result.pages[0].items[0].fragmentIndex).toBe(0); expect(result.pages[0].items[1].fragmentIndex).toBe(1); diff --git a/packages/layout-engine/layout-resolved/src/resolveLayout.ts b/packages/layout-engine/layout-resolved/src/resolveLayout.ts index 00efebcef9..fd16d0b15d 100644 --- a/packages/layout-engine/layout-resolved/src/resolveLayout.ts +++ b/packages/layout-engine/layout-resolved/src/resolveLayout.ts @@ -6,16 +6,21 @@ import type { Fragment, DrawingFragment, ImageFragment, + TableFragment, Line, ResolvedLayout, ResolvedPage, - ResolvedFragmentItem, + ResolvedPaintItem, ResolvedParagraphContent, ListMeasure, ParagraphBlock, ParagraphMeasure, } from '@superdoc/contracts'; import { resolveParagraphContent } from './resolveParagraph.js'; +import { resolveTableItem } from './resolveTable.js'; +import { resolveImageItem } from './resolveImage.js'; +import { resolveDrawingItem } from './resolveDrawing.js'; +import type { BlockMapEntry } from './resolvedBlockLookup.js'; export type ResolveLayoutInput = { layout: Layout; @@ -24,8 +29,6 @@ export type ResolveLayoutInput = { measures: Measure[]; }; -type BlockMapEntry = { block: FlowBlock; measure: Measure }; - function buildBlockMap(blocks: FlowBlock[], measures: Measure[]): Map { const map = new Map(); for (let i = 0; i < blocks.length; i++) { @@ -124,21 +127,32 @@ function resolveFragmentItem( fragmentIndex: number, pageIndex: number, blockMap: Map, -): ResolvedFragmentItem { - return { - kind: 'fragment', - id: resolveFragmentId(fragment), - pageIndex, - x: fragment.x, - y: fragment.y, - width: fragment.width, - height: computeFragmentHeight(fragment, blockMap), - zIndex: resolveFragmentZIndex(fragment), - fragmentKind: fragment.kind, - blockId: fragment.blockId, - fragmentIndex, - content: resolveParagraphContentIfApplicable(fragment, blockMap), - }; +): ResolvedPaintItem { + // Route to kind-specific resolvers for types that carry extracted block/measure data. + switch (fragment.kind) { + case 'table': + return resolveTableItem(fragment as TableFragment, fragmentIndex, pageIndex, blockMap); + case 'image': + return resolveImageItem(fragment as ImageFragment, fragmentIndex, pageIndex, blockMap); + case 'drawing': + return resolveDrawingItem(fragment as DrawingFragment, fragmentIndex, pageIndex, blockMap); + default: + // para, list-item — existing generic resolution + return { + kind: 'fragment', + id: resolveFragmentId(fragment), + pageIndex, + x: fragment.x, + y: fragment.y, + width: fragment.width, + height: computeFragmentHeight(fragment, blockMap), + zIndex: resolveFragmentZIndex(fragment), + fragmentKind: fragment.kind, + blockId: fragment.blockId, + fragmentIndex, + content: resolveParagraphContentIfApplicable(fragment, blockMap), + }; + } } export function resolveLayout(input: ResolveLayoutInput): ResolvedLayout { diff --git a/packages/layout-engine/layout-resolved/src/resolveTable.ts b/packages/layout-engine/layout-resolved/src/resolveTable.ts new file mode 100644 index 0000000000..f88a692109 --- /dev/null +++ b/packages/layout-engine/layout-resolved/src/resolveTable.ts @@ -0,0 +1,45 @@ +import type { TableFragment, ResolvedTableItem } from '@superdoc/contracts'; +import { getCellSpacingPx } from '@superdoc/contracts'; +import { requireResolvedBlockAndMeasure, type BlockMapEntry } from './resolvedBlockLookup.js'; + +/** Mirrors fragmentKey() for table fragments. */ +function resolveTableFragmentId(fragment: TableFragment): string { + const partialKey = fragment.partialRow + ? `:${fragment.partialRow.fromLineByCell.join(',')}-${fragment.partialRow.toLineByCell.join(',')}` + : ''; + return `table:${fragment.blockId}:${fragment.fromRow}:${fragment.toRow}${partialKey}`; +} + +/** + * Resolves a table fragment into a ResolvedTableItem with pre-extracted block/measure data. + * + * Pre-computes: + * - cellSpacingPx: measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing) + * - effectiveColumnWidths: fragment.columnWidths ?? measure.columnWidths + */ +export function resolveTableItem( + fragment: TableFragment, + fragmentIndex: number, + pageIndex: number, + blockMap: Map, +): ResolvedTableItem { + const { block, measure } = requireResolvedBlockAndMeasure(blockMap, fragment.blockId, 'table', 'table', 'table'); + + return { + kind: 'fragment', + fragmentKind: 'table', + id: resolveTableFragmentId(fragment), + pageIndex, + x: fragment.x, + y: fragment.y, + width: fragment.width, + height: fragment.height, + zIndex: undefined, // tables don't have zIndex at fragment level + blockId: fragment.blockId, + fragmentIndex, + block, + measure, + cellSpacingPx: measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing), + effectiveColumnWidths: fragment.columnWidths ?? measure.columnWidths, + }; +} diff --git a/packages/layout-engine/layout-resolved/src/resolvedBlockLookup.ts b/packages/layout-engine/layout-resolved/src/resolvedBlockLookup.ts new file mode 100644 index 0000000000..d3822279b7 --- /dev/null +++ b/packages/layout-engine/layout-resolved/src/resolvedBlockLookup.ts @@ -0,0 +1,65 @@ +import type { FlowBlock, Measure } from '@superdoc/contracts'; + +export type BlockMapEntry = { block: FlowBlock; measure: Measure }; + +function buildMissingEntryMessage(fragmentKind: string, blockId: string): string { + return `[layout-resolved] Missing block/measure entry for ${fragmentKind} fragment "${blockId}".`; +} + +function buildWrongKindMessage( + fragmentKind: string, + blockId: string, + expectedBlockKind: FlowBlock['kind'], + expectedMeasureKind: Measure['kind'], + actualBlockKind: FlowBlock['kind'], + actualMeasureKind: Measure['kind'], +): string { + return `[layout-resolved] Expected ${fragmentKind} fragment "${blockId}" to resolve to ${expectedBlockKind}/${expectedMeasureKind}, got ${actualBlockKind}/${actualMeasureKind}.`; +} + +/** + * Reads a block-map entry and verifies that both the block and measure kinds + * match what the resolved paint item expects. + * + * We validate here instead of silently fabricating fallback data so failures + * remain visible and diagnosable during the migration to resolved paint items. + */ +export function requireResolvedBlockAndMeasure< + TBlockKind extends FlowBlock['kind'], + TMeasureKind extends Measure['kind'], +>( + blockMap: Map, + blockId: string, + fragmentKind: string, + expectedBlockKind: TBlockKind, + expectedMeasureKind: TMeasureKind, +): { + block: Extract; + measure: Extract; +} { + const entry = blockMap.get(blockId); + if (!entry) { + throw new Error(buildMissingEntryMessage(fragmentKind, blockId)); + } + + const actualBlockKind = entry.block.kind; + const actualMeasureKind = entry.measure.kind; + const kindsMatch = actualBlockKind === expectedBlockKind && actualMeasureKind === expectedMeasureKind; + if (!kindsMatch) { + throw new Error( + buildWrongKindMessage( + fragmentKind, + blockId, + expectedBlockKind, + expectedMeasureKind, + actualBlockKind, + actualMeasureKind, + ), + ); + } + + return { + block: entry.block as Extract, + measure: entry.measure as Extract, + }; +} diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index f03ece9ae6..7735154a33 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -151,22 +151,9 @@ const _PX_PER_PT = 96 / 72; // Reserved for future pt↔px conversions const twipsToPx = (twips: number): number => twips / TWIPS_PER_PX; const pxToTwips = (px: number): number => Math.round(px * TWIPS_PER_PX); -/** - * Resolves table cell spacing to pixels (for border-spacing). - * Handles number (px) or { type, value }. The editor/DOCX decoder often stores value - * already in pixels (twipsToPixels), so we use value as px. If value is in twips (raw OOXML), - * type is 'dxa' and we convert; otherwise value is treated as px. - */ -export function getCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { - if (cellSpacing == null) return 0; - if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); - const v = cellSpacing.value; - if (typeof v !== 'number' || !Number.isFinite(v)) return 0; - const t = (cellSpacing.type ?? '').toLowerCase(); - // Editor/store often has value already in px; raw OOXML has twips (dxa). Only convert when value looks like twips (large). - const asPx = t === 'dxa' && v >= 20 ? twipsToPx(v) : v; - return Math.max(0, asPx); -} +// Canonical implementation moved to @superdoc/contracts; re-imported for local use and re-exported. +export { getCellSpacingPx } from '@superdoc/contracts'; +import { getCellSpacingPx } from '@superdoc/contracts'; /** * Returns the border width in pixels for a table border value (matches painter border-utils logic). diff --git a/packages/layout-engine/painters/dom/package.json b/packages/layout-engine/painters/dom/package.json index c2afee04ca..8c61a76763 100644 --- a/packages/layout-engine/painters/dom/package.json +++ b/packages/layout-engine/painters/dom/package.json @@ -20,13 +20,11 @@ "@superdoc/common": "workspace:*", "@superdoc/contracts": "workspace:*", "@superdoc/font-utils": "workspace:*", - "@superdoc/measuring-dom": "workspace:*", - "@superdoc/layout-engine": "workspace:*", - "@superdoc/pm-adapter": "workspace:*", "@superdoc/preset-geometry": "workspace:*", "@superdoc/url-validation": "workspace:*" }, "devDependencies": { + "@superdoc/layout-engine": "workspace:*", "vitest": "catalog:" } } diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 9d3c743960..7fbd6966b1 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -1,5 +1,6 @@ -import { describe, expect, it, beforeEach, afterEach } from 'vitest'; +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest'; import { createDomPainter, sanitizeUrl, linkMetrics, applyRunDataAttributes } from './index.js'; +import { DomPainter } from './renderer.js'; import { resolveListMarkerGeometry } from '../../../../../shared/common/list-marker-utils.js'; import type { FlowBlock, @@ -1216,6 +1217,146 @@ describe('DomPainter', () => { expect(lines[1].style.wordSpacing).toBe(''); }); + it('renders an error placeholder when a legacy table fragment is missing its lookup entry', () => { + const missingTableLayout: Layout = { + pageSize: { w: 300, h: 300 }, + pages: [ + { + number: 1, + fragments: [ + { + kind: 'table', + blockId: 'missing-table', + x: 0, + y: 0, + width: 200, + height: 30, + fromRow: 0, + toRow: 1, + }, + ], + }, + ], + }; + + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => { + // Intentionally empty - suppress expected error logging during this regression test. + }); + + const painter = createDomPainter({ blocks: [], measures: [] }); + expect(() => painter.paint(missingTableLayout, mount)).not.toThrow(); + + const placeholder = mount.querySelector('.render-error-placeholder') as HTMLElement | null; + expect(placeholder).toBeTruthy(); + expect(placeholder?.textContent).toContain('[Render Error: missing-table]'); + expect(consoleErrorSpy).toHaveBeenCalled(); + + consoleErrorSpy.mockRestore(); + }); + + it('renders an error placeholder when table-cell line rendering throws', () => { + const renderLineError = new Error('renderLine forced error'); + const tableBlock: TableBlock = { + kind: 'table', + id: 'table-err', + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0', + blocks: [ + { + kind: 'paragraph', + id: 'cell-para-err', + runs: [{ text: 'Cell text', fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 10 }], + }, + ], + attrs: {}, + }, + ], + }, + ], + }; + const tableMeasure: TableMeasure = { + kind: 'table', + rows: [ + { + height: 24, + cells: [ + { + width: 120, + height: 24, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 9, + width: 60, + ascent: 10, + descent: 4, + lineHeight: 16, + }, + ], + totalHeight: 16, + }, + ], + }, + ], + }, + ], + columnWidths: [120], + totalWidth: 120, + totalHeight: 24, + }; + const tableLayout: Layout = { + pageSize: { w: 400, h: 500 }, + pages: [ + { + number: 1, + fragments: [ + { + kind: 'table', + blockId: 'table-err', + fromRow: 0, + toRow: 1, + x: 0, + y: 0, + width: 120, + height: 24, + }, + ], + }, + ], + }; + + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => { + // Intentionally empty - suppress expected error logging during this regression test. + }); + const renderLineSpy = vi.spyOn(DomPainter.prototype as any, 'renderLine').mockImplementation(() => { + throw renderLineError; + }); + + try { + const painter = createDomPainter({ blocks: [tableBlock], measures: [tableMeasure] }); + expect(() => painter.paint(tableLayout, mount)).not.toThrow(); + + const placeholder = mount.querySelector('.render-error-placeholder') as HTMLElement | null; + expect(placeholder).toBeTruthy(); + expect(placeholder?.textContent).toContain('[Render Error: table-err]'); + expect(placeholder?.title).toBe('renderLine forced error'); + expect(consoleErrorSpy).toHaveBeenCalled(); + } finally { + renderLineSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + } + }); + it('applies negative word-spacing for compressed justify lines', () => { // When the measurer allows small overflow for justified text, the painter applies // negative word-spacing so the line still fits the available width. diff --git a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts b/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts index 50539a8ec8..b08c4164f0 100644 --- a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts @@ -9,9 +9,8 @@ * here with a resolution target — not fixed in production code. */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { createDomPainter } from './index.js'; -import { DomPainter } from './renderer.js'; import type { FlowBlock, Measure, Layout, Line } from '@superdoc/contracts'; import { normalizeLines } from './test-utils/normalize-line.js'; @@ -246,10 +245,6 @@ function renderAndNormalize(fixtures: { blocks: FlowBlock[]; measures: Measure[] // --------------------------------------------------------------------------- describe('known divergences (frozen — delete when resolved)', () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - // ----------------------------------------------------------------------- // Resolution target: PR 7 (collapse list-item parallel paint model) // ----------------------------------------------------------------------- @@ -319,143 +314,4 @@ describe('known divergences (frozen — delete when resolved)', () => { expect(listLines[0]!.wordSpacing).toBe(''); }); }); - - // ----------------------------------------------------------------------- - // Resolution target: PR 6 (migrate table-cell to shared flow) - // ----------------------------------------------------------------------- - - describe('table render errors propagate uncaught — Resolution target: PR 6 (migrate table-cell to shared flow)', () => { - // The divergence: renderParagraphFragment has try/catch INSIDE its body, - // but the table rendering chain (renderer.ts:renderTableFragment → renderTableFragmentElement - // → renderTableCell → renderLine callback) does NOT have a catch wrapper. - // - // To test this properly, we spy on renderLine (called by both paths) and - // force it to throw. The paragraph path's try/catch catches it; the table - // path lets it propagate. - - const renderLineError = new Error('renderLine forced error'); - - it('table path: renderLine error propagates as thrown exception', () => { - vi.spyOn(DomPainter.prototype as any, 'renderLine').mockImplementation(() => { - throw renderLineError; - }); - - const block: FlowBlock = { - kind: 'table', - id: 'table-err', - rows: [ - { - id: 'row-0', - cells: [ - { - id: 'cell-0', - blocks: [ - { - kind: 'paragraph', - id: 'p-err', - runs: [{ text: 'Cell text', fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 10 }], - }, - ], - attrs: {}, - }, - ], - }, - ], - }; - const measure: Measure = { - kind: 'table', - rows: [ - { - height: 24, - cells: [ - { - width: 120, - height: 24, - gridColumnStart: 0, - blocks: [ - { - kind: 'paragraph', - lines: [ - { - fromRun: 0, - fromChar: 0, - toRun: 0, - toChar: 9, - width: 60, - ascent: 10, - descent: 4, - lineHeight: 16, - }, - ], - totalHeight: 16, - }, - ], - }, - ], - }, - ], - columnWidths: [120], - totalWidth: 120, - totalHeight: 24, - }; - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [ - { - number: 1, - fragments: [ - { kind: 'table', blockId: 'table-err', fromRow: 0, toRow: 1, x: 0, y: 0, width: 120, height: 24 }, - ], - }, - ], - }; - - const container = document.createElement('div'); - const painter = createDomPainter({ blocks: [block], measures: [measure] }); - expect(() => painter.paint(layout, container)).toThrow('renderLine forced error'); - }); - - it('body paragraph path: renderLine error is caught and does not propagate', () => { - vi.spyOn(DomPainter.prototype as any, 'renderLine').mockImplementation(() => { - throw renderLineError; - }); - - const block: FlowBlock = { - kind: 'paragraph', - id: 'para-err', - runs: [{ text: 'Test text', fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 10 }], - }; - const measure: Measure = { - kind: 'paragraph', - lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 9, width: 60, ascent: 10, descent: 4, lineHeight: 16 }], - totalHeight: 16, - }; - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'para', - blockId: 'para-err', - fromLine: 0, - toLine: 1, - x: 0, - y: 0, - width: 300, - pmStart: 1, - pmEnd: 10, - }, - ], - }, - ], - }; - - const container = document.createElement('div'); - const painter = createDomPainter({ blocks: [block], measures: [measure] }); - // Body path: try/catch in renderParagraphFragment catches the error - expect(() => painter.paint(layout, container)).not.toThrow(); - }); - }); }); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index c385563c6d..b141a3b30d 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -41,6 +41,7 @@ import type { TableBlock, TableCellAttrs, TableFragment, + TableMeasure, MathRun, TextRun, TrackedChangeKind, @@ -50,8 +51,18 @@ import type { ResolvedLayout, ResolvedFragmentItem, ResolvedPage, + ResolvedPaintItem, + ResolvedTableItem, + ResolvedImageItem, + ResolvedDrawingItem, +} from '@superdoc/contracts'; +import { + calculateJustifySpacing, + computeLinePmRange, + getCellSpacingPx, + shouldApplyJustify, + SPACE_CHARS, } from '@superdoc/contracts'; -import { calculateJustifySpacing, computeLinePmRange, shouldApplyJustify, SPACE_CHARS } from '@superdoc/contracts'; import { toCssFontFamily } from '@superdoc/font-utils'; import { getPresetShapeSvg } from '@superdoc/preset-geometry'; import { encodeTooltip, sanitizeHref } from '@superdoc/url-validation'; @@ -1222,7 +1233,7 @@ export class DomPainter { } /** Returns the resolved fragment item for a given page/fragment index, or undefined. */ - private getResolvedFragmentItem(pageIndex: number, fragmentIndex: number): ResolvedFragmentItem | undefined { + private getResolvedFragmentItem(pageIndex: number, fragmentIndex: number): ResolvedPaintItem | undefined { const page = this.getResolvedPage(pageIndex); if (!page) return undefined; const item = page.items[fragmentIndex]; @@ -2667,22 +2678,34 @@ export class DomPainter { context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, betweenInfo?: BetweenBorderInfo, - resolvedItem?: ResolvedFragmentItem, + resolvedItem?: ResolvedPaintItem, ): HTMLElement { if (fragment.kind === 'para') { - return this.renderParagraphFragment(fragment, context, sdtBoundary, betweenInfo, resolvedItem); + return this.renderParagraphFragment( + fragment, + context, + sdtBoundary, + betweenInfo, + resolvedItem as ResolvedFragmentItem | undefined, + ); } if (fragment.kind === 'list-item') { - return this.renderListItemFragment(fragment, context, sdtBoundary, betweenInfo, resolvedItem); + return this.renderListItemFragment( + fragment, + context, + sdtBoundary, + betweenInfo, + resolvedItem as ResolvedFragmentItem | undefined, + ); } if (fragment.kind === 'image') { - return this.renderImageFragment(fragment, context, resolvedItem); + return this.renderImageFragment(fragment, context, resolvedItem as ResolvedImageItem | undefined); } if (fragment.kind === 'drawing') { - return this.renderDrawingFragment(fragment, context, resolvedItem); + return this.renderDrawingFragment(fragment, context, resolvedItem as ResolvedDrawingItem | undefined); } if (fragment.kind === 'table') { - return this.renderTableFragment(fragment, context, sdtBoundary, resolvedItem); + return this.renderTableFragment(fragment, context, sdtBoundary, resolvedItem as ResolvedTableItem | undefined); } throw new Error(`DomPainter: unsupported fragment kind ${(fragment as Fragment).kind}`); } @@ -3371,20 +3394,25 @@ export class DomPainter { private renderImageFragment( fragment: ImageFragment, context: FragmentRenderContext, - resolvedItem?: ResolvedFragmentItem, + resolvedItem?: ResolvedImageItem, ): HTMLElement { try { - const lookup = this.blockLookup.get(fragment.blockId); - if (!lookup || lookup.block.kind !== 'image' || lookup.measure.kind !== 'image') { - throw new Error(`DomPainter: missing image block for fragment ${fragment.blockId}`); - } + // Use pre-extracted block from resolved item; fall back to blockLookup when resolved item + // is a legacy ResolvedFragmentItem without the block field. + const block: ImageBlock = + resolvedItem?.block ?? + (() => { + const lookup = this.blockLookup.get(fragment.blockId); + if (!lookup || lookup.block.kind !== 'image' || lookup.measure.kind !== 'image') { + throw new Error(`DomPainter: missing image block for fragment ${fragment.blockId}`); + } + return lookup.block as ImageBlock; + })(); if (!this.doc) { throw new Error('DomPainter: document is not available'); } - const block = lookup.block as ImageBlock; - const fragmentEl = this.doc.createElement('div'); fragmentEl.classList.add(CLASS_NAMES.fragment, DOM_CLASS_NAMES.IMAGE_FRAGMENT); applyStyles(fragmentEl, fragmentStyles); @@ -3485,18 +3513,23 @@ export class DomPainter { private renderDrawingFragment( fragment: DrawingFragment, context: FragmentRenderContext, - resolvedItem?: ResolvedFragmentItem, + resolvedItem?: ResolvedDrawingItem, ): HTMLElement { try { - const lookup = this.blockLookup.get(fragment.blockId); - if (!lookup || lookup.block.kind !== 'drawing' || lookup.measure.kind !== 'drawing') { - throw new Error(`DomPainter: missing drawing block for fragment ${fragment.blockId}`); - } + // Use pre-extracted block from resolved item; fall back to blockLookup when resolved item + // is a legacy ResolvedFragmentItem without the block field. + const block: DrawingBlock = + resolvedItem?.block ?? + (() => { + const lookup = this.blockLookup.get(fragment.blockId); + if (!lookup || lookup.block.kind !== 'drawing' || lookup.measure.kind !== 'drawing') { + throw new Error(`DomPainter: missing drawing block for fragment ${fragment.blockId}`); + } + return lookup.block as DrawingBlock; + })(); if (!this.doc) { throw new Error('DomPainter: document is not available'); } - - const block = lookup.block as DrawingBlock; const isVectorShapeBlock = block.kind === 'drawing' && block.drawingKind === 'vectorShape'; const fragmentEl = this.doc.createElement('div'); @@ -4319,107 +4352,131 @@ export class DomPainter { return renderChartToElement(this.doc!, block.chartData, block.geometry); } + private resolveTableRenderData( + fragment: TableFragment, + resolvedItem?: ResolvedTableItem, + ): { + block: TableBlock; + measure: TableMeasure; + cellSpacingPx: number; + effectiveColumnWidths: number[]; + } { + if (resolvedItem) { + return { + block: resolvedItem.block, + measure: resolvedItem.measure, + cellSpacingPx: resolvedItem.cellSpacingPx, + effectiveColumnWidths: resolvedItem.effectiveColumnWidths, + }; + } + + const lookup = this.blockLookup.get(fragment.blockId); + if (!lookup || lookup.block.kind !== 'table' || lookup.measure.kind !== 'table') { + throw new Error(`DomPainter: missing table block for fragment ${fragment.blockId}`); + } + + const block = lookup.block as TableBlock; + const measure = lookup.measure as TableMeasure; + + return { + block, + measure, + cellSpacingPx: measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing), + effectiveColumnWidths: fragment.columnWidths ?? measure.columnWidths, + }; + } + private renderTableFragment( fragment: TableFragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, - resolvedItem?: ResolvedFragmentItem, + resolvedItem?: ResolvedTableItem, ): HTMLElement { - if (!this.doc) { - throw new Error('DomPainter: document is not available'); - } + try { + if (!this.doc) { + throw new Error('DomPainter: document is not available'); + } - // Wrap applyFragmentFrame to capture section from context - // This ensures table cell fragments receive proper section context for PM position validation - const applyFragmentFrameWithSection = (el: HTMLElement, frag: Fragment): void => { - // Table cell inner fragments always use legacy applyFragmentFrame (they are not resolved yet) - this.applyFragmentFrame(el, frag, context.section); - }; + // Wrap applyFragmentFrame to capture section from context. + // Table cell inner fragments always stay on the legacy frame path for now. + const applyFragmentFrameWithSection = (el: HTMLElement, frag: Fragment): void => { + this.applyFragmentFrame(el, frag, context.section); + }; - // Create a wrapper for renderLine that applies Word's justification rules for table cells. - // Word DOES justify text inside table cells, but skips justification on the last line - // (unless the paragraph ends with a line break, which shifts the "last line" down). - const renderLineForTableCell = ( - block: ParagraphBlock, - line: Line, - ctx: FragmentRenderContext, - lineIndex: number, - isLastLine: boolean, - resolvedListTextStartPx?: number, - ): HTMLElement => { - // Check if paragraph ends with a line break - const lastRun = block.runs.length > 0 ? block.runs[block.runs.length - 1] : null; - const paragraphEndsWithLineBreak = lastRun?.kind === 'lineBreak'; - - // Skip justify only on the last line, unless the paragraph ends with a line break - const shouldSkipJustify = isLastLine && !paragraphEndsWithLineBreak; - - return this.renderLine(block, line, ctx, undefined, lineIndex, shouldSkipJustify, resolvedListTextStartPx); - }; + // Word justifies text inside table cells, but not the final line unless the + // paragraph ends with an explicit line break. + const renderLineForTableCell = ( + block: ParagraphBlock, + line: Line, + ctx: FragmentRenderContext, + lineIndex: number, + isLastLine: boolean, + resolvedListTextStartPx?: number, + ): HTMLElement => { + const lastRun = block.runs.length > 0 ? block.runs[block.runs.length - 1] : null; + const paragraphEndsWithLineBreak = lastRun?.kind === 'lineBreak'; + const shouldSkipJustify = isLastLine && !paragraphEndsWithLineBreak; - /** - * Wrapper function for rendering drawing content inside table cells. - * - * This function delegates to the appropriate DomPainter methods based on the drawing kind: - * - 'image': Creates a standard image element with src and object-fit - * - 'shapeGroup': Creates a group container with positioned child shapes (images and vectors) - * - 'vectorShape': Creates an SVG element for the vector shape (without geometry transforms in table cells) - * - * For unsupported or unrecognized drawing kinds, returns a placeholder element with diagonal stripes. - * - * @param block - The DrawingBlock to render - * @returns HTMLElement representing the rendered drawing content - * - * @remarks - * This wrapper is specifically designed for table cell rendering where: - * - Vector shapes are rendered without geometry transforms (to avoid layout conflicts) - * - The returned element will have width: 100% and height: 100% applied by the table cell renderer - */ - const renderDrawingContentForTableCell = (block: DrawingBlock): HTMLElement => { - if (block.drawingKind === 'image') { - return this.createDrawingImageElement(block); - } - if (block.drawingKind === 'shapeGroup') { - return this.createShapeGroupElement(block, context); - } - if (block.drawingKind === 'vectorShape') { - // For vectorShapes in table cells, render without geometry transforms - return this.createVectorShapeElement(block, block.geometry, false, 1, 1, context); - } - if (block.drawingKind === 'chart') { - return this.createChartElement(block); - } - return this.createDrawingPlaceholder(); - }; + return this.renderLine(block, line, ctx, undefined, lineIndex, shouldSkipJustify, resolvedListTextStartPx); + }; - const el = renderTableFragmentElement({ - doc: this.doc, - fragment, - context, - blockLookup: this.blockLookup, - sdtBoundary, - renderLine: renderLineForTableCell, - captureLineSnapshot: (lineEl, lineContext, options) => { - this.capturePaintSnapshotLine(lineEl, lineContext, { - inTableFragment: true, - inTableParagraph: options?.inTableParagraph ?? false, - wrapperEl: options?.wrapperEl, - }); - }, - renderDrawingContent: renderDrawingContentForTableCell, - applyFragmentFrame: applyFragmentFrameWithSection, - applySdtDataset: this.applySdtDataset.bind(this), - applyContainerSdtDataset: this.applyContainerSdtDataset.bind(this), - applyStyles, - }); + /** + * Renders drawing content that lives inside a table cell. + * Table-cell vector shapes intentionally skip outer geometry transforms. + */ + const renderDrawingContentForTableCell = (block: DrawingBlock): HTMLElement => { + if (block.drawingKind === 'image') { + return this.createDrawingImageElement(block); + } + if (block.drawingKind === 'shapeGroup') { + return this.createShapeGroupElement(block, context); + } + if (block.drawingKind === 'vectorShape') { + return this.createVectorShapeElement(block, block.geometry, false, 1, 1, context); + } + if (block.drawingKind === 'chart') { + return this.createChartElement(block); + } + return this.createDrawingPlaceholder(); + }; - // Override outer wrapper positioning with resolved data when available. - // Inner cell fragments still use legacy applyFragmentFrame via deps closure. - if (resolvedItem) { - this.applyResolvedFragmentFrame(el, resolvedItem, fragment, context.section); - } + const tableRenderData = this.resolveTableRenderData(fragment, resolvedItem); - return el; + const el = renderTableFragmentElement({ + doc: this.doc, + fragment, + context, + block: tableRenderData.block, + measure: tableRenderData.measure, + cellSpacingPx: tableRenderData.cellSpacingPx, + effectiveColumnWidths: tableRenderData.effectiveColumnWidths, + sdtBoundary, + renderLine: renderLineForTableCell, + captureLineSnapshot: (lineEl, lineContext, options) => { + this.capturePaintSnapshotLine(lineEl, lineContext, { + inTableFragment: true, + inTableParagraph: options?.inTableParagraph ?? false, + wrapperEl: options?.wrapperEl, + }); + }, + renderDrawingContent: renderDrawingContentForTableCell, + applyFragmentFrame: applyFragmentFrameWithSection, + applySdtDataset: this.applySdtDataset.bind(this), + applyContainerSdtDataset: this.applyContainerSdtDataset.bind(this), + applyStyles, + }); + + // Override outer wrapper positioning with resolved data when available. + // Inner cell fragments still use legacy applyFragmentFrame via deps closure. + if (resolvedItem) { + this.applyResolvedFragmentFrame(el, resolvedItem, fragment, context.section); + } + + return el; + } catch (error) { + console.error('[DomPainter] Table fragment rendering failed:', { fragment, error }); + return this.createErrorPlaceholder(fragment.blockId, error); + } } /** @@ -6166,15 +6223,18 @@ export class DomPainter { el: HTMLElement, fragment: Fragment, section?: 'body' | 'header' | 'footer', - resolvedItem?: ResolvedFragmentItem, + resolvedItem?: ResolvedPaintItem, ): void { - if (fragment.kind === 'list-item' && resolvedItem) { - this.applyResolvedListItemWrapperFrame(el, fragment, resolvedItem, section); + // Narrow to fragment-kind resolved items (excludes ResolvedGroupItem) + const fragmentItem = resolvedItem?.kind === 'fragment' ? resolvedItem : undefined; + + if (fragment.kind === 'list-item' && fragmentItem) { + this.applyResolvedListItemWrapperFrame(el, fragment, fragmentItem as ResolvedFragmentItem, section); return; } - if (resolvedItem) { - this.applyResolvedFragmentFrame(el, resolvedItem, fragment, section); + if (fragmentItem) { + this.applyResolvedFragmentFrame(el, fragmentItem, fragment, section); } else { this.applyFragmentFrame(el, fragment, section); if (fragment.kind === 'image' || fragment.kind === 'drawing') { @@ -6303,7 +6363,7 @@ export class DomPainter { private applyResolvedFragmentFrame( el: HTMLElement, - item: ResolvedFragmentItem, + item: ResolvedFragmentItem | ResolvedTableItem | ResolvedImageItem | ResolvedDrawingItem, fragment: Fragment, section?: 'body' | 'header' | 'footer', ): void { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index 2a2399a9a1..a69033491d 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -18,15 +18,12 @@ import type { WrapExclusion, WrapTextMode, } from '@superdoc/contracts'; -import { effectiveTableCellSpacing } from '@superdoc/contracts'; +import { effectiveTableCellSpacing, rescaleColumnWidths, normalizeZIndex, getCellSpacingPx } from '@superdoc/contracts'; import { toCssFontFamily } from '@superdoc/font-utils'; -import { rescaleColumnWidths } from '@superdoc/layout-engine'; -import { normalizeZIndex } from '@superdoc/pm-adapter/utilities.js'; import type { FragmentRenderContext } from '../renderer.js'; import { applyParagraphBorderStyles, applyParagraphShadingStyles, - type BlockLookup, } from '../features/paragraph-borders/index.js'; import { applySquareWrapExclusionsToLines } from '../utils/anchor-helpers'; import { applyImageClipPath } from '../utils/image-clip-path.js'; @@ -575,7 +572,6 @@ type EmbeddedTableRenderParams = { * Version identifier for embedded table block lookups. * Used to distinguish nested tables from top-level tables in the block lookup map. */ -const EMBEDDED_TABLE_VERSION = 'embedded-table'; /** * Renders a nested table that appears inside a table cell. @@ -639,16 +635,9 @@ const renderEmbeddedTable = (params: EmbeddedTableRenderParams): HTMLElement => columnWidths, partialRow: paramPartialRow, }; - const blockLookup: BlockLookup = new Map([ - [ - table.id, - { - block: table, - measure, - version: EMBEDDED_TABLE_VERSION, - }, - ], - ]); + const effectiveColumnWidths = columnWidths ?? measure.columnWidths; + const embeddedCellSpacingPx = measure.cellSpacingPx ?? getCellSpacingPx(table.attrs?.cellSpacing); + const applyFragmentFrame = (el: HTMLElement, frag: Fragment): void => { el.style.left = `${frag.x}px`; el.style.top = `${frag.y}px`; @@ -660,7 +649,10 @@ const renderEmbeddedTable = (params: EmbeddedTableRenderParams): HTMLElement => doc, fragment, context, - blockLookup, + block: table, + measure, + cellSpacingPx: embeddedCellSpacingPx, + effectiveColumnWidths, renderLine, captureLineSnapshot, renderDrawingContent, diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts index afef2ddefe..ef5933de1d 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts @@ -13,7 +13,7 @@ import type { TableRowBoundary, ParagraphBlock, } from '@superdoc/contracts'; -import type { BlockLookup, FragmentRenderContext } from '../renderer.js'; +import type { FragmentRenderContext } from '../renderer.js'; /** * Create a minimal table block for testing @@ -92,12 +92,10 @@ function createTestTableFragment( describe('renderTableFragment', () => { let doc: Document; - let blockLookup: BlockLookup; let context: FragmentRenderContext; beforeEach(() => { doc = document.implementation.createHTMLDocument('test'); - blockLookup = new Map(); context = { sectionIndex: 0, pageIndex: 0, @@ -213,13 +211,14 @@ describe('renderTableFragment', () => { height: 40, }; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -245,13 +244,14 @@ describe('renderTableFragment', () => { const columnBoundaries: TableColumnBoundary[] = [{ index: 0, x: 0, width: 100, minWidth: 25, resizable: true }]; const fragment = createTestTableFragment(columnBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -286,13 +286,14 @@ describe('renderTableFragment', () => { const rowBoundaries: TableRowBoundary[] = [{ index: 0, y: 0, height: 20, minHeight: 10, resizable: true }]; const fragment = createTestTableFragment(columnBoundaries, rowBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -338,13 +339,14 @@ describe('renderTableFragment', () => { const rowBoundaries: TableRowBoundary[] = [{ index: 0, y: 3, height: 20, minHeight: 10, resizable: false }]; const fragment = createTestTableFragment(columnBoundaries, rowBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -380,13 +382,14 @@ describe('renderTableFragment', () => { ]; const fragment = createTestTableFragment(columnBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -414,13 +417,14 @@ describe('renderTableFragment', () => { const measure = createTestTableMeasure(); const fragment = createTestTableFragment(); // No metadata - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -444,13 +448,14 @@ describe('renderTableFragment', () => { const measure = createTestTableMeasure(); const fragment = createTestTableFragment([]); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -479,13 +484,14 @@ describe('renderTableFragment', () => { ]; const fragment = createTestTableFragment(columnBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -512,13 +518,14 @@ describe('renderTableFragment', () => { const measure = createTestTableMeasure(); const fragment = createTestTableFragment(); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -539,13 +546,14 @@ describe('renderTableFragment', () => { const measure = createTestTableMeasure(); const fragment = createTestTableFragment(); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -563,92 +571,11 @@ describe('renderTableFragment', () => { }); describe('error handling', () => { - it('should return placeholder when block not found', () => { - const fragment = createTestTableFragment(); - // Don't add block to lookup - - // Spy on console.error to verify logging - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => { - // Intentionally empty - suppress console output during tests - }); - - const element = renderTableFragment({ - doc, - fragment, - context, - blockLookup, - renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), - applyFragmentFrame: () => { - // Intentionally empty for test mock - }, - applySdtDataset: () => { - // Intentionally empty for test mock - }, - applyStyles: () => { - // Intentionally empty for test mock - }, - }); - - expect(element.classList.contains('superdoc-error-placeholder')).toBe(true); - expect(element.textContent).toContain('Table rendering error'); - - // Verify error was logged - expect(consoleErrorSpy).toHaveBeenCalled(); - expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('DomPainter: missing table block'), - expect.objectContaining({ - blockId: 'test-table-1', - }), - ); - - consoleErrorSpy.mockRestore(); - }); - - it('should return placeholder when block is wrong kind', () => { - const fragment = createTestTableFragment(); - const wrongBlock: ParagraphBlock = { - kind: 'paragraph', - id: 'test-table-1' as BlockId, - runs: [], - }; - const wrongMeasure = { - kind: 'paragraph' as const, - lines: [], - totalHeight: 0, - }; - - blockLookup.set(fragment.blockId, { - block: wrongBlock as unknown as TableBlock, - measure: wrongMeasure as unknown as TableMeasure, - }); - - const element = renderTableFragment({ - doc, - fragment, - context, - blockLookup, - renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), - applyFragmentFrame: () => { - // Intentionally empty for test mock - }, - applySdtDataset: () => { - // Intentionally empty for test mock - }, - applyStyles: () => { - // Intentionally empty for test mock - }, - }); - - expect(element.classList.contains('superdoc-error-placeholder')).toBe(true); - }); - it('should return placeholder when doc is not available', () => { const block = createTestTableBlock(); const measure = createTestTableMeasure(); const fragment = createTestTableFragment(); - blockLookup.set(fragment.blockId, { block, measure }); - // Spy on console.error to verify logging const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => { // Intentionally empty - suppress console output during tests @@ -658,7 +585,10 @@ describe('renderTableFragment', () => { doc: null as unknown as Document, // Simulate missing doc fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -679,48 +609,6 @@ describe('renderTableFragment', () => { consoleErrorSpy.mockRestore(); }); - - it('should return placeholder element when measure is wrong kind', () => { - const fragment = createTestTableFragment(); - const block = createTestTableBlock(); - const wrongMeasure = { - kind: 'paragraph' as const, - lines: [], - totalHeight: 0, - }; - - blockLookup.set(fragment.blockId, { - block, - measure: wrongMeasure as unknown as TableMeasure, - }); - - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => { - // Intentionally empty - suppress console output during tests - }); - - const element = renderTableFragment({ - doc, - fragment, - context, - blockLookup, - renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), - applyFragmentFrame: () => { - // Intentionally empty for test mock - }, - applySdtDataset: () => { - // Intentionally empty for test mock - }, - applyStyles: () => { - // Intentionally empty for test mock - }, - }); - - expect(element.classList.contains('superdoc-error-placeholder')).toBe(true); - expect(element.textContent).toContain('Table rendering error'); - expect(consoleErrorSpy).toHaveBeenCalled(); - - consoleErrorSpy.mockRestore(); - }); }); describe('metadata format', () => { @@ -730,13 +618,14 @@ describe('renderTableFragment', () => { const columnBoundaries: TableColumnBoundary[] = [{ index: 0, x: 50, width: 100, minWidth: 25, resizable: true }]; const fragment = createTestTableFragment(columnBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -774,13 +663,14 @@ describe('renderTableFragment', () => { ]; const fragment = createTestTableFragment(columnBoundaries); - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -875,13 +765,14 @@ describe('renderTableFragment', () => { columnWidths: [312, 312], // rescaled from [432, 432] }; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: fragment.columnWidths ?? measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => {}, applySdtDataset: () => {}, @@ -919,13 +810,14 @@ describe('renderTableFragment', () => { // no columnWidths }; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: fragment.columnWidths ?? measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => {}, applySdtDataset: () => {}, @@ -1041,13 +933,14 @@ describe('renderTableFragment', () => { fragment.fromRow = 0; fragment.toRow = 2; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -1196,13 +1089,14 @@ describe('renderTableFragment', () => { fragment.fromRow = 0; fragment.toRow = 3; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -1311,13 +1205,14 @@ describe('renderTableFragment', () => { fragment.fromRow = 0; fragment.toRow = 3; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -1489,13 +1384,14 @@ describe('renderTableFragment', () => { fragment.fromRow = 0; fragment.toRow = 3; - blockLookup.set(fragment.blockId, { block, measure }); - const element = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block, _line, _ctx, _lineIndex, _isLastLine) => doc.createElement('div'), applyFragmentFrame: () => { // Intentionally empty for test mock @@ -1641,7 +1537,10 @@ describe('renderTableFragment', () => { const renderDeps = { doc, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: (_block: ParagraphBlock, _line: unknown, _ctx: unknown, _lineIndex: number, _isLastLine: boolean) => doc.createElement('div'), applyFragmentFrame: () => {}, @@ -1663,7 +1562,6 @@ describe('renderTableFragment', () => { metadata: { columnBoundaries, coordinateSystem: 'fragment' }, }; - blockLookup.set(fragment1.blockId, { block, measure }); const el1 = renderTableFragment({ ...renderDeps, fragment: fragment1 }); const parsed1 = JSON.parse(el1.getAttribute('data-table-boundaries')!); @@ -1763,13 +1661,14 @@ describe('renderTableFragment', () => { height: 20, }; - blockLookup.set('rtl-ghost-table', { block, measure }); - const el = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: () => doc.createElement('div'), applyStyles: (e, s) => Object.assign(e.style, s), applyFragmentFrame: () => {}, @@ -1831,13 +1730,14 @@ describe('renderTableFragment', () => { height: 20, }; - blockLookup.set('rtl-pass-table', { block, measure }); - const el = renderTableFragment({ doc, fragment, context, - blockLookup, + block, + measure, + cellSpacingPx: 0, + effectiveColumnWidths: measure.columnWidths, renderLine: () => doc.createElement('div'), applyStyles: (e, s) => Object.assign(e.style, s), applyFragmentFrame: () => {}, diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index ace45d7b99..62cb36abe3 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -8,10 +8,9 @@ import type { TableFragment, TableMeasure, } from '@superdoc/contracts'; -import { getCellSpacingPx } from '@superdoc/measuring-dom'; import { CLASS_NAMES, fragmentStyles } from '../styles.js'; import { DOM_CLASS_NAMES } from '../constants.js'; -import type { FragmentRenderContext, BlockLookup } from '../renderer.js'; +import type { FragmentRenderContext } from '../renderer.js'; import { renderTableRow } from './renderTableRow.js'; import { applySdtContainerStyling, type SdtBoundaryOptions } from '../utils/sdt-helpers.js'; import { applyBorder, borderValueToSpec, hasExplicitCellBorders } from './border-utils.js'; @@ -22,7 +21,7 @@ type ApplyStylesFn = (el: HTMLElement, styles: Partial) => * Dependencies required for rendering a table fragment. * * Encapsulates all external dependencies needed to render a table, including - * document access, rendering context, block lookup, and helper functions. + * document access, rendering context, pre-resolved table data, and helper functions. */ export type TableRenderDependencies = { /** Document object for creating DOM elements */ @@ -31,8 +30,14 @@ export type TableRenderDependencies = { fragment: TableFragment; /** Rendering context (section info, etc.) */ context: FragmentRenderContext; - /** Lookup map for retrieving block data and measurements */ - blockLookup: BlockLookup; + /** Pre-extracted TableBlock (replaces blockLookup.get()) */ + block: TableBlock; + /** Pre-extracted TableMeasure (replaces blockLookup.get()) */ + measure: TableMeasure; + /** Pre-computed cell spacing in pixels */ + cellSpacingPx: number; + /** Pre-computed effective column widths (fragment.columnWidths ?? measure.columnWidths) */ + effectiveColumnWidths: number[]; /** Optional SDT boundary overrides for container styling */ sdtBoundary?: SdtBoundaryOptions; /** Function to render a line of paragraph content */ @@ -72,12 +77,9 @@ export type TableRenderDependencies = { * - Metadata embedding for interactive table resizing * * **Error Handling:** - * If the table block cannot be found or is invalid, returns an error placeholder - * element instead of throwing. This maintains rendering stability when: - * - Block is missing from blockLookup - * - Block is wrong kind (not 'table') - * - Measure is wrong kind (not 'table') - * - Document object is not available + * If the document is unavailable, returns an error placeholder instead of + * throwing. Table block/measure validation is performed by the caller before + * invoking this helper. * * **SDT Container Styling:** * If the table block has SDT metadata (`block.attrs?.sdt`), applies appropriate @@ -118,7 +120,10 @@ export type TableRenderDependencies = { * doc: document, * fragment: tableFragment, * context: renderContext, - * blockLookup: blocks, + * block: tableBlock, + * measure: tableMeasure, + * cellSpacingPx: 0, + * effectiveColumnWidths: tableMeasure.columnWidths, * renderLine, * applyFragmentFrame, * applySdtDataset, @@ -131,7 +136,10 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement const { doc, fragment, - blockLookup, + block, + measure, + cellSpacingPx, + effectiveColumnWidths, context, sdtBoundary, renderLine, @@ -159,28 +167,6 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement throw new Error('Document is required for table rendering'); } - - const lookup = blockLookup.get(fragment.blockId); - if (!lookup || lookup.block.kind !== 'table' || lookup.measure.kind !== 'table') { - console.error(`DomPainter: missing table block for fragment ${fragment.blockId}`, { - blockId: fragment.blockId, - lookup: lookup ? { kind: lookup.block.kind } : null, - }); - - // Return placeholder element instead of crashing (doc is guaranteed to exist here) - const placeholder = doc.createElement('div'); - placeholder.classList.add(CLASS_NAMES.fragment, 'superdoc-error-placeholder'); - placeholder.textContent = '[Table rendering error]'; - placeholder.style.border = '1px dashed red'; - placeholder.style.padding = '8px'; - return placeholder; - } - - const block = lookup.block as TableBlock; - const measure = lookup.measure as TableMeasure; - // Use per-fragment rescaled column widths when available (SD-1859: mixed-orientation docs - // where measurement width differs from section width). Falls back to measured widths. - const effectiveColumnWidths = fragment.columnWidths ?? measure.columnWidths; const tableBorders = block.attrs?.borders; const tableIndentValue = (block.attrs?.tableIndent as { width?: unknown } | null | undefined)?.width; const tableIndent = typeof tableIndentValue === 'number' && Number.isFinite(tableIndentValue) ? tableIndentValue : 0; @@ -215,8 +201,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // Add table-specific class for resize overlay targeting and click mapping container.classList.add(DOM_CLASS_NAMES.TABLE_FRAGMENT); - // Cell spacing in px (border-spacing). Use measure when present, else resolve from block attrs (e.g. stale/cached measure). - const cellSpacingPx = measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing); + // Cell spacing pre-computed by the resolver; no cross-stage import needed. // Add metadata for interactive table resizing if (fragment.metadata?.columnBoundaries) { diff --git a/packages/layout-engine/pm-adapter/src/utilities.ts b/packages/layout-engine/pm-adapter/src/utilities.ts index 1af7679a7f..f657e1aa56 100644 --- a/packages/layout-engine/pm-adapter/src/utilities.ts +++ b/packages/layout-engine/pm-adapter/src/utilities.ts @@ -1443,62 +1443,9 @@ export function normalizeTextInsets( return { top, right, bottom, left }; } -/** - * Base z-index offset for OOXML relativeHeight values. - * - * OOXML relativeHeight values typically start around 251658240 (a base value). - * By subtracting this base, we get the relative stacking order within the document. - * This preserves fine-grained differences between overlapping elements. - * - * For example: - * - 251658240 (base) → 0 (background layer) - * - 251658242 → 2 (slightly above base) - * - 251658291 → 51 (further above) - */ -export const OOXML_Z_INDEX_BASE = 251658240; - -// ============================================================================ -// OOXML Element Utilities (z-index from relativeHeight) -// ============================================================================ - -/** - * Coerces relativeHeight from OOXML (number or string) to a finite number. - */ -export function coerceRelativeHeight(raw: unknown): number | undefined { - if (typeof raw === 'number' && Number.isFinite(raw)) return raw; - if (typeof raw === 'string' && raw.trim() !== '') { - const n = Number(raw); - if (Number.isFinite(n)) return n; - } - return undefined; -} - -/** - * Normalizes z-index from OOXML relativeHeight value. - * - * OOXML uses large numbers starting around 251658240. To preserve the relative - * stacking order, we subtract the base value to get a small positive number - * suitable for CSS z-index. This ensures elements with close relativeHeight - * values maintain their correct stacking order. - * - * @param originalAttributes - The originalAttributes object from ProseMirror node attrs - * @returns Normalized z-index number or undefined if no relativeHeight - * - * @example - * ```typescript - * normalizeZIndex({ relativeHeight: 251658240 }); // 0 (background) - * normalizeZIndex({ relativeHeight: 251658242 }); // 2 (above background) - * normalizeZIndex({ relativeHeight: 251658291 }); // 51 (further above) - * normalizeZIndex({}); // undefined - * normalizeZIndex(null); // undefined - * ``` - */ -export function normalizeZIndex(originalAttributes: unknown): number | undefined { - if (!isPlainObject(originalAttributes)) return undefined; - const relativeHeight = coerceRelativeHeight(originalAttributes.relativeHeight); - if (relativeHeight === undefined) return undefined; - return Math.max(0, relativeHeight - OOXML_Z_INDEX_BASE); -} +// Canonical implementations moved to @superdoc/contracts; re-imported for local use and re-exported. +export { OOXML_Z_INDEX_BASE, coerceRelativeHeight, normalizeZIndex } from '@superdoc/contracts'; +import { normalizeZIndex } from '@superdoc/contracts'; /** * Resolves the CSS z-index for a floating object based on its behindDoc flag diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 188f07e1c0..8acc0eda74 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2187,15 +2187,6 @@ importers: '@superdoc/font-utils': specifier: workspace:* version: link:../../../../shared/font-utils - '@superdoc/layout-engine': - specifier: workspace:* - version: link:../../layout-engine - '@superdoc/measuring-dom': - specifier: workspace:* - version: link:../../measuring/dom - '@superdoc/pm-adapter': - specifier: workspace:* - version: link:../../pm-adapter '@superdoc/preset-geometry': specifier: workspace:* version: link:../../../preset-geometry @@ -2203,6 +2194,9 @@ importers: specifier: workspace:* version: link:../../../../shared/url-validation devDependencies: + '@superdoc/layout-engine': + specifier: workspace:* + version: link:../../layout-engine vitest: specifier: 'catalog:' version: 3.2.4(@types/debug@4.1.12)(@types/node@25.5.0)(esbuild@0.27.4)(happy-dom@20.4.0)(jiti@2.6.1)(jsdom@27.3.0(canvas@3.2.1))(less@4.4.2)(sass@1.97.3)(terser@5.46.1)(tsx@4.21.0)(yaml@2.8.3)