fix(desktop): preserve layered PPTX backgrounds - #6845
Conversation
|
🧪 Queued for QA validation — this PR changes the editable PPTX export output users open in WPS/PowerPoint, so we want a manual QA pass before merge. Nothing needed from you right now; we'll update here once it's validated. Thanks! |
nettee
left a comment
There was a problem hiding this comment.
Two correctness blockers remain in apps/desktop/src/main/deck-capture.ts. The focused regression test passes, and pnpm run typecheck / pnpm run build pass in apps/desktop, but the changed gradient allowlist and layout mutation still regress existing deck content. See the inline comments for the concrete cases.
Keep unsupported gradients and static nested panels on their authored elements, and avoid duplicating the slide background shim when Chromium capture owns the layer.\n\nGenerated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — 8cd874e
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
Fresh pass over the current head found two correctness blockers in apps/desktop/src/main/deck-capture.ts. The positioned-layer case is isolated correctly, but the new preprocessing still drops or mis-renders other authored gradient content; the inline comments describe the failing paths and the concrete fixes needed.
Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — fad335c
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
Two correctness issues remain in the layered-background extraction: it changes masked gradient layers and does not collect pseudo-element gradient layers. The focused desktop suite, desktop typecheck/build, workspace typecheck, and guard pass, but the output-fidelity cases below still need a fix before merge.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — d3d0b3b
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
Fresh implementation pass found three blocking output-fidelity issues in the new layered-background extraction. The focused desktop suite (334 passed, 1 skipped), desktop typecheck/build, guard, and workspace typecheck all pass locally, but these cases are not covered by the current media-extension assertions.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — 6b17766
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
nettee
left a comment
There was a problem hiding this comment.
@Siri-Ray I found two remaining blocking correctness/performance issues in the layered-background export path. A painted intermediate ancestor can be omitted from a flattened compositor group, and the clipped/masked backdrop fallback can perform an unbounded DOM hit-test scan. The focused fixtures exercise small filtered and stripe cases, but neither case is bounded or covered for the general DOM shapes below; please address the anchored issues before merge.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Preserve intermediate wrapper fills inside compositor captures so raster replacements keep authored paint order. Bound clipped-backdrop fallback work with stratified sampling instead of area-scaled hit testing. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — a330761
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The normalization/capture split is thoughtful, but the new bounded clipped-backdrop sampler still loses authored paint. The focused desktop regression suite is red on this head: 45/46 tests pass, while the clipped-stripe Chromium comparison exceeds its stated threshold.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Replace the fixed paint-membership grid with a clip-aware Chromium paint-order probe so narrow masked or clipped backdrops cannot fall between sample centers. Cover the 1920x1080 DPR-2 case with a focused 1px stripe pixel comparison. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — 9cfcd85
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
@Siri-Ray I found one remaining blocking output-fidelity gap in apps/desktop/src/main/deck-capture.ts: CSS clipping and masking are not treated as compositor boundaries, so the Chromium background raster can diverge from the native editable foreground. The inline finding covers the self and ancestor cases and the focused fixture coverage needed to close it.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — c2836fa
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The focused Electron layered-background regression is still red on c2836fa: the clipped-stripe fixture exports pixels that do not match Chromium, so this output-fidelity path is not ready to merge.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Let clip and mask candidates reach the clip-free paint-order probe, and clear the first probe state before evaluating the second pass. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — 1781778
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
@Siri-Ray The desktop typecheck and repository guard pass, but the new layered-background regression suite is still red on this head: the focused Electron test run reports 49 passing tests and one failure in the clipped-stripe backdrop case. That mismatch is blocking because it exercises the PR's core Chromium-capture path.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Resolve each Chromium capture through its slide picture geometry and relationship, using pixel similarity only to disambiguate overlapping images. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — ab689ae
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The new geometry-based media association is a useful correction, but the final Electron regression test is still renderer-scale dependent. I reproduced the clipped-stripe case failing on the repository’s macOS Electron runtime: the Chromium and exported captures are both 192×96, but their center colors are 117 and 128 and the mean channel delta is 8.19 (the assertion allows 1). The inline comment points to the hard-coded probe scale. Please make that capture DPR-aware so this new regression remains green on both DPR 1 and DPR 2; the production path and the other probe captures already establish the needed convention.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Match the Electron media probe capture scale to the production 2-physical-pixels-per-CSS-pixel formula across DPR 1 and DPR 2 renderers. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — 4a05094
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The Chromium isolation work and expanded Electron/PPTX coverage are a strong step toward preserving editable layered backgrounds. One blocking output-fidelity failure remains on this head: pnpm --filter @open-design/desktop test reports 375 passed, 1 failed, and 1 skipped because the clipped-stripe blend case emits [128,128,128] instead of Chromium's [117,117,117] (mean channel delta 8.195 vs. the threshold of 1). Please correct the clipped-backdrop capture before merge; the detailed reproduction and fix direction are anchored below.
Use full-coverage clip and mask probe values so Chromium paint-order sampling preserves the candidates' stacking contexts before authored styles are restored.\n\nGenerated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — d778871
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The current head still has one blocking output-fidelity regression in the clipped-stripe blend path. I reproduced it with the changed desktop suite; the detailed failure and fix direction are inline.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Expand paint-order probes across standard and prefixed clip or mask ancestors, then keep the focused Chromium-to-PPTX stripe regression on that path. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
|
Looper fixer round complete — ddc7933
🔁 Powered by Looper · runner=fixer · agent=codex · An autonomous AI dev team for your GitHub repos. |
nettee
left a comment
There was a problem hiding this comment.
The latest ancestor-clipped backdrop change still leaves the focused editable-PPTX regression red. The local focused Electron suite reports 375 passing, 1 skipped, with .clipped-stripe-target exporting [128,128,128] instead of Chromium [117,117,117]; the inline finding below explains why the new probe still drops those backdrop pixels.
| element.style.setProperty("pointer-events", "auto", "important"); | ||
| } | ||
| for (const element of [slide, ...Array.from(slide.querySelectorAll<HTMLElement>("*"))]) { | ||
| if (element === target || element.contains(target) || target.contains(element)) continue; |
There was a problem hiding this comment.
Blocking — nested clipped backdrops are skipped during isolation.
When the capture target is the clipping ancestor (.clipped-stripe-clip) and the layered blend element is its sibling, this guard excludes every descendant of the target. That also excludes .clipped-stripe-backdrop, which is the only paint behind the blend but is not a captured compositing member. Isolation then hides it, so the captured PNG is uniformly [128,128,128] while Chromium renders [117,117,117]; apps/desktop/tests/main/pptx-layered-background.test.ts:577 fails on this head.
Probe descendants of a compositor root that are not members (or conservatively retain uncertain nested backdrop paint) before hiding them, and keep the clipped-stripe Chromium/PPTX comparison green.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Probe non-member compositor descendants against the actual backdrop-dependent capture members and keep selected backdrop paint visible during isolation. Generated-By: looper 0.11.8 (runner=fixer, agent=codex)
Why
An editable PPTX exported from a real Open Design deck rendered its cream paper background as black in WPS and macOS Quick Look. The source deck uses two CSS
linear-gradient(...)background layers for its paper texture. The vendoreddom-to-pptxgradient parser assumes a single gradient and greedily combines the two layers into one invalid SVG gradient, producing the dark fallback image.This fixes the reported export without changing the deck source or flattening the whole slide. It is intentionally scoped to the layered-background problem; the font and heading fidelity work remains in #6840.
What users will see
Exporting a deck with layered CSS gradients through the existing editable PPTX action preserves the authored background instead of turning it black. The background-only layer is rasterized with Chromium, while authored text and shapes remain native editable PowerPoint objects.
Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-packflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies); workspace-packagepackage.jsonfiles are out of scope. Include a paragraph on what we get vs. what bytes we ship (seeCONTRIBUTING.md→ Code style)Screenshots
No new UI entry point. Manual visual verification used the reported 10-page deck: the exported slide background changed from the black fallback to the authored cream paper texture in macOS Quick Look, and the PPTX package contains native text/shape objects alongside the isolated background image.
Bug fix verification
apps/desktop/tests/main/pptx-layered-background.test.tsmain: yes — the converter received the original two-layer gradient and the assertion failed because no isolated Chromium-capture layer existed.Validation
pnpm --filter @open-design/desktop test— 327 passed, 1 skippedpnpm --filter @open-design/desktop typecheckpnpm --filter @open-design/desktop buildpnpm guardpnpm typecheck