diff --git a/.github/workflows/build-natives.yaml b/.github/workflows/build-natives.yaml index 8df4c55d..585ef6ee 100644 --- a/.github/workflows/build-natives.yaml +++ b/.github/workflows/build-natives.yaml @@ -78,6 +78,10 @@ jobs: test -f build/nativeLibs/linux-${{ matrix.arch }}/libLinuxTray.so ls -la build/nativeLibs/linux-${{ matrix.arch }}/ + - name: IconPixmap pyramid does not upscale (issue #436) + working-directory: src/native/linux + run: bash run_pixmap_test.sh + - name: Upload Linux library uses: actions/upload-artifact@v4 with: diff --git a/build.gradle.kts b/build.gradle.kts index b1754ce9..06765564 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -54,6 +54,7 @@ kotlin { } jvmTest.dependencies { implementation(kotlin("test")) + implementation(compose.desktop.currentOs) } } } diff --git a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/ComposableIconUtils.kt b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/ComposableIconUtils.kt index b2d65a0e..a2a789d5 100644 --- a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/ComposableIconUtils.kt +++ b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/ComposableIconUtils.kt @@ -39,7 +39,9 @@ object ComposableIconUtils { * Renders a Composable to a PNG image and returns the result as a byte array. * This function creates an [ImageComposeScene] based on the provided [IconRenderProperties], * renders the Composable content, and encodes the output into PNG format. - * If scaling is required based on the [IconRenderProperties], the rendered content is scaled before encoding. + * If [IconRenderProperties.requiresScaling] is true, the rendered content is scaled + * to [IconRenderProperties.targetWidth]/[IconRenderProperties.targetHeight] before encoding. + * Otherwise the scene-resolution master is encoded as-is so native backends can downsample. * * @param iconRenderProperties Properties for rendering the icon * @param content The Composable content to render @@ -49,74 +51,10 @@ object ComposableIconUtils { fun renderComposableToPngBytes( iconRenderProperties: IconRenderProperties, content: @Composable () -> Unit, - ): ByteArray { - var scene: ImageComposeScene? = null - var renderedIcon: Image? = null - var scaledBitmap: Bitmap? = null - var scaledImage: Image? = null - - try { - // Try to create and render the scene - try { - scene = - ImageComposeScene( - width = iconRenderProperties.sceneWidth, - height = iconRenderProperties.sceneHeight, - density = iconRenderProperties.sceneDensity, - coroutineContext = Dispatchers.Unconfined, - ) { - content() - } - - renderedIcon = scene.render() - } catch (e: Exception) { - // Log the error but don't modify any system properties - val errorMessage = e.message ?: "Unknown error" - errorln { "[ComposableIconUtils] Failed to render scene: $errorMessage" } - - // Check if it's a DirectX error on Windows - if (errorMessage.contains("DirectX12", ignoreCase = true) || - errorMessage.contains("Failed to choose DirectX12 adapter", ignoreCase = true) - ) { - errorln { "[ComposableIconUtils] DirectX12 not available on this system. Scene rendering failed." } - } - - // Re-throw the exception - let the caller handle it - throw e - } - - val image = - if (iconRenderProperties.requiresScaling) { - scaledBitmap = - Bitmap().apply { - allocN32Pixels(iconRenderProperties.targetWidth, iconRenderProperties.targetHeight) - } - - renderedIcon.scalePixels( - scaledBitmap.peekPixels()!!, - FilterMipmap(FilterMode.LINEAR, MipmapMode.LINEAR), - true, - ) - - scaledImage = Image.makeFromBitmap(scaledBitmap) - scaledImage - } else { - renderedIcon - } - - return image.encodeToPngBytes() - } finally { - // Ensure proper cleanup - try { - scaledImage?.close() - scaledBitmap?.close() - renderedIcon?.close() - scene?.close() - } catch (e: Exception) { - debugln { "[ComposableIconUtils] Error during cleanup: ${e.message}" } - } + ): ByteArray = + withRenderedIcon(iconRenderProperties, content) { image -> + image.encodeToPngBytes() } - } /** * Encodes an [Image] to PNG bytes, tolerating Skiko binary signature changes. @@ -160,8 +98,10 @@ object ComposableIconUtils { /** * Renders a Composable to ICO format bytes. - * Since ICO format is not directly supported by the encoding library, - * this method first renders to PNG and then creates a simple ICO wrapper. + * + * Encodes a multi-frame ICO (16/20/24/32/40/48/64, clipped to the master size) + * so the Windows shell can pick an exact match at any DPI. Frames larger than + * the master are omitted — never upscaled. * * @param iconRenderProperties Properties for rendering the icon * @param content The Composable content to render @@ -171,53 +111,14 @@ object ComposableIconUtils { fun renderComposableToIcoBytes( iconRenderProperties: IconRenderProperties, content: @Composable () -> Unit, - ): ByteArray { - // First render to PNG format (which is supported) - val pngBytes = renderComposableToPngBytes(iconRenderProperties, content) - - // Create a simple ICO format wrapper around the PNG data - // ICO header (6 bytes) + ICO directory entry (16 bytes) + PNG data - val icoHeaderSize = 6 - val icoDirEntrySize = 16 - val icoData = ByteArray(icoHeaderSize + icoDirEntrySize + pngBytes.size) - - // ICO header - icoData[0] = 0 // Reserved, must be 0 - icoData[1] = 0 // Reserved, must be 0 - icoData[2] = 1 // Type: 1 for ICO - icoData[3] = 0 // Type: 1 for ICO (high byte) - icoData[4] = 1 // Number of images - icoData[5] = 0 // Number of images (high byte) - - // ICO directory entry - icoData[6] = iconRenderProperties.targetWidth.toByte() // Width (0 means 256) - icoData[7] = iconRenderProperties.targetHeight.toByte() // Height (0 means 256) - icoData[8] = 0 // Color palette size (0 for no palette) - icoData[9] = 0 // Reserved, must be 0 - icoData[10] = 1 // Color planes - icoData[11] = 0 // Color planes (high byte) - icoData[12] = 32 // Bits per pixel - icoData[13] = 0 // Bits per pixel (high byte) - - // Size of image data in bytes - val dataSize = pngBytes.size - icoData[14] = (dataSize and 0xFF).toByte() - icoData[15] = ((dataSize shr 8) and 0xFF).toByte() - icoData[16] = ((dataSize shr 16) and 0xFF).toByte() - icoData[17] = ((dataSize shr 24) and 0xFF).toByte() - - // Offset to image data - val offset = icoHeaderSize + icoDirEntrySize - icoData[18] = (offset and 0xFF).toByte() - icoData[19] = ((offset shr 8) and 0xFF).toByte() - icoData[20] = ((offset shr 16) and 0xFF).toByte() - icoData[21] = ((offset shr 24) and 0xFF).toByte() - - // Copy PNG data - System.arraycopy(pngBytes, 0, icoData, offset, pngBytes.size) - - return icoData - } + ): ByteArray = + withRenderedIcon(iconRenderProperties, content) { master -> + val frames = + icoFrameSizesFor(master.width, master.height).map { size -> + size to master.encodeScaledPng(size, size) + } + packPngFramesAsIco(frames) + } /** * Creates a temporary file that will be deleted when the JVM exits. @@ -258,4 +159,145 @@ object ComposableIconUtils { System.currentTimeMillis() } } + + private fun withRenderedIcon( + iconRenderProperties: IconRenderProperties, + content: @Composable () -> Unit, + block: (Image) -> T, + ): T { + var scene: ImageComposeScene? = null + var renderedIcon: Image? = null + var scaledBitmap: Bitmap? = null + var scaledImage: Image? = null + try { + try { + scene = + ImageComposeScene( + width = iconRenderProperties.sceneWidth, + height = iconRenderProperties.sceneHeight, + density = iconRenderProperties.sceneDensity, + coroutineContext = Dispatchers.Unconfined, + ) { + content() + } + renderedIcon = scene.render() + } catch (e: Exception) { + val errorMessage = e.message ?: "Unknown error" + errorln { "[ComposableIconUtils] Failed to render scene: $errorMessage" } + if (errorMessage.contains("DirectX12", ignoreCase = true) || + errorMessage.contains("Failed to choose DirectX12 adapter", ignoreCase = true) + ) { + errorln { "[ComposableIconUtils] DirectX12 not available on this system. Scene rendering failed." } + } + throw e + } + + val image = + if (iconRenderProperties.requiresScaling) { + scaledBitmap = + Bitmap().apply { + allocN32Pixels(iconRenderProperties.targetWidth, iconRenderProperties.targetHeight) + } + renderedIcon.scalePixels( + scaledBitmap.peekPixels()!!, + FilterMipmap(FilterMode.LINEAR, MipmapMode.LINEAR), + true, + ) + scaledImage = Image.makeFromBitmap(scaledBitmap) + scaledImage + } else { + renderedIcon + } + + return block(image) + } finally { + try { + scaledImage?.close() + scaledBitmap?.close() + renderedIcon?.close() + scene?.close() + } catch (e: Exception) { + debugln { "[ComposableIconUtils] Error during cleanup: ${e.message}" } + } + } + } + + private fun Image.encodeScaledPng( + width: Int, + height: Int, + ): ByteArray { + if (this.width == width && this.height == height) { + return encodeToPngBytes() + } + var bitmap: Bitmap? = null + var scaled: Image? = null + try { + bitmap = Bitmap().apply { allocN32Pixels(width, height) } + scalePixels( + bitmap.peekPixels()!!, + FilterMipmap(FilterMode.LINEAR, MipmapMode.LINEAR), + true, + ) + scaled = Image.makeFromBitmap(bitmap) + return scaled.encodeToPngBytes() + } finally { + scaled?.close() + bitmap?.close() + } + } +} + +/** Standard Windows small-icon sizes covering 100%–400% DPI. */ +internal val WINDOWS_ICO_FRAME_SIZES = intArrayOf(16, 20, 24, 32, 40, 48, 64) + +/** + * ICO frame sizes to emit for a master of [masterWidth]×[masterHeight]. + * Never larger than the master — the shell upscaling a missing size is better + * than us interpolating past the source. + */ +internal fun icoFrameSizesFor( + masterWidth: Int, + masterHeight: Int, +): List { + val max = minOf(masterWidth, masterHeight) + val sizes = WINDOWS_ICO_FRAME_SIZES.filter { it <= max } + return sizes.ifEmpty { listOf(max.coerceIn(1, 256)) } +} + +/** Packs PNG blobs into a multi-frame ICO container (Vista+ PNG-in-ICO). */ +internal fun packPngFramesAsIco(frames: List>): ByteArray { + require(frames.isNotEmpty()) { "ICO must contain at least one frame" } + val headerSize = 6 + val entrySize = 16 + val dataStart = headerSize + entrySize * frames.size + val total = dataStart + frames.sumOf { it.second.size } + val ico = ByteArray(total) + ico[2] = 1 + ico[4] = frames.size.toByte() + ico[5] = (frames.size shr 8).toByte() + var offset = dataStart + frames.forEachIndexed { index, (size, png) -> + val entry = headerSize + index * entrySize + val dim = if (size >= 256) 0 else size + ico[entry] = dim.toByte() + ico[entry + 1] = dim.toByte() + ico[entry + 4] = 1 + ico[entry + 6] = 32 + writeIntLe(ico, entry + 8, png.size) + writeIntLe(ico, entry + 12, offset) + System.arraycopy(png, 0, ico, offset, png.size) + offset += png.size + } + return ico +} + +private fun writeIntLe( + dest: ByteArray, + index: Int, + value: Int, +) { + dest[index] = (value and 0xFF).toByte() + dest[index + 1] = ((value shr 8) and 0xFF).toByte() + dest[index + 2] = ((value shr 16) and 0xFF).toByte() + dest[index + 3] = ((value shr 24) and 0xFF).toByte() } diff --git a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/IconRenderProperties.kt b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/IconRenderProperties.kt index f7ce94b2..240c347d 100644 --- a/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/IconRenderProperties.kt +++ b/src/jvmMain/kotlin/dev/nucleusframework/composenativetray/utils/IconRenderProperties.kt @@ -6,11 +6,21 @@ import dev.nucleusframework.core.runtime.Platform /** * Properties for rendering a Composable icon. * + * [sceneWidth]/[sceneHeight] are the master bitmap resolution. [targetWidth]/[targetHeight] + * are the logical (unscaled) size the native backend presents, **unless** + * [jvmOwnsDownscaling] is true, in which case they are physical pixels the JVM + * downsamples to before encoding. + * * @property sceneWidth Width of the [androidx.compose.ui.ImageComposeScene] in pixels * @property sceneHeight Height of the [androidx.compose.ui.ImageComposeScene] in pixels - * @property sceneDensity Density for [androidx.compose.ui.ImageComposeScene] - * @property targetWidth Width of the rendered icon in pixels - * @property targetHeight Height of the rendered icon in pixels + * @property sceneDensity Density for [androidx.compose.ui.ImageComposeScene]. + * Controls how Compose `dp` maps into the scene; it is **not** the display scale factor. + * @property targetWidth Logical width (or physical width when [jvmOwnsDownscaling] is true) + * @property targetHeight Logical height (or physical height when [jvmOwnsDownscaling] is true) + * @property jvmOwnsDownscaling When true, the JVM scales the scene to + * [targetWidth]×[targetHeight] before encoding. When false, the JVM emits the + * scene-resolution master and native backends (or a multi-frame ICO) own + * display-scale downsampling. */ data class IconRenderProperties( val sceneWidth: Int = 192, @@ -18,21 +28,24 @@ data class IconRenderProperties( val sceneDensity: Density = Density(2f), val targetWidth: Int = 192, val targetHeight: Int = 192, + val jvmOwnsDownscaling: Boolean = true, ) { - val requiresScaling = sceneWidth != targetWidth || sceneHeight != targetHeight + val requiresScaling = + jvmOwnsDownscaling && (sceneWidth != targetWidth || sceneHeight != targetHeight) companion object { /** * Provides an [IconRenderProperties] configured for the current operating system. * - * This method determines the rendering size based on the current operating system, - * defaulting to specific dimensions for Windows, macOS, and Linux. For unsupported operating - * systems, it defaults to the provided scene width and height. + * The scene is kept at full master resolution. [targetWidth]/[targetHeight] record the + * typical logical tray size per platform (Windows 32, macOS 44 / 18pt@2x, Linux 24); + * the JVM does **not** downsample to those sizes. Native backends pick the display + * scale at draw time (SNI pixmap pyramid, multi-frame ICO, AppKit point size). * * @param sceneWidth Width of the [androidx.compose.ui.ImageComposeScene] in pixels. * @param sceneHeight Height of the [androidx.compose.ui.ImageComposeScene] in pixels. * @param density Density of the [androidx.compose.ui.ImageComposeScene]. - * @return An instance of [IconRenderProperties] with the appropriate target width and height + * @return An instance of [IconRenderProperties] with the appropriate logical size * based on the operating system. */ fun forCurrentOperatingSystem( @@ -54,6 +67,7 @@ data class IconRenderProperties( sceneDensity = density, targetWidth = targetWidth, targetHeight = targetHeight, + jvmOwnsDownscaling = false, ) } @@ -80,16 +94,14 @@ data class IconRenderProperties( /** * Provides an [IconRenderProperties] configured for menu items. * - * Menu items typically require smaller icons than tray icons. - * The default sizes are optimized for each operating system: - * - Windows: 16x16 pixels (standard menu icon size) - * - macOS: 16x16 pixels (NSMenu standard) - * - Linux: 16x16 pixels (GTK menu standard) + * Menu items are presented at 16 logical pixels/points on every platform. The scene + * defaults to 64px (4×) so Retina / 200% DPI menus stay sharp: the JVM keeps that + * master and native backends size it in points (macOS) or DPI-scaled pixels (Windows). * * @param sceneWidth Width of the [androidx.compose.ui.ImageComposeScene] in pixels. Defaults to 64. * @param sceneHeight Height of the [androidx.compose.ui.ImageComposeScene] in pixels. Defaults to 64. * @param density Density of the [androidx.compose.ui.ImageComposeScene]. Defaults to 2.0 for high-DPI support. - * @return An instance of [IconRenderProperties] with the appropriate target width and height for menu items. + * @return An instance of [IconRenderProperties] with a 16px logical size and a high-res master. */ fun forMenuItem( sceneWidth: Int = 64, @@ -110,6 +122,7 @@ data class IconRenderProperties( sceneDensity = density, targetWidth = targetWidth, targetHeight = targetHeight, + jvmOwnsDownscaling = false, ) } } diff --git a/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/tray/Issue436WindowsTrayE2ETest.kt b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/tray/Issue436WindowsTrayE2ETest.kt new file mode 100644 index 00000000..a0763b0c --- /dev/null +++ b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/tray/Issue436WindowsTrayE2ETest.kt @@ -0,0 +1,200 @@ +package dev.nucleusframework.composenativetray.tray + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color +import dev.nucleusframework.composenativetray.tray.impl.WindowsTrayInitializer +import dev.nucleusframework.composenativetray.utils.ComposableIconUtils +import dev.nucleusframework.composenativetray.utils.IconRenderProperties +import java.io.File +import java.nio.ByteBuffer +import java.nio.ByteOrder +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.test.fail + +/** + * End-to-end coverage for #436 on Windows: the JVM must hand the native layer a + * scene-resolution PNG and a multi-frame ICO, and [WindowsTrayInitializer] must + * accept both as a live notification-area icon plus a menu-item bitmap. + */ +class Issue436WindowsTrayE2ETest { + @Test + fun `native tray loads a hidpi master ico and a hidpi menu icon`() { + if (!isWindows) return + + val trayProps = IconRenderProperties.forCurrentOperatingSystem() + val menuProps = IconRenderProperties.forMenuItem() + val trayIco = + try { + ComposableIconUtils.renderComposableToIcoBytes(trayProps) { SolidRedIcon() } + } catch (t: Throwable) { + fail("failed to render tray ICO master: ${t.message}") + } + val menuIco = + try { + ComposableIconUtils.renderComposableToIcoBytes(menuProps) { SolidBlueIcon() } + } catch (t: Throwable) { + fail("failed to render menu ICO master: ${t.message}") + } + val trayPng = + try { + ComposableIconUtils.renderComposableToPngBytes(trayProps) { SolidRedIcon() } + } catch (t: Throwable) { + fail("failed to render tray PNG master: ${t.message}") + } + + val (pngW, pngH) = pngSize(trayPng) + assertEquals(trayProps.sceneWidth, pngW) + assertEquals(trayProps.sceneHeight, pngH) + + val trayFrames = icoFrameSizes(trayIco) + assertTrue(trayFrames.size > 1, "tray ICO is not a DPI pyramid: $trayFrames") + assertTrue(16 in trayFrames && 32 in trayFrames, "tray ICO missing 16/32: $trayFrames") + assertTrue(trayFrames.max() >= 64, "tray ICO missing ≥64px frame: $trayFrames") + + val menuFrames = icoFrameSizes(menuIco) + assertTrue(menuFrames.isNotEmpty(), "menu ICO has no frames") + assertTrue( + menuFrames.all { it <= menuProps.sceneWidth }, + "menu ICO upscaled past the 64px master: $menuFrames", + ) + assertTrue(16 in menuFrames, "menu ICO missing the 16px frame: $menuFrames") + + val trayFile = kotlin.io.path.createTempFile(prefix = "issue436-tray-", suffix = ".ico").toFile() + trayFile.writeBytes(trayIco) + trayFile.deleteOnExit() + + val (smCxSmallIcon, extracted) = probeLoadImageSize(trayFile) + assertTrue(extracted > 0, "LoadImageW failed on the multi-frame ICO") + assertEquals( + smCxSmallIcon, + extracted, + "LoadImageW at SM_CXSMICON=$smCxSmallIcon picked ${extracted}px — the ICO pyramid must supply that size", + ) + + val id = "issue-436-e2e" + try { + WindowsTrayInitializer.initialize( + id = id, + iconPath = trayFile.absolutePath, + tooltip = "issue-436", + menuContent = { + Item( + label = "HiDPI", + iconContent = { SolidBlueIcon() }, + iconRenderProperties = menuProps, + ) {} + }, + ) + WindowsTrayInitializer.refreshPosition(id) + } finally { + WindowsTrayInitializer.dispose(id) + } + } + + private fun pngSize(png: ByteArray): Pair { + require(png.size >= 24) { "PNG too short: ${png.size}" } + val buf = ByteBuffer.wrap(png, 16, 8).order(ByteOrder.BIG_ENDIAN) + return buf.int to buf.int + } + + private fun icoFrameSizes(ico: ByteArray): List { + require(ico.size >= 6) { "ICO too short: ${ico.size}" } + val count = ico[4].toInt() and 0xFF + return (0 until count).map { i -> + val w = ico[6 + i * 16].toInt() and 0xFF + if (w == 0) 256 else w + } + } + + private val isWindows: Boolean + get() = System.getProperty("os.name").orEmpty().lowercase().contains("win") + + /** + * Asks user32 to load the ICO at [SM_CXSMICON] — the same size the tray + * and menu-item bitmaps request after the native DPI fix. + */ + private fun probeLoadImageSize(ico: File): Pair { + val csc = + File("""C:\Windows\Microsoft.NET\Framework64\v4.0.30319\csc.exe""") + if (!csc.isFile) { + fail("csc.exe not found; cannot probe LoadImageW against the ICO") + } + val dir = ico.parentFile + val cs = File(dir, "Issue436IcoProbe.cs") + val exe = File(dir, "Issue436IcoProbe.exe") + cs.writeText( + """ + using System; + using System.Runtime.InteropServices; + class Issue436IcoProbe { + [DllImport("user32.dll", CharSet=CharSet.Unicode)] + static extern IntPtr LoadImage(IntPtr h, string n, uint t, int cx, int cy, uint f); + [DllImport("user32.dll")] static extern bool DestroyIcon(IntPtr h); + [DllImport("user32.dll")] static extern bool GetIconInfo(IntPtr h, out ICONINFO i); + [DllImport("gdi32.dll")] static extern int GetObject(IntPtr h, int n, out BITMAP b); + [DllImport("gdi32.dll")] static extern bool DeleteObject(IntPtr h); + [DllImport("user32.dll")] static extern int GetSystemMetrics(int n); + [StructLayout(LayoutKind.Sequential)] + struct ICONINFO { public bool fIcon; public int xHotspot; public int yHotspot; public IntPtr hbmMask; public IntPtr hbmColor; } + [StructLayout(LayoutKind.Sequential)] + struct BITMAP { public int bmType; public int bmWidth; public int bmHeight; public int bmWidthBytes; public short bmPlanes; public short bmBitsPixel; public IntPtr bmBits; } + static int Main(string[] args) { + int sm = GetSystemMetrics(49); + IntPtr h = LoadImage(IntPtr.Zero, args[0], 1, sm, sm, 0x0010); + if (h == IntPtr.Zero) { Console.WriteLine("SM_CXSMICON="+sm+" EXTRACTED=0"); return 2; } + ICONINFO info; GetIconInfo(h, out info); + BITMAP bmp; GetObject(info.hbmColor, Marshal.SizeOf(typeof(BITMAP)), out bmp); + Console.WriteLine("SM_CXSMICON="+sm+" EXTRACTED="+bmp.bmWidth); + if (info.hbmColor != IntPtr.Zero) DeleteObject(info.hbmColor); + if (info.hbmMask != IntPtr.Zero) DeleteObject(info.hbmMask); + DestroyIcon(h); + return 0; + } + } + """.trimIndent(), + ) + val compile = + ProcessBuilder(csc.absolutePath, "/nologo", "/out:${exe.absolutePath}", cs.absolutePath) + .redirectErrorStream(true) + .start() + val compileOut = compile.inputStream.bufferedReader().readText() + check(compile.waitFor() == 0) { "csc failed: $compileOut" } + val run = + ProcessBuilder(exe.absolutePath, ico.absolutePath) + .redirectErrorStream(true) + .start() + val output = run.inputStream.bufferedReader().readText().trim() + check(run.waitFor() == 0) { "icon probe failed: $output" } + val sm = + Regex("""SM_CXSMICON=(\d+)""") + .find(output) + ?.groupValues + ?.get(1) + ?.toInt() + ?: fail("probe output missing SM_CXSMICON: $output") + val extracted = + Regex("""EXTRACTED=(\d+)""") + .find(output) + ?.groupValues + ?.get(1) + ?.toInt() + ?: fail("probe output missing EXTRACTED: $output") + return sm to extracted + } +} + +@Composable +private fun SolidRedIcon() { + Box(Modifier.fillMaxSize().background(Color.Red)) +} + +@Composable +private fun SolidBlueIcon() { + Box(Modifier.fillMaxSize().background(Color.Blue)) +} diff --git a/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/utils/Issue436IconScalingTest.kt b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/utils/Issue436IconScalingTest.kt new file mode 100644 index 00000000..2aa58cf2 --- /dev/null +++ b/src/jvmTest/kotlin/dev/nucleusframework/composenativetray/utils/Issue436IconScalingTest.kt @@ -0,0 +1,225 @@ +package dev.nucleusframework.composenativetray.utils + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color +import java.nio.ByteBuffer +import java.nio.ByteOrder +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.test.fail + +/** + * Regression coverage for #436: `forCurrentOperatingSystem()` / `forMenuItem()` used to + * downsample the 192px (resp. 64px) Compose scene to a single physical pixel 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. + * + * Expected: + * - BEFORE the fix: PNG is 24/32/44 (tray) or 16 (menu); ICO has a single frame. + * - AFTER the fix: PNG keeps the scene-resolution master; ICO is a DPI pyramid + * downsampled from that master, never an upscale. + */ +class Issue436IconScalingTest { + @Test + fun `forCurrentOperatingSystem does not ask the JVM to crush the scene master`() { + val props = IconRenderProperties.forCurrentOperatingSystem() + assertFalse( + props.jvmOwnsDownscaling, + "forCurrentOperatingSystem() must not downsample on the JVM", + ) + assertFalse( + props.requiresScaling, + "forCurrentOperatingSystem() must leave downscaling to native backends " + + "(requiresScaling=${props.requiresScaling}, " + + "scene=${props.sceneWidth}x${props.sceneHeight}, " + + "target=${props.targetWidth}x${props.targetHeight})", + ) + } + + @Test + fun `forMenuItem does not ask the JVM to crush the scene master`() { + val props = IconRenderProperties.forMenuItem() + assertFalse( + props.jvmOwnsDownscaling, + "forMenuItem() must not downsample on the JVM", + ) + assertFalse( + props.requiresScaling, + "forMenuItem() must leave downscaling to native backends " + + "(requiresScaling=${props.requiresScaling}, " + + "scene=${props.sceneWidth}x${props.sceneHeight}, " + + "target=${props.targetWidth}x${props.targetHeight})", + ) + } + + @Test + fun `ico pyramid never upscales past the master`() { + assertEquals(listOf(16, 20, 24, 32, 40, 48, 64), icoFrameSizesFor(192, 192)) + assertEquals(listOf(16, 20, 24), icoFrameSizesFor(24, 24)) + assertEquals(listOf(8), icoFrameSizesFor(8, 8)) + } + + @Test + fun `ico container writes one directory entry per frame`() { + val png = MINIMAL_PNG + val ico = packPngFramesAsIco(listOf(16 to png, 32 to png, 64 to png)) + assertEquals(listOf(16, 32, 64), icoFrames(ico).map { it.size }) + } + + @Test + fun `tray png keeps the scene-resolution master instead of a fixed 24-32-44 px icon`() { + val props = IconRenderProperties.forCurrentOperatingSystem() + val png = renderSolid(props) + val (width, height) = pngSize(png) + assertEquals( + props.sceneWidth, + width, + "tray PNG width must be the scene master, not the OS physical target " + + "(got ${width}x$height, scene=${props.sceneWidth}, target=${props.targetWidth})", + ) + assertEquals(props.sceneHeight, height) + assertTrue( + width >= 128 && height >= 128, + "tray master must be large enough for the Linux 128px SNI pyramid and 3x Retina " + + "(got ${width}x$height)", + ) + } + + @Test + fun `menu-item png keeps the scene-resolution master instead of 16 px`() { + val props = IconRenderProperties.forMenuItem() + val png = renderSolid(props) + val (width, height) = pngSize(png) + assertEquals( + props.sceneWidth, + width, + "menu PNG width must be the scene master, not 16px " + + "(got ${width}x$height, scene=${props.sceneWidth}, target=${props.targetWidth})", + ) + assertEquals(props.sceneHeight, height) + assertTrue( + width >= 48 && height >= 48, + "menu master must cover 16pt at ≥3x (got ${width}x$height)", + ) + } + + @Test + fun `windows ico is a dpi pyramid downsampled from the master not a single 32px frame`() { + val props = IconRenderProperties.forCurrentOperatingSystem() + val ico = ComposableIconUtils.renderComposableToIcoBytes(props) { SolidRedIcon() } + val frames = icoFrames(ico) + + assertTrue( + frames.size > 1, + "ICO must contain multiple DPI frames so the shell can pick an exact match, " + + "got ${frames.map { it.size }}", + ) + val sizes = frames.map { it.size } + assertTrue(16 in sizes, "ICO is missing the 16px (100% scale) frame: $sizes") + assertTrue(32 in sizes, "ICO is missing the 32px (200% scale) frame: $sizes") + assertTrue( + sizes.max() >= 64, + "ICO must include at least a 64px frame for 200%+ / SM_CXSMICON at high DPI, " + + "got $sizes", + ) + assertTrue( + sizes.all { it <= props.sceneWidth && it <= props.sceneHeight }, + "ICO must never upscale past the scene master (scene=${props.sceneWidth}, frames=$sizes)", + ) + frames.forEach { frame -> + assertEquals( + frame.size, + frame.pngWidth, + "ICO directory size ${frame.size} does not match PNG IHDR ${frame.pngWidth}", + ) + assertEquals(frame.size, frame.pngHeight) + } + } + + @Test + fun `explicit jvm target still downsamples when the caller opts in`() { + val props = + IconRenderProperties( + sceneWidth = 64, + sceneHeight = 64, + targetWidth = 16, + targetHeight = 16, + ) + assertTrue(props.requiresScaling, "explicit target != scene must still scale on the JVM") + val png = renderSolid(props) + val (width, height) = pngSize(png) + assertEquals(16, width) + assertEquals(16, height) + } + + private fun renderSolid(props: IconRenderProperties): ByteArray = + try { + ComposableIconUtils.renderComposableToPngBytes(props) { SolidRedIcon() } + } catch (t: Throwable) { + fail("ImageComposeScene failed to render the icon master: ${t.message}") + } + + private fun pngSize(png: ByteArray): Pair { + require(png.size >= 24) { "PNG too short: ${png.size}" } + require(png[0] == 0x89.toByte() && png[1] == 0x50.toByte()) { "not a PNG" } + val buf = ByteBuffer.wrap(png, 16, 8).order(ByteOrder.BIG_ENDIAN) + return buf.int to buf.int + } + + private data class IcoFrame( + val size: Int, + val pngWidth: Int, + val pngHeight: Int, + ) + + private fun icoFrames(ico: ByteArray): List { + require(ico.size >= 6) { "ICO too short: ${ico.size}" } + require(ico[2].toInt() and 0xFF == 1) { "not an ICO (type=${ico[2]})" } + val count = ico[4].toInt() and 0xFF + return (0 until count).map { i -> + val entry = 6 + i * 16 + val dirSize = ico[entry].toInt() and 0xFF + val size = if (dirSize == 0) 256 else dirSize + val dataSize = + (ico[entry + 8].toInt() and 0xFF) or + ((ico[entry + 9].toInt() and 0xFF) shl 8) or + ((ico[entry + 10].toInt() and 0xFF) shl 16) or + ((ico[entry + 11].toInt() and 0xFF) shl 24) + val offset = + (ico[entry + 12].toInt() and 0xFF) or + ((ico[entry + 13].toInt() and 0xFF) shl 8) or + ((ico[entry + 14].toInt() and 0xFF) shl 16) or + ((ico[entry + 15].toInt() and 0xFF) shl 24) + require(offset >= 0 && offset + dataSize <= ico.size) { + "ICO frame $i offset=$offset size=$dataSize exceeds file ${ico.size}" + } + val png = ico.copyOfRange(offset, offset + dataSize) + val (pngW, pngH) = pngSize(png) + IcoFrame(size = size, pngWidth = pngW, pngHeight = pngH) + } + } +} + +@Composable +private fun SolidRedIcon() { + Box(Modifier.fillMaxSize().background(Color.Red)) +} + +/** Minimal valid 1×1 RGBA PNG (same bytes as the Linux SNI concurrency fixture). */ +private val MINIMAL_PNG = + byteArrayOf( + 0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, + 0x1F, 0x15.toByte(), 0xC4.toByte(), 0x89.toByte(), + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9C.toByte(), + 0x62, 0xF8.toByte(), 0xCF.toByte(), 0xC0.toByte(), 0x00, 0x00, 0x00, 0x03, 0x00, 0x01, + 0x73, 0xF8.toByte(), 0x6C, 0xC4.toByte(), + 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, 0x44, 0xAE.toByte(), 0x42, 0x60, 0x82.toByte(), + ) diff --git a/src/native/linux/run_pixmap_test.sh b/src/native/linux/run_pixmap_test.sh new file mode 100644 index 00000000..3be86c55 --- /dev/null +++ b/src/native/linux/run_pixmap_test.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# Build and run the issue #436 IconPixmap pyramid regression test. +# Header-only: does not need sd-bus or a session bus. + +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +BIN="$SCRIPT_DIR/test_sni_pixmap" + +echo "Compiling pixmap pyramid test..." +gcc -O2 -g -Wall -Wextra -Werror \ + -I "$SCRIPT_DIR" \ + "$SCRIPT_DIR/test_sni_pixmap.c" \ + -o "$BIN" + +"$BIN" +status=$? +rm -f "$BIN" +exit $status diff --git a/src/native/linux/sni.c b/src/native/linux/sni.c index ce6cdb6b..edfd7084 100644 --- a/src/native/linux/sni.c +++ b/src/native/linux/sni.c @@ -45,9 +45,7 @@ #define MAX_MENU_ITEMS 512 #define DCLICK_INTERVAL 500 /* ms */ -/* Icon target sizes for multi-resolution pixmap (matches Go implementation) */ -static const int ICON_SIZES[] = {16, 22, 24, 32, 48, 64, 128}; -#define NUM_ICON_SIZES (sizeof(ICON_SIZES) / sizeof(ICON_SIZES[0])) + /* ========================================================================== */ /* Menu item */ @@ -297,7 +295,28 @@ static void free_pixmap_list(pixmap_list *pl) { pl->count = 0; } -/* Build multi-resolution pixmaps from raw PNG/JPG data. */ +static int append_pixmap(pixmap_list *pl, const uint8_t *src, int src_w, int src_h, int s) { + uint8_t *resized = malloc((size_t)s * s * 4); + if (!resized) return 0; + + stbir_resize_uint8_linear(src, src_w, src_h, src_w * 4, + resized, s, s, s * 4, STBIR_RGBA); + + uint8_t *argb = rgba_to_argb32_be(resized, s, s); + free(resized); + if (!argb) return 0; + + pl->entries[pl->count].width = s; + pl->entries[pl->count].height = s; + pl->entries[pl->count].data = argb; + pl->entries[pl->count].data_len = (size_t)s * s * 4; + pl->count++; + return 1; +} + +/* Build multi-resolution pixmaps from raw PNG/JPG data. + * Never upscales past the source: a 24px JVM master used to make every + * 32–128 level an interpolation. The JVM now ships the scene master. */ static pixmap_list build_pixmaps(const uint8_t *data, size_t len) { pixmap_list pl = {NULL, 0}; if (!data || len == 0) return pl; @@ -306,26 +325,22 @@ static pixmap_list build_pixmaps(const uint8_t *data, size_t len) { uint8_t *src = stbi_load_from_memory(data, (int)len, &src_w, &src_h, &channels, 4); if (!src) return pl; - pl.entries = calloc(NUM_ICON_SIZES, sizeof(pixmap)); - if (!pl.entries) { stbi_image_free(src); return pl; } - - for (size_t i = 0; i < NUM_ICON_SIZES; i++) { - int s = ICON_SIZES[i]; - uint8_t *resized = malloc((size_t)s * s * 4); - if (!resized) continue; + int src_min = src_w < src_h ? src_w : src_h; + int levels = sni_pixmap_level_count_for_source(src_w, src_h); + if (levels <= 0) { stbi_image_free(src); return pl; } - stbir_resize_uint8_linear(src, src_w, src_h, src_w * 4, - resized, s, s, s * 4, STBIR_RGBA); + pl.entries = calloc((size_t)levels, sizeof(pixmap)); + if (!pl.entries) { stbi_image_free(src); return pl; } - uint8_t *argb = rgba_to_argb32_be(resized, s, s); - free(resized); - if (!argb) continue; + for (size_t i = 0; i < SNI_NUM_ICON_SIZES; i++) { + int s = SNI_ICON_SIZES[i]; + if (s > src_min) continue; + if (pl.count >= levels) break; + append_pixmap(&pl, src, src_w, src_h, s); + } - pl.entries[pl.count].width = s; - pl.entries[pl.count].height = s; - pl.entries[pl.count].data = argb; - pl.entries[pl.count].data_len = (size_t)s * s * 4; - pl.count++; + if (pl.count == 0 && src_min > 0) { + append_pixmap(&pl, src, src_w, src_h, src_min); } stbi_image_free(src); diff --git a/src/native/linux/sni.h b/src/native/linux/sni.h index d1ee7b02..adc8a555 100644 --- a/src/native/linux/sni.h +++ b/src/native/linux/sni.h @@ -14,6 +14,24 @@ extern "C" { #endif +/* IconPixmap pyramid sizes (SNI). Levels larger than the source bitmap are + * omitted so the panel never receives an upscaled frame. */ +static const int SNI_ICON_SIZES[] = {16, 22, 24, 32, 48, 64, 128}; +#define SNI_NUM_ICON_SIZES (sizeof(SNI_ICON_SIZES) / sizeof(SNI_ICON_SIZES[0])) + +/* How many pyramid levels [src_w]×[src_h] can fill without upscaling. + * Returns 1 when the source is smaller than every catalog size (emit native). */ +static inline int sni_pixmap_level_count_for_source(int src_w, int src_h) { + int src_min = src_w < src_h ? src_w : src_h; + int n = 0; + size_t i; + if (src_min <= 0) return 0; + for (i = 0; i < SNI_NUM_ICON_SIZES; i++) { + if (SNI_ICON_SIZES[i] <= src_min) n++; + } + return n > 0 ? n : 1; +} + /* Opaque tray handle */ typedef struct sni_tray sni_tray; diff --git a/src/native/linux/test_sni_pixmap.c b/src/native/linux/test_sni_pixmap.c new file mode 100644 index 00000000..ab29848f --- /dev/null +++ b/src/native/linux/test_sni_pixmap.c @@ -0,0 +1,43 @@ +/* + * test_sni_pixmap.c – regression test for issue #436. + * + * The SNI IconPixmap pyramid must never upscale past the source bitmap. + * A 24px JVM master used to make every 32–128 level an interpolation; the + * JVM now ships the 192px scene master and this helper must cap the pyramid. + * + * Expected: + * - BEFORE the fix: a 24px source still produced 7 levels (16..128). + * - AFTER the fix: 24px → 3 levels (16,22,24); 192px → all 7; 8px → 1. + */ + +#include "sni.h" + +#include + +int main(void) { + int n24 = sni_pixmap_level_count_for_source(24, 24); + int n192 = sni_pixmap_level_count_for_source(192, 192); + int n8 = sni_pixmap_level_count_for_source(8, 8); + int n0 = sni_pixmap_level_count_for_source(0, 0); + + if (n24 != 3) { + fprintf(stderr, "FAIL: 24px source produced %d levels, expected 3 (16,22,24)\n", n24); + return 1; + } + if (n192 != (int)SNI_NUM_ICON_SIZES) { + fprintf(stderr, "FAIL: 192px source produced %d levels, expected %d\n", + n192, (int)SNI_NUM_ICON_SIZES); + return 1; + } + if (n8 != 1) { + fprintf(stderr, "FAIL: 8px source produced %d levels, expected 1 (native fallback)\n", n8); + return 1; + } + if (n0 != 0) { + fprintf(stderr, "FAIL: 0px source produced %d levels, expected 0\n", n0); + return 1; + } + + printf("PASS: pixmap pyramid 24→%d 192→%d 8→%d 0→%d\n", n24, n192, n8, n0); + return 0; +} diff --git a/src/native/macos/tray.swift b/src/native/macos/tray.swift index 0f87b72f..ee91d1b4 100644 --- a/src/native/macos/tray.swift +++ b/src/native/macos/tray.swift @@ -235,12 +235,10 @@ private func nativeMenu(from menuPtr: UnsafeMutableRawPointer, statusItem: NSSta item.state = checked ? .on : .off item.representedObject = currentPtr - // Ajouter l'icône si disponible + // Menu icons are 16pt; the JVM ships a ≥3x master (see forMenuItem). if let iconPath = iconPathPtr.flatMap({ String(cString: $0) }), let image = NSImage(contentsOfFile: iconPath) { - // Redimensionner l'icône à une taille appropriée pour le menu - let menuIconSize = NSSize(width: 16, height: 16) - image.size = menuIconSize + image.size = NSSize(width: 16, height: 16) item.image = image } diff --git a/src/native/windows/tray_windows.c b/src/native/windows/tray_windows.c index b3201d60..c381a9fe 100644 --- a/src/native/windows/tray_windows.c +++ b/src/native/windows/tray_windows.c @@ -120,6 +120,7 @@ static void destroy_ctx(TrayContext *ctx); static HMENU tray_menu_item(struct tray_menu_item *m, UINT *id); static void ensure_critical_section(void); static HBITMAP load_icon_bitmap(const char *icon_path); +static int small_icon_px(void); /* -------------------------------------------------------------------------- */ /* Critical section helper */ @@ -269,6 +270,13 @@ static HBITMAP bitmap_from_icon(HICON hIcon, int cx, int cy) return hbmp; } +/* SM_CXSMICON follows system DPI (16@96dpi, 20@120dpi, 24@144dpi, 32@192dpi). */ +static int small_icon_px(void) +{ + int cx = GetSystemMetrics(SM_CXSMICON); + return cx > 0 ? cx : 16; +} + /* ------------------------------------------------------------------ */ /* Generic loading of icon/bitmap from disk → ARGB bitmap */ /* ------------------------------------------------------------------ */ @@ -276,33 +284,35 @@ static HBITMAP load_icon_bitmap(const char *icon_path) { if (!icon_path || !*icon_path) return NULL; + int size = small_icon_px(); + /* Convert UTF-8 path to Wide */ LPWSTR wpath = utf8_to_wide(icon_path); if (!wpath) return NULL; - /* 1st: try direct .bmp/.png as 32-bit DIB */ + /* 1st: try direct .bmp/.png as 32-bit DIB at the DPI-scaled size */ HBITMAP hbmp = (HBITMAP)LoadImageW( NULL, wpath, IMAGE_BITMAP, - 16, 16, - LR_LOADFROMFILE | LR_CREATEDIBSECTION | LR_DEFAULTSIZE + size, size, + LR_LOADFROMFILE | LR_CREATEDIBSECTION ); if (hbmp) { free(wpath); return hbmp; } - /* 2nd: try .ico → ARGB conversion */ + /* 2nd: try .ico → ARGB conversion, picking the matching ICO frame */ HICON hIcon = (HICON)LoadImageW( NULL, wpath, IMAGE_ICON, - 16, 16, - LR_LOADFROMFILE | LR_DEFAULTSIZE + size, size, + LR_LOADFROMFILE ); free(wpath); if (hIcon) { - hbmp = bitmap_from_icon(hIcon, 16, 16); + hbmp = bitmap_from_icon(hIcon, size, size); DestroyIcon(hIcon); } return hbmp; @@ -587,12 +597,16 @@ void tray_update(struct tray *tray) UINT id = ID_TRAY_FIRST; ctx->hmenu = tray_menu_item(tray->menu, &id); - /* Icon */ + /* Icon: request SM_CXSMICON so a multi-frame ICO yields an exact DPI match. */ HICON icon = NULL; if (tray->icon_filepath && *tray->icon_filepath) { LPWSTR wpath = utf8_to_wide(tray->icon_filepath); if (wpath) { - ExtractIconExW(wpath, 0, NULL, &icon, 1); + int size = small_icon_px(); + icon = (HICON)LoadImageW(NULL, wpath, IMAGE_ICON, size, size, LR_LOADFROMFILE); + if (!icon) { + ExtractIconExW(wpath, 0, NULL, &icon, 1); + } free(wpath); } }