fix(icons): keep the hi-DPI master instead of downscaling on the JVM (#436) - #437
Merged
Merged
Conversation
…436) forCurrentOperatingSystem() and forMenuItem() downsampled the Compose scene to a single physical size before the native layer ever saw the bitmap, so high-DPI detail was lost on the JVM. The JVM now emits the scene-resolution master and each backend owns display-scale downsampling. - Windows: multi-frame ICO (16/20/24/32/40/48/64, clipped to the master) and icon loading at SM_CXSMICON so the shell picks an exact DPI match. - Linux: the SNI pixmap pyramid skips levels larger than the source instead of upscaling every level past a 24px master. - macOS: menu icons stay at 16pt over a >=3x master.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #436.
Summary
IconRenderProperties.forCurrentOperatingSystem()andforMenuItem()downsampled the Compose scene (192px / 64px) to a single physical size — Windows 32, macOS 44, Linux 24, menu items 16 — before the native layer ever saw the bitmap. High-DPI information was destroyed on the JVM, and native backends could only upscale from there.The JVM now ships the scene-resolution master and each backend owns display-scale downsampling:
IconRenderProperties: newjvmOwnsDownscalingflag (defaulttrue, preserving explicit-size behaviour). The two platform factories set it tofalse;targetWidth/targetHeightbecome the logical size rather than a resample target.renderComposableToIcoBytesemits a real multi-frame ICO (16/20/24/32/40/48/64, clipped to the master — never upscaled) instead of a single-frame PNG wrapper.tray_windows.cloads icons and menu bitmaps atSM_CXSMICONviaLoadImageW, so the shell picks the exact frame for the current DPI (ExtractIconExWkept as fallback).IconPixmappyramid skips levels larger than the source instead of interpolating every 32–128 level from a 24px master; falls back to a single native-size pixmap when the source is smaller than every catalog size.ComposableIconUtils: extracted the shared render/cleanup path intowithRenderedIconso PNG and ICO encoding share scene lifecycle handling.Test plan
Issue436IconScalingTest— asserts the platform factories no longer request JVM downscaling, the PNG keeps the scene master, and the ICO is a multi-frame pyramid with no frame larger than the masterIssue436WindowsTrayE2ETest— Windows-only; renders a tray ICO + menu ICO and drivesWindowsTrayInitializerwith them (no-ops on other platforms)test_sni_pixmap.c+run_pixmap_test.sh— assertssni_pixmap_level_count_for_sourcenever yields an upscaled level; wired into the Linux job ofbuild-natives.yaml./gradlew jvmTest ktlintCheckpasses locally (Windows)