Embed the display ICC profile in captures on macOS (#4341) - #4787
Embed the display ICC profile in captures on macOS (#4341)#4787jankarres wants to merge 1 commit into
Conversation
|
@jankarres Did you test the case where multiple monitors have different profiles? Could you please remove the unit tests? I don't think they will be long term useful or maintained so I don't want to add them. If you try adding linux + windows it may solve: #4615 and #3151 |
On wide-gamut/HDR macOS displays, screenshots were saved without an embedded ICC profile. Color-managed apps then assume sRGB, making captures look desaturated next to the native macOS screenshot tool (which tags the file with the active display profile, e.g. "LG HDR 5K"). Add a macOS-only color profile path: - New ColorProfileProvider resolves the active display's QColorSpace via CoreGraphics (CGDisplayCopyColorSpace -> ICC -> QColorSpace::fromIccProfile), with a P3/untagged fallback chain. On other platforms it is a no-op, so behavior there is unchanged. - ScreenGrabber captures the display color space at grab time and exposes it; it is carried on CaptureContext (QPixmap cannot hold a QColorSpace). - For full-desktop captures across multiple monitors, each screen is converted into a common target space (the primary display's) before compositing, then the result is tagged with it, so every region is correct - not just the primary one. Screens whose profile already matches are drawn as-is. - The color space is threaded (default-empty, so existing callers are unaffected) through exportCapture into every byte-producing sink: file save, --raw, clipboard, pin (incl. the pin's own copy/save) and the Open With launcher temp file. Tagging happens at the QPixmap->bytes boundary via QImage::setColorSpace, which only labels and does not resample. Verified end-to-end on an "LG HDR 5K" display: a saved capture now reports `profile: LG HDR 5K` (sips), matching the native screenshot tool. The multi-monitor mixed-profile path is correct by construction but could not be exercised on the single-display machine used here. Upload/Imgur remains untagged for now (documented follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ba677f4 to
856f1cd
Compare
|
Thanks for taking a look, and good questions. Multiple monitors with different profiles: No, I didn't test that (as I have just one monitor), and to be honest it's a case I hadn't even considered until you brought it up. You're right that it matters. Originally the full desktop capture just tagged the whole stitched image with the primary display's profile, which is wrong for any region that comes from a second monitor with a different profile. I've now changed it so each screen gets converted into a common target space (the primary display's) before it's composited, and the final image is tagged with that. Screens that already use the same profile are left untouched, so there's no unnecessary reprocessing. The honest caveat is that I only have a single monitor here, so I could confirm the single display path still works (a saved Unit tests: Removed, as requested. Linux and Windows (#4615 and #3151): For what it's worth, from reading both they actually look like Windows HDR issues rather than the wide gamut tagging this PR is about. With HDR on, Windows works in an extended range (scRGB) and shows SDR content at an adjustable white level, and capturing that into a plain 8 bit sRGB image without tone mapping is what makes it come out washed out (#4615) or too bright (#3151), which also fits "turning HDR off fixes it". So an embedded color profile probably won't solve those on its own; they'd need proper HDR to SDR tone mapping. So I don't think this PR is the right place to fix the issue. |
2 participants
Disclaimer: I'm not a C++/Qt developer. I put this together with AI assistance and tried hard to match the patterns and style already in the codebase, but it needs a review from someone who knows Qt well.
Problem
On a wide-gamut/HDR display the captured pixels are in the display's color space, but Flameshot saved the PNG with no ICC profile attached. Without that tag, other apps fall back to assuming sRGB, so the screenshot looks noticeably desaturated next to the one macOS' built-in tool makes — that one embeds the active display profile (
LG HDR 5K, in my case).The reason it happens: captures travel through the app as a
QPixmap, which can't hold aQColorSpace, and every place that writes the image writes from that pixmap. So the display profile was never asked for and never written out.Fix
The display's color profile is now read once, at capture time, and travels alongside the capture until it's written:
ColorProfileProviderasks CoreGraphics for the active display's profile (CGDisplayCopyColorSpace→ ICC →QColorSpace). If that doesn't give us something usable it falls back to Display P3 for wide-gamut screens, and otherwise just leaves the image untagged like before. On non-macOS platforms it returns nothing, so those builds behave exactly as they do today.ScreenGrabbergrabs the profile when it takes the screenshot and hands it on; it rides along onCaptureContextsince the pixmap itself can't carry it.exportCaptureto each thing that writes an image, saving to a file,--raw, the clipboard, pins (including a pinned image's own copy/save), and the "Open With" temp file. The parameter defaults to empty, so nothing else in the codebase had to change. The actual tagging is justQImage::setColorSpace, which labels the pixels without recoloring them — which is what we want, because they're already in the display's space.Why macOS only
That's the only platform I could actually reproduce the bug on, so it's the only one I felt comfortable fixing. From what I can tell the same thing can affect wide-gamut displays on Windows and on Linux (X11/Wayland) as well, but I had no way to test there, so I deliberately left those paths untouched. I designed the provider so adding the other platforms later is mostly a matter of filling in
forScreen()for each — the reasoning, trade-offs and a full test plan are in the write-up attached to #4341.Testing
There weren't any C++ tests in the repo yet, so I added a small QtTest suite (
tests/unit/test_colorprofile.cpp) and hooked it into the CTest step that CI already runs. It's headless and platform-neutral: it covers the tagging helper, the fallback logic, and a PNG round-trip that proves the profile actually ends up embedded (plus a baseline showing an untagged image stays untagged).I also checked it for real on my
LG HDR 5Kmonitor: before, a saved capture had no profile; nowsipsreportsprofile: LG HDR 5K, same as the native screenshot tool.Known gaps
QColorSpaceis valid (matrix-based RGB); unusual display profiles fall back to Display P3.ENABLE_IMGURoption (off by default), so I couldn't compile or test that path, and it runs through its own separate code rather thanexportCapture.