Skip to content

Fix SVG viewport and path geometry - #143

Open
dajiaohuang wants to merge 1 commit into
dompdf:masterfrom
dajiaohuang:fix/svg-viewbox-and-path-geometry
Open

dajiaohuang wants to merge 1 commit into
dompdf:masterfrom
dajiaohuang:fix/svg-viewbox-and-path-geometry

Conversation

@dajiaohuang

@dajiaohuang dajiaohuang commented Oct 1, 2026 •

Copy link
Copy Markdown

Related: #128, #93

Reassessment of the current patch at 358c38d:

  • The arc endpoint change addresses a concrete omission in the SVG from Eyes in QR code get slanted/mangled (elliptic arcs are rendered incorrectly?) #128: the arc from (2.75, 0) to (0, 2.75) has distinct endpoints although dx + dy is zero. Document/render reaches Path.drawArc, and the old test returns before emitting the curve to the PDF surface. The coincident-endpoint and zero-radius handling remain necessary. This is source-level evidence, not rendered-PDF acceptance.
  • The viewport changes are incomplete; this PR does not yet fix viewbox is not correctly mapped to SVG viewport #93. Document lowercases attribute keys, but applyViewbox reads preserveAspectRatio rather than preserveaspectratio. Also, standard align values contain YMid/YMax, whereas the conditions test yMid/yMax, so changing the destination axis alone does not enable those branches for standard values. The traced caller is Symbol through use; root Document and nested Group do not call applyViewbox.
  • Fixed-length path slicing normalizes the arrays returned by Path.parse. Its rendering-defect necessity has not been established: Path.start reads only the prescribed leading operands, and valid arc groups use the separate arc parser. The previous claim that this prevents overlapping rendered coordinate groups was unsupported.

Follow-up source work is still needed: repair the normalized attribute key and standard Y alignment tests, and remove the unsupported rendering claim/hunk rather than treating it as proven. Any wider #93 root/nested viewport repair requires its own complete caller-level assessment. The existing source branch has not been revised in this reassessment because source pushes would trigger the configured CI, which this task is not authorized to execute.

Validation limits: no PHP CLI, tests, lint, build, SVG/PDF/font rendering, browser validation, installation, or CI execution was performed in this reassessment. Existing green PHP 7.1-8.4 checks and CodeFactor are not dedicated geometry regression proof. Prior whitespace-check evidence applies only to the unchanged source patch.

@bsweeney bsweeney added this to the 1.0.3 milestone Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

viewbox is not correctly mapped to SVG viewport

2 participants