Skip to content

Commit 834d578

Browse files
kjkclaude
andcommitted
Record fork chrome-fix evaluation: all superseded by darkmodelib
Evaluated the sumatrapdf-plus dark-chrome bug fixes against master; none apply: the dark-to-light scrollbar reset is already handled (darkmodelib's setDarkExplorerTheme resets the native theme when experimental dark is inactive, and we call it on every theme change), frame border colors are set via UpdateWindowFrameBorderColor/dwm::SetWindowBorderColor, and the toolbar-separator work is tied to fork-only toolbar buttons. Details in ai/super-themes.md; also record the F outcome in ai/plus-merge.md and a PrintWindow verification gotcha in the verify skill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> prompt: port fork theme/chrome improvements: evaluate remaining chrome fixes
1 parent c837e3d commit 834d578

3 files changed

Lines changed: 40 additions & 1 deletion

File tree

.claude/skills/verify/SKILL.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ postMessage(frame, 0x0010 /*WM_CLOSE*/, 0, 0);
3939

4040
## Gotchas
4141

42+
- `captureWindowToPng` (PrintWindow) does NOT capture the custom caption row's
43+
buttons (painted via `BeginPaint` directly on the frame DC) nor native
44+
scrollbars (non-client area) — those regions come out blank. Screen capture
45+
(`CopyFromScreen`) is also unavailable in this environment (blank desktop).
46+
To verify such drawing code, dump its output to a bitmap via a temporary
47+
harness (render into a `CreateDIBSection` DC, save with `HBITMAPToBmpFormat`
48+
+ `file::WriteFile`, view, then remove the harness).
49+
4250
- `out/dbg64/SumatraPDF-settings.txt` exists (portable mode): the dbg build **loads** it even under `-for-testing` (which only prevents saving). Stale values there change app behavior in tests — e.g. a non-default `PdfDocumentColorMode` silently alters rendering. Check it when the app behaves unexpectedly at startup; it's written only by non-`-for-testing` (manual) launches.
4351

4452
- Unit tests: `bun cmd/run-unit-tests.ts -dbg` (but verification = driving the app, not tests).

ai/plus-merge.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ additions (we generate via premake).
274274
| _(pending)_ | C. Ask AI || |
275275
| 2026-07-11 | D. Smart PDF dark mode | **merged** | as `86e5cb24c`; ported all 15 `PdfDarkMode*` modules + 2 unit-test files by hand (Vec API `.Size()/.at()` → `len()/[]`, `utils/`→`base/` includes, no `#pragma once`, PCH opt-outs). KEY FINDING: the fork ships with the object-level OKLab renderer compile-time OFF (`kPdfDarkModeRenderer = LegacyBitmapPostProcess` in PdfDarkModeColor.cpp) — the live feature is per-doc color mode + classifier-driven image preservation in the bitmap recolor; we kept it identically dormant (wired into RenderPage but unreachable). SEMANTIC ADAPTATION: fork keys dark mode off their dark themes (feature F recolors pages per theme); master's themes never touch page colors, so ours keys off the effective page colors (`FixedPageUI.InvertColors` / custom dark colors) — with invert on, `auto` preserves photos (new default), `black` = old invert-everything, `light` = don't invert PDFs. Integration: `RenderPageArgs.darkProfile` + `darkModeEpoch` (request/entry staleness) in RenderCache, `UpdateBitmapColors` extended with linkColor+skipRects, `GetBitmapRecolorSkipRects` (content-stream image collection + classifier gating) in EngineMupdf, `PdfDarkModeNoOp.cpp` for filter/preview/test_engines/mac, `fz_colorspace_n` added to libmupdf.def. Setting `PdfDocumentColorMode` (auto/black/light) + 4 commands. Verified end-to-end with a synthetic photo PDF (screenshots: photo preserved in auto, inverted in black, original in light; deterministic re-renders). Fork's RenderCache non-blocking CancelRendering + UpdateAfterThemeChange reorder (735894b11) still not taken — master's CancelRendering was already reworked. |
276276
| 2026-07-10 | E. CAD line enhancement (`735894b11`, `b058ba496`, `38006d0e8`) | **merged** | as `211f989d1`; ported by hand against master (fork base predates Pixmap/PCH/Str refactors). Key adaptations: `EngineeringDrawingEnhance` mode pushed via setter from `LoadSettings` (gDisableFormJavaScript pattern) instead of reading gGlobalPrefs in PdfCadDetect.cpp — so no `PdfCadEnhanceNoOp.cpp` needed; real files compile everywhere (incl. mac) with mode defaulting to Off outside the app. Fixed fork's read-after-free of analysis-device stats (stats now caller-owned) and their unguarded fz_rethrow out of DetectCadPdf. Added 12 missing fz_* exports to libmupdf.def (fork's approach was the NoOp stub instead). Skipped: their RenderCache/UpdateAfterThemeChange hunks (dark-mode toolbar fix, belongs to D). Setting editable in Advanced Settings dialog (enum values added). |
277-
| _(pending)_ | F. Themes & chrome | | |
277+
| 2026-07-11 | F. Themes & chrome | **ideas ported** | analysis + outcome in [super-themes.md](super-themes.md): light/dark toggle + System theme as `0de700a15`, Light Warm palette as `8797b20f3` (Dracula/black already existed), Win11 caption glyphs as `c837e3d92`; their page-color-driving theme philosophy rejected (kept master's chrome-only themes); all dark-chrome bug fixes evaluated and found superseded by master's darkmodelib handling |
278278
| _(pending)_ | G. Ebook engine work || |
279279
| 2026-07-10 | H. Selection toolbar (`db0b32b7a` + follow-up polish) | **merged** | as `dba10007e`; ported `SelectionToolbar.{cpp,h}` by hand; folded the shared `FloatingPopupStyle.{cpp,h}` helpers into it (they only exist to share chrome with WordLookup, feature B). Buttons rewritten on top of master's `CommandAvailability` (`GetCommandVisibility`, `CommandSurface::Toolbar`): Copy, Read Aloud, Highlight, Underline, Squiggly, Strike Out; dropped fork's Ask AI / Look Up buttons (features C/B). Theme colors mapped to master's Theme.h (`IsLightColor(ThemeWindowBackgroundColor())` for dark detection). Setting `Annotations.SelectionToolbar` (bool, default true). Skipped unrelated hunks bundled in `db0b32b7a`: Menu.cpp UINT_PTR fix (master already fixed via CommandAvailability refactor), `EngineMupdfGetAnnotations` quick-load/no-pagesLock change (master's locking was rewritten; possible perf win for large docs — evaluate separately), `HwndSetIcon` ICON_SMALL addition (unrelated; could take separately). |
280280
| _(pending)_ | I. Home page list view || |

ai/super-themes.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,37 @@ polish. Commit series `1cd0c85d6`→`b0002c393`.
7878
theme; maximized frame edge color in dark themes). The rest is presumed
7979
superseded by darkmodelib.
8080

81+
## Outcome (2026-07-11)
82+
83+
1. Done as `0de700a15` (System theme + `CmdToggleLightDarkTheme` +
84+
`LastLightTheme`/`LastDarkTheme`).
85+
2. Done as `8797b20f3` — only **Light Warm** was added; the fork's Dracula and
86+
pure-black palettes already exist in master as the `Dracula` and `Dark`
87+
themes.
88+
3. Done as `c837e3d92` (`src/CaptionGlyphs.{cpp,h}` +
89+
`tools/gen_caption_glyphs.py`, wired into `DrawCaptionButton`).
90+
4. **All evaluated chrome fixes are superseded on master — nothing ported:**
91+
- scrollbar reset (`597f0c47d`): master calls
92+
`DarkMode::setDarkScrollBar(hwndCanvas)` on *every* theme change and
93+
darkmodelib's `setDarkExplorerTheme` passes `nullptr` (reset) when
94+
experimental dark is inactive, so switching back to a light theme already
95+
resets the scrollbar. The fork fixed a code shape (dark-only call) master
96+
doesn't have.
97+
- maximized frame edges / border color (`c4018ec06`): master already has
98+
`UpdateWindowFrameBorderColor` + `dwm::SetWindowBorderColor` on theme
99+
change.
100+
- toolbar separators (`0e493c658`, parts of `c4018ec06`): tied to their
101+
extra toolbar buttons; master's toolbar has since been reworked (overlay
102+
mode etc.); no symptom observed.
103+
- `81defc78b` (Light-Warm menubar without tabs), `36468333a` (broad dark
104+
polish): fork-theme-specific; master themes chrome via darkmodelib's
105+
`setWindowMenuBarSubclass`/`setChildCtrlsSubclassAndTheme`.
106+
107+
Verification note: neither the custom caption nor native scrollbars render
108+
into `PrintWindow` captures (caption paints via `BeginPaint` on the frame DC;
109+
scrollbars are non-client) — glyphs were verified by dumping
110+
`DrawCaptionSysButtonGlyph` output to bitmaps via a temporary harness.
111+
81112
## Gotchas
82113

83114
- Fork's theme code assumes their fixed 5-theme indexes (`kThemeIdx*`) —

0 commit comments

Comments
 (0)