Skip to content

fix: resolve worksheet-drawing scheme colors against workbook theme#434

Merged
developer0hye merged 1 commit into
mainfrom
fix/xlsx-drawing-theme-colors
Jul 25, 2026
Merged

fix: resolve worksheet-drawing scheme colors against workbook theme#434
developer0hye merged 1 commit into
mainfrom
fix/xlsx-drawing-theme-colors

Conversation

@developer0hye

@developer0hye developer0hye commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • resolve worksheet-drawing schemeClr fills against the workbook theme
  • apply shared DrawingML color transforms, including lumMod, lumOff, tint, and shade
  • preserve the legacy light/dark fallback when a workbook theme is missing
  • add an Excel-authored, A4-landscape, 100%-zoom regression fixture and complete visual evidence

Related issue

Fixes #430

Testing

  • cargo fmt --all -- --check
  • CARGO_TARGET_DIR=/private/tmp/office2pdf-pr-434-target cargo test --workspace -j 1
  • CARGO_TARGET_DIR=/private/tmp/office2pdf-pr-434-target cargo clippy --workspace --all-targets --all-features -j 1 -- -D warnings
  • CARGO_TARGET_DIR=/private/tmp/office2pdf-pr-434-target cargo test -p office2pdf --test xlsx_fixtures theme_color_drawing
  • python3 -m unittest scripts.tests.test_check_visual_pr
  • native Microsoft Excel PDF export with scripts/macos/export_excel_pdfs.applescript
  • pdfinfo page-count and page-size inspection
  • pdftoppm -r 150 rendering, full-resolution matched crops, and magick compare -metric AE -fuzz 5%
  • exact post-fix fill samples match Excel: #4472C4, #B4C7E7, #548235

Visual impact

  • No rendered PDF change
  • Rendered PDF change or visual evidence added
  • Reason: worksheet-drawing fills now use the workbook theme and DrawingML transforms.

Visual audit

Visual comparison

GT Before After
GT Before After

Required inspection

  • Rendered all evidence at 150 DPI or higher
  • Stored progressive JPEG quality 86 assets with metadata stripped
  • Inspected matched region crops at full resolution
  • Ran the 5% fuzz pixel-difference sweep
  • Inventoried hairlines and border dash styles
  • Inventoried font weight, italic, and underline emphasis

The normalized GT/after comparison has 289,249 differing pixels at 5% fuzz. Every highlighted cluster is explained by the open issues below. The fixture has no hairlines, borders, bold, italic, or underlined runs; all text is regular weight.

Deviation audit

Check Result
Page count/order Remaining: #459 - Excel emits one page; office2pdf adds a blank second page
Element presence Matches GT - the title and all three drawing labels are present
Position/size Remaining: #459 tracks same-row vertical stacking; #460 tracks undersized anchor bounds and cumulative horizontal drift
Rotation/flip No deviation observed - the fixture contains no rotated or flipped elements
Fill Fixed - all three post-fix RGB values match Excel exactly
Stroke/border No deviation observed - the shapes intentionally have no outline or hairline
Text content Matches GT - all four strings are preserved
Font family/weight/style Remaining: #461 tracks drawing theme-font inheritance; #462 tracks the unstyled cell Normal font; regular weight/style is preserved
Text color Matches GT - all text remains black
Alignment Matches GT - drawing labels remain horizontally and vertically centered
Line/paragraph spacing No deviation observed - each text region contains one regular single-line paragraph
Clipping/overflow Remaining: #459 - flow-height reservation spills onto a blank second page; no page-1 glyph clipping was found

Checklist

  • Commits include a Signed-off-by line
  • PR scope contains one root cause
  • Remaining visual deviations each reference an open issue

@developer0hye
developer0hye force-pushed the fix/xlsx-drawing-theme-colors branch from b62dd4e to ad7d36f Compare July 24, 2026 08:52
@developer0hye

Copy link
Copy Markdown
Owner Author

The Visual PR Contract check fails on assets/bugfixes/issue-430/gt.jpg: required evidence file is missing. The validator requires all three of gt/before/after once any of them is touched, and this PR ships only before.jpg and after.jpg.

I tried to produce the missing ground truth from the PR's own fixture and could not:

  • scripts/macos/export_excel_pdfs.applescript with tests/fixtures/xlsx/theme_color_drawing.xlsx fails with Microsoft Excel got an error: parameter error. (-50).
  • A minimal direct save as ... PDF file format script fails the same way, while the same scripts export the business corpus workbooks fine.

The fixture is a hand-built eight-entry archive with no styles.xml, sharedStrings.xml, or docProps, which is likely why Excel declines to open it. So there is no native Excel GT available for this file.

Two ways forward:

  1. Build the evidence fixture with a writer Excel accepts (for example the umya-spreadsheet path the XLSX tests already use) so the same workbook can be exported from Excel for gt.jpg, and keep theme_color_drawing.xlsx as the unit-test fixture.
  2. Add a scheme-colored worksheet drawing to one of the ten tests/golden_mocks/business/sources/xlsx/ workbooks, which already carry native Excel PDFs, and anchor the audit there. None of the ten currently contains any xl/drawings/ part, so this would mean regenerating that case's GT.

I did not change the PR because the choice of fixture is a design decision for this change.

Read the workbook theme palette and resolve worksheet drawing scheme colors through the shared DrawingML color parser, including tint, shade, luminance, and spreadsheet bg/tx aliases. Preserve the historical light/dark fallback when a theme cannot be read.

Add an Excel-authored one-page regression fixture, parser and integration coverage, and 150 DPI native-Excel/before/after evidence for issue #430. Independent drawing layout and font deviations remain tracked in #459, #460, #461, and #462.

Fixes #430

Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
@developer0hye
developer0hye force-pushed the fix/xlsx-drawing-theme-colors branch from ad7d36f to e61a6fa Compare July 25, 2026 13:50
@developer0hye
developer0hye merged commit 8840c6a into main Jul 25, 2026
17 of 65 checks passed
@developer0hye
developer0hye deleted the fix/xlsx-drawing-theme-colors branch July 25, 2026 16:27
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.

XLSX: worksheet-drawing scheme colors ignore the workbook theme

1 participant