Skip to content

fix(converter): rotation3d presets, perspective 0, HSL lumMod - #634

Open
ShotaroKataoka wants to merge 2 commits into
mainfrom
fix/rotation3d-and-lummod
Open

ShotaroKataoka wants to merge 2 commits into
mainfrom
fix/rotation3d-and-lummod

Conversation

@ShotaroKataoka

@ShotaroKataoka ShotaroKataoka commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the three converter bugs that #617 marked xfail(strict=True) and removes the markers. Closes #631, closes #632, closes #633.

Changes

  • rotation3d presets (Import: rotation3d presets perspective-bottom / perspective-left-most / perspective-right-most are dropped #631) — the importer's camera map was a hand-written copy that lacked perspectiveBelow / perspectiveContrastingLeftFacing / perspectiveContrastingRightFacing. It is now the inverse of the builder's ROTATION3D_PRESETS, so the two cannot drift again.
  • perspective 0 (Import: rotation3d with perspective 0 comes back with the 120° default #632) — the builder writes perspective: 0 as an orthographicFront camera without fov. The importer now reads an orthographicFront camera that carries a non-zero rotation as perspective: 0. A bevel-only scene (orthographic camera, no rotation) still imports without rotation3d.
  • lumMod / lumOff (Import: lumMod / lumOff on srgbClr and sysClr are applied in RGB instead of HSL luminance #633) — converter/color.py had the transform math twice (_resolve_color_with_transforms, apply_element_transforms) and xml_helpers._apply_srgb_transforms a third, RGB-space variant used by gradient stops on srgbClr and by sysClr. There is now one apply_transforms(hex, {name: val}) (shade / tint in linear gamma, lumMod / lumOff on HSL luminance, then satMod); the other three call it.
  • Rounding — apply_transforms rounds each channel instead of truncating it, so 4F81BD at lumMod 75% is PowerPoint's 376092 (it was 366092). This applies to every transformed color, which can move by 1 per channel.
  • Same mistake elsewhere: theme table style fills (table.py resolve_fill) dropped lumMod / lumOff and mixed tint / shade in sRGB with their own helpers. They now go through _resolve_color_with_transforms; _apply_tint / _apply_shade are removed (no other callers).
  • Tests: the xfails pass and their markers are gone; the preset test now runs over every key of ROTATION3D_PRESETS; new cases for a gradient stop and a sysClr with lumMod, and a theme table style fill with schemeClr + lumMod; the lumMod 75% case is checked exactly (376092).
  • CHANGELOG [Unreleased] → Fixed.

Testing

  • uv run pytest tests/test_effects_roundtrip.py tests/test_converter_fill_line.py tests/test_converter_robustness.py tests/test_element_roundtrip.py tests/test_converter_elements.py tests/test_pptx_import.py tests/test_roundtrip.py tests/test_html_template.py tests/test_chart_roundtrip.py tests/test_converter_structure.py → 439 passed, 4 xfailed (the 4 are unrelated element round-trip losses)
  • uv run ruff check sdpm/ clean
  • Full suite (after the rounding commit): the failures were test_html_assemble.py::test_svg_icon_is_inlined_with_prefixed_ids, which fails the same way on main in a full run, and test_checkout_update.py::test_sigint_during_prepare_rolls_back_and_exits_130. Both pass alone.

Notes

  • Behaviour change on import: gradient stops and sysClr colors with lumMod / lumOff come out a few units different (closer to PowerPoint), and every transformed color can move by 1 per channel from the rounding; theme table style fills with lumMod / lumOff now apply them, and their tint / shade match every other color element (linear gamma). For an srgbClr gradient stop with several transforms, they are now applied in the same order as for schemeClr (shade, tint, lum, sat). Re-importing an existing deck can therefore shift these colors slightly; decks built from JSON are unchanged.
  • Not changed: apply_color_transforms (exported from sdpm.engine.converter, no internal callers, its own lumMod+tint heuristic) and table border / table text schemeClr, which still ignore child transforms. Separate follow-ups if wanted.

🤖 Created by Kiro Crew

ShotaroKataoka added a commit that referenced this pull request Nov 14, 2026
The task finishes in 0.4 s, so it can reach completed before the poll ever
sees working and the wait would time out. The final completed check already
shows the refused cancel left the task running.

SPEC: 20261114-2000_pr-trio-takeover
Progress: #628 R1
Next: #634, #629
@ShotaroKataoka
ShotaroKataoka force-pushed the fix/rotation3d-and-lummod branch from 9007f94 to e217f54 Compare November 14, 2026 11:06
apply_transforms truncated each channel, so 4F81BD at lumMod 75% came out
366092 instead of PowerPoint's 376092. Now that every transformed color goes
through it, rounding there fixes them all (at most 1 per channel).

The CHANGELOG no longer claims every color element: table borders and text of
a theme table style still ignore transforms. The docstring says which paths
pass satMod.

SPEC: 20261114-2000_pr-trio-takeover
Progress: #634 R5 R6 R7
Next: #629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant