Skip to content

fix: drop icp4/icp5 icns slices that macOS renders scrambled - #479

Merged
shannah merged 1 commit into
masterfrom
claude/mac-icns-small-icon-fix
Sep 8, 2026
Merged

fix: drop icp4/icp5 icns slices that macOS renders scrambled#479
shannah merged 1 commit into
masterfrom
claude/mac-icns-small-icon-fix

Conversation

@shannah

@shannah shannah commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the corrupted small app icon reported against 6.1.0-dev.0 — the icon renders correctly in the Finder preview pane but as garbage in the Finder list view and the Force Quit dialog.

This is a regression from #475, which switched MacBundler from writing a single icns slice to writing the whole size family. The family it wrote included icp4 (16x16) and icp5 (32x32).

Root cause

macOS reads a PNG payload in the icp4/icp5 slice types correctly from a standalone icns, but renders it scrambled when the icns is an app bundle's icon. That split is exactly the reported symptom: the Finder preview pane reads the file standalone and looks right, while the list view and Force Quit read the bundle's icon and don't.

This is a long-standing, independently reported macOS behaviour, not a malformed file on our side — see the png2icons issue ("Icons look scrambled in macOS Finder list view"), whose adopted resolution was to "simply exclude those types from the generation", because "the image interpolation in the Finder (or the OS) manages to display correct images at all sizes, even if the types mentioned above are missing in the ICNS file."

I verified the generated icns is structurally sound before concluding this — every slice's PNG payload dimensions match the size its OSType declares, all PNG with alpha. So the bug is which types we emit, not how we emit them.

Before #475 only one slice was written (typically ic09 at 512px), so macOS downscaled it for the small sizes and they looked fine — which is why this regression is new.

Fix

Emit ic11 (16pt@2x, 32px) and ic12 (32pt@2x, 64px) as the smallest slices instead of icp4/icp5. Those two are unambiguously PNG-only, serve the small sizes natively on retina displays, and macOS interpolates the 16pt/32pt non-retina renderings from them.

The bound on the family — "don't emit a slice larger than the source, upscaling would tag a blurry slice as native" — was previously derived by looking the source's own OSType up in the emit table. That coupling breaks once icp4/icp5 leave the table: a 16x16 or 32x32 source would no longer resolve a bound. The size now comes from the source's own slice type (IcnsType.of(osType).getWidth()), independent of what we emit.

A source too small to fill any emitted slice (a 16x16 icon) keeps its single native slice, since an icns with no icon in it at all would be worse than a scrambled one.

The icns writing is extracted into a package-private writeIcns(...) so it can be tested directly.

Testing

New MacBundlerIconTest (6 tests, all passing) covering:

  • no icp4/icp5/icp6 slice is ever emitted
  • ic11 and ic12 are present, and the full emitted set is exactly [ic11, ic12, ic07, ic08, ic13, ic09, ic14, ic10] for a 1024px source
  • every slice's payload dimensions match the size its OSType declares
  • a 128px source emits only [ic11, ic12, ic07] — nothing upscaled
  • a 16px source falls back to its single native slice rather than an empty icns
  • the intermediate icon-*.png thumbnails are cleaned up

I confirmed the tests catch the regression: against the pre-fix slice table 3 of the 6 fail, naming icp4/icp5 in the diff. Full shared suite: 234 tests, 0 failures.

Not fixed here

The other half of the report — the app icon still being too large in the Cmd-Tab switcher — is not addressed by this PR, and I don't believe it is fixable by changing which icns slices we write. #475's premise was that macOS 26 shrinks a single-slice icns onto a grey plate, so writing the full family would correct the size; the reporter confirms the size is unchanged with the family present. The switcher/Dock sizing on macOS 26 is governed by Tahoe's icon plate and masking, which expects either an Icon Composer .icon asset or source artwork authored with the padding Apple's icon grid assumes — neither of which follows from the icns slice set. That's separate work; happy to dig into it if wanted.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KQuAnJXfX5ZvXTgnHWdPqs


Generated by Claude Code

The full icns size family added in #475 included icp4 (16x16) and icp5
(32x32). macOS reads a PNG payload in those two slice types correctly from
a standalone icns, but renders it scrambled when the icns is an app
bundle's icon, so the Finder preview pane showed the icon correctly while
the Finder list view and the Force Quit dialog showed garbage.

Emit ic11 (16pt@2x) and ic12 (32pt@2x) as the smallest slices instead.
Those are unambiguously PNG-only, serve the small sizes natively on retina
displays, and macOS interpolates the 16pt/32pt non-retina renderings from
them.

The source size that bounds the family is now read from the source's own
slice type rather than looked up in the emit table, so dropping icp4/icp5
does not also lose the bound for 16x16 and 32x32 sources. A source too
small to fill any emitted slice keeps its single native slice, since an
icns with no icon at all would be worse.

Adds MacBundlerIconTest, which fails on the pre-fix slice table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQuAnJXfX5ZvXTgnHWdPqs

shannah commented Sep 8, 2026

Copy link
Copy Markdown
Owner Author

CI: build-win-arm (21) failed — not caused by this PR.

Job log. The Maven reactor succeeded in that job (jdeploy-shared, jdeploy-installer, jdeploy-cli all SUCCESS, BUILD SUCCESS at 18:21:09). The failure comes afterwards, in the tests/ integration step, installing a published app over the network:

Installing Hello Commands 0.1.4...
Installation failed: Application update timed out after 30 seconds

Reasons this isn't this PR's:

  1. The diff can't reach that code path. It touches only MacBundler.java (macOS .icns slice generation) and adds MacBundlerIconTest. MacBundler is not invoked when installing an app on Windows.
  2. The same suite passes on Windows x64. build-win-x64 at JDK 8 and 17 ran the identical build_and_test.sh — including this same "Hello Commands" install — and both passed on this commit. Only the windows-11-arm runner failed.
  3. The same check passed on the same base. build-win-arm (21) was green on Publish installers to current repo for cross-repo tag releases; mark branch releases as pre-releases #478 an hour earlier, off the same base commit (1340dc4).

The failure is a 30-second network timeout downloading a remote app on the ARM runner, so there is no fix to port — nothing exists to port. I'll re-run this job once to confirm, as soon as the workflow run finishes (build-win-x64 (11) is still in progress, and GitHub only allows re-running failed jobs on a completed run). If it fails a second time I'll treat it as real and dig in rather than re-running again.

Every other check on this commit is green.


Generated by Claude Code

shannah commented Sep 8, 2026

Copy link
Copy Markdown
Owner Author

Correction to the previous comment: I can't perform that re-run — rerun-failed-jobs returns 403 Resource not accessible by integration for my token. So the confirming re-run needs a maintainer to click Re-run failed jobs on the run, or it'll happen naturally on the next push to this branch.

I'm not making the timeout more robust here — that would mean changing the installer's update timeout or the integration-test harness on windows-11-arm, which is unrelated to a macOS .icns change and would widen this PR beyond its purpose. The analysis above stands on its own: the Maven build passed, the identical suite passed on Windows x64 for this commit, and the same check was green on the same base an hour earlier.

I'll keep watching this PR until it's merged or closed.


Generated by Claude Code

shannah commented Sep 8, 2026

Copy link
Copy Markdown
Owner Author

Resolved: the run was re-run and build-win-arm (21) passed on the same commit (663679c), with no code change in between. That confirms the transient-network-timeout diagnosis above. All 19 checks are now green and the PR is mergeable.


Generated by Claude Code

@shannah
shannah marked this pull request as ready for review September 8, 2026 18:33
@shannah
shannah merged commit 3c3abf5 into master Sep 8, 2026
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants