fix(shapes): render grouped DrawingML shapes with custom geometry (SD-1877) by tupizz · Pull Request #2105 · superdoc-dev/superdoc

tupizz · 2026-02-19T01:04:12Z

Summary

Fixed grouped DrawingML shapes rendering as solid black rectangles when they use custom geometry (a:custGeom) instead of preset geometry (a:prstGeom)
Added extractCustomGeometry() parser that converts DrawingML path commands (moveTo, lnTo, cubicBezTo, quadBezTo, close) to SVG path data, wired through the full pipeline: DOCX import → PM node → pm-adapter → contracts → DomPainter + ShapeGroupView
Used fill-rule="evenodd" for hollow frame paths and vector-effect="non-scaling-stroke" hairline strokes to keep thin borders visible after non-uniform group coordinate scaling
Fixed floating drawing width constraint: wrapNone drawings no longer incorrectly constrained to content area width
Rewrote group coordinate rendering to use visual-space positioning (pre-scaled by importer) instead of CSS scale transforms

Visual Comparison

Before (main)	After (this PR)	Word Reference

Solid black rectangle hides all content	Border frame + gray background render correctly	Expected rendering in Microsoft Word

Test plan

All unit tests pass (super-editor: 6162, painter-dom: 598, measuring-dom: 219, pm-adapter: 1533)
All 56 visual regression tests pass (0 failures)
Verified in browser with the sample DOCX — shape group with border frame and gray background renders correctly matching Word
Test with other documents containing grouped shapes to verify no regressions
Test with documents containing preset geometry shapes to verify existing rendering is unaffected

…-1877) Grouped DrawingML shapes with custom geometry (a:custGeom) were rendering as solid black rectangles because the importer and renderers only supported preset geometry (a:prstGeom). This adds full custom geometry support and fixes several related sizing issues. - Add extractCustomGeometry() parser that converts DrawingML path commands (moveTo, lnTo, cubicBezTo, quadBezTo, close) to SVG path data - Wire custom geometry through the full pipeline: import → PM node attr → pm-adapter → contracts → DomPainter + ShapeGroupView - Use fill-rule="evenodd" for hollow frame paths (winding creates cutout) - Add vector-effect="non-scaling-stroke" hairline stroke so thin borders remain visible after non-uniform group coordinate scaling - Fix floating drawing width: wrapNone drawings no longer constrained to content area width, matching Word's behavior - Rewrite group coordinate rendering to use visual-space positioning (pre-scaled by importer) instead of CSS scale transforms

linear · 2026-02-19T01:04:14Z

SD-1877 Grouped DrawingML shape renders as solid black (hiding all content) instead of hollow frame

chatgpt-codex-connector · 2026-02-19T01:04:16Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Copilot

Pull request overview

This PR fixes rendering of grouped DrawingML shapes that use custom geometry (a:custGeom) instead of preset geometry (a:prstGeom). Previously, these shapes would render as solid black rectangles. The fix adds a complete pipeline for parsing custom geometry from DOCX files and rendering them as SVG paths.

Changes:

Added extractCustomGeometry() function to parse DrawingML path commands and convert them to SVG path data
Updated rendering in both DomPainter and ShapeGroupView to handle custom geometry with proper coordinate space mapping
Fixed floating drawing width constraints to not constrain wrapNone drawings to content area width
Simplified group coordinate rendering by removing CSS scale transforms in favor of pre-scaled positions

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
packages/super-editor/src/extensions/vector-shape/vector-shape.js	Added `customGeometry` attribute to vectorShape node schema
packages/super-editor/src/extensions/shape-group/ShapeGroupView.js	Added custom geometry rendering logic with nested SVG and viewBox mapping
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js	Added `extractCustomGeometry()` and `convertDrawingMLPathToSvg()` to parse DrawingML path commands
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.js	Integrated custom geometry extraction into shape and shape group import
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers.test.js	Updated test to reflect new behavior when shape kind is missing
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-shape-group-helpers.test.js	Added mock for `extractCustomGeometry` function
packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/encode-image-node-helpers-header.test.js	Added mock for `extractCustomGeometry` function
packages/layout-engine/pm-adapter/src/converters/shapes.ts	Added `customGeometry` to DrawingBlock conversion
packages/layout-engine/painters/dom/src/renderer.ts	Added `tryCreateCustomGeometrySvg()` method with hairline stroke workaround and simplified group transform rendering
packages/layout-engine/measuring/dom/src/index.ts	Fixed width constraint logic to exclude floating drawings (`wrapNone`)
packages/layout-engine/contracts/src/index.ts	Added `CustomGeometryData` type definition

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js

Copilot · 2026-02-19T01:11:03Z

packages/layout-engine/painters/dom/src/renderer.ts

+        const transform = needsTransform ? ` transform="scale(${scaleX}, ${scaleY})"` : '';
+        const strokeAttr =
+          strokeColor !== 'none' ? ` stroke="${strokeColor}" stroke-width="${strokeWidth}"` : edgeStroke;
+        return `<path d="${p.d}" fill="${fillColor}" fill-rule="evenodd"${strokeAttr}${transform} />`;


The path data p.d is directly interpolated into HTML without escaping. While the coordinates come from XML attributes that are typically numeric, if the XML parser allows special characters or if there's any way to inject SVG commands through malformed coordinates, this could create an XSS vector. The parseSafeSvg method strips unsafe content but only after the string is constructed. Consider HTML-escaping the path data before interpolation as a defense-in-depth measure, even though the current implementation should be safe for well-formed DOCX files.

Copilot · 2026-02-19T01:11:03Z

packages/layout-engine/painters/dom/src/renderer.ts

+        const scaleX = viewW / pathW;
+        const scaleY = viewH / pathH;
+        const transform = needsTransform ? ` transform="scale(${scaleX}, ${scaleY})"` : '';


If pathW or pathH is zero (from parsing w="0" or h="0" attributes), the division viewW / pathW or viewH / pathH will produce Infinity. This will create an invalid transform="scale(Infinity, ...)" attribute in the SVG, potentially causing rendering issues. Consider adding a guard to skip the transform when either dimension is zero, or fallback to a default non-zero value.

Suggested change

const scaleX = viewW / pathW;

const scaleY = viewH / pathH;

const transform = needsTransform ? ` transform="scale(${scaleX}, ${scaleY})"` : '';

let transform = '';

if (needsTransform && pathW !== 0 && pathH !== 0) {

const scaleX = viewW / pathW;

const scaleY = viewH / pathH;

if (Number.isFinite(scaleX) && Number.isFinite(scaleY)) {

transform = ` transform="scale(${scaleX}, ${scaleY})"`;

}

}

Copilot · 2026-02-19T01:11:03Z

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js

+      case 'a:moveTo': {
+        const pt = cmd.elements?.find((el) => el.name === 'a:pt');
+        if (pt) {
+          parts.push(`M ${pt.attributes?.['x'] || 0} ${pt.attributes?.['y'] || 0}`);


If the XML contains non-numeric values for coordinate attributes, the expression pt.attributes?.['x'] || 0 will first evaluate to the string value (which is truthy), and then that string is directly interpolated into the SVG path. For example, if x="abc", the path becomes M abc 0, which is invalid SVG. Consider parsing the coordinate values with parseInt() or parseFloat() and validating the result, falling back to 0 for NaN values.

Copilot · 2026-02-19T01:11:04Z

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js

+      case 'a:lnTo': {
+        const pt = cmd.elements?.find((el) => el.name === 'a:pt');
+        if (pt) {
+          parts.push(`L ${pt.attributes?.['x'] || 0} ${pt.attributes?.['y'] || 0}`);
+        }
+        break;
+      }
+      case 'a:cubicBezTo': {
+        const pts = cmd.elements?.filter((el) => el.name === 'a:pt') || [];
+        if (pts.length === 3) {
+          parts.push(
+            `C ${pts[0].attributes?.['x'] || 0} ${pts[0].attributes?.['y'] || 0} ` +
+              `${pts[1].attributes?.['x'] || 0} ${pts[1].attributes?.['y'] || 0} ` +
+              `${pts[2].attributes?.['x'] || 0} ${pts[2].attributes?.['y'] || 0}`,
+          );
+        }
+        break;
+      }
+      case 'a:quadBezTo': {
+        const pts = cmd.elements?.filter((el) => el.name === 'a:pt') || [];
+        if (pts.length === 2) {
+          parts.push(
+            `Q ${pts[0].attributes?.['x'] || 0} ${pts[0].attributes?.['y'] || 0} ` +
+              `${pts[1].attributes?.['x'] || 0} ${pts[1].attributes?.['y'] || 0}`,
+          );
+        }
+        break;


Similar to the moveTo case, non-numeric coordinate values will be directly interpolated into the SVG path without validation. Consider parsing and validating coordinate values to ensure they are numeric before interpolation.

Copilot · 2026-02-19T01:11:04Z

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js

+export function extractCustomGeometry(spPr) {
+  const custGeom = spPr?.elements?.find((el) => el.name === 'a:custGeom');
+  if (!custGeom) return null;
+
+  const pathLst = custGeom.elements?.find((el) => el.name === 'a:pathLst');
+  if (!pathLst?.elements) return null;
+
+  const paths = pathLst.elements
+    .filter((el) => el.name === 'a:path')
+    .map((pathEl) => {
+      const w = parseInt(pathEl.attributes?.['w'] || '0', 10);
+      const h = parseInt(pathEl.attributes?.['h'] || '0', 10);
+      const d = convertDrawingMLPathToSvg(pathEl);
+      return { d, w, h };
+    })
+    .filter((p) => p.d);
+
+  if (paths.length === 0) return null;
+  return { paths };
+}


There are no unit tests for the new extractCustomGeometry() function. Given that this is a core parser that converts DrawingML path commands to SVG, it should have comprehensive test coverage for each supported command type (moveTo, lnTo, cubicBezTo, quadBezTo, close), edge cases (missing attributes, empty paths, multiple paths with different coordinate spaces), and error handling. The existing test file vector-shape-helpers.test.js would be an appropriate place to add these tests.

Copilot · 2026-02-19T01:11:04Z

packages/super-editor/src/extensions/shape-group/ShapeGroupView.js

+        // Scale if this path has a different coordinate space
+        const pathW = pathData.w || viewW;
+        const pathH = pathData.h || viewH;
+        if (pathW !== viewW || pathH !== viewH) {


Similar to the DomPainter issue, if pathW or pathH is zero, the division viewW / pathW or viewH / pathH will produce Infinity, creating an invalid transform="scale(Infinity, ...)" attribute. Consider adding a guard condition to skip the transform or use a safe fallback value when either dimension is zero.

Suggested change

if (pathW !== viewW || pathH !== viewH) {

const hasValidDimensions = pathW !== 0 && pathH !== 0;

if (hasValidDimensions && (pathW !== viewW || pathH !== viewH)) {

Copilot · 2026-02-19T01:11:05Z

packages/super-editor/src/core/super-converter/v3/handlers/wp/helpers/vector-shape-helpers.js

+      const w = parseInt(pathEl.attributes?.['w'] || '0', 10);
+      const h = parseInt(pathEl.attributes?.['h'] || '0', 10);


The fallback value for missing w or h attributes is '0', which when parsed with parseInt() produces 0. This means paths without explicit dimensions will have zero width/height, which can cause division-by-zero issues downstream when computing scale factors (as seen in DomPainter line 3136 and ShapeGroupView line 330). Consider using a non-zero default value or handling the zero case explicitly in the rendering code.

Suggested change

const w = parseInt(pathEl.attributes?.['w'] || '0', 10);

const h = parseInt(pathEl.attributes?.['h'] || '0', 10);

const rawW = pathEl.attributes?.['w'];

const rawH = pathEl.attributes?.['h'];

let w = rawW != null ? parseInt(rawW, 10) : 1;

let h = rawH != null ? parseInt(rawH, 10) : 1;

if (!Number.isFinite(w) || w <= 0) w = 1;

if (!Number.isFinite(h) || h <= 0) h = 1;

Copilot · 2026-02-19T01:11:05Z

packages/layout-engine/painters/dom/src/renderer.ts

+    const firstPath = custGeom.paths[0];
+    const viewW = firstPath.w || width;
+    const viewH = firstPath.h || height;


If firstPath.w is 0 (from the parser's default), viewW will be 0, which will cause the SVG viewBox="0 0 0 ..." to be invalid. This creates an invalid SVG that may not render. Consider using the shape's actual width parameter as a minimum fallback, or validating that viewW and viewH are positive before using them.

Copilot · 2026-02-19T01:11:05Z

packages/super-editor/src/extensions/shape-group/ShapeGroupView.js

+      const firstPath = customGeometry.paths[0];
+      const viewW = firstPath.w || width;
+      const viewH = firstPath.h || height;


Similar to DomPainter, if firstPath.w or firstPath.h is 0, the viewBox will have zero dimensions, creating an invalid SVG. Consider using the shape's actual width and height as minimum fallbacks, or validating that the dimensions are positive.

Copilot AI review requested due to automatic review settings February 19, 2026 01:04

github-actions bot added the risk: critical label Feb 19, 2026

Copilot started reviewing on behalf of tupizz February 19, 2026 01:04 View session

tupizz self-assigned this Feb 19, 2026

github-actions bot added risk: sensitive and removed risk: critical labels Feb 19, 2026

Copilot AI reviewed Feb 19, 2026

View reviewed changes

	if (pathW !== viewW \|\| pathH !== viewH) {
	const hasValidDimensions = pathW !== 0 && pathH !== 0;
	if (hasValidDimensions && (pathW !== viewW \|\| pathH !== viewH)) {

		const w = parseInt(pathEl.attributes?.['w'] \|\| '0', 10);
		const h = parseInt(pathEl.attributes?.['h'] \|\| '0', 10);

-      const w = parseInt(pathEl.attributes?.['w'] || '0', 10);
-      const h = parseInt(pathEl.attributes?.['h'] || '0', 10);
+      const rawW = pathEl.attributes?.['w'];
+      const rawH = pathEl.attributes?.['h'];
+      let w = rawW != null ? parseInt(rawW, 10) : 1;
+      let h = rawH != null ? parseInt(rawH, 10) : 1;
+      if (!Number.isFinite(w) || w <= 0) w = 1;
+      if (!Number.isFinite(h) || h <= 0) h = 1;

Conversation

tupizz commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Visual Comparison

Test plan

Uh oh!

linear bot commented Feb 19, 2026

Uh oh!

chatgpt-codex-connector bot commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

tupizz commented Feb 19, 2026 •

edited

Loading