Skip to content

fix(notifications): keep the colors that carry a small icon's shape - #54

Open
A-nony-mous wants to merge 1 commit into
jimrandomh:mainfrom
A-nony-mous:fix/notification-icons
Open

A-nony-mous wants to merge 1 commit into
jimrandomh:mainfrom
A-nony-mous:fix/notification-icons

Conversation

@A-nony-mous

Copy link
Copy Markdown
Contributor

Notification icons reached the glasses as blank(or green) blocks for the apps whose small icon is a filled color bitmap : the loader tinted every small icon white on the assumption that a small icon is an alpha template, which does hold for well-developed app. Tinting kept the alpha and discarded the colors, and the alpha was solid, so the whole icon became one flat white(green) square.

Control group Experimental group
control exp

Some app still looks terrible but better.

The debug-icon dump also grows without bound: one file per icon per index, written on every refresh and never removed, in release builds nobody can pull from. Write it in debug builds only and sweep the directory once per process.

@A-nony-mous
A-nony-mous force-pushed the fix/notification-icons branch from 7ab7f8c to 9bdd7d2 Compare October 23, 2026 22:49
Apps that ship a filled colour bitmap as their small icon (QQ, WeChat, DiDi)
reached the glasses as a flat block: tinting every small icon white keeps the
alpha and drops the colours that carried the shape. System icons are unaffected,
since their alpha is cut out.

Classify from the rendered pixels instead: hue means colour artwork and keeps
its colours; a colourless icon is a template only when its alpha is cut out.
Template icons render exactly as before.

The debug-icon dump now writes in debug builds only and sweeps once per process.
@A-nony-mous
A-nony-mous force-pushed the fix/notification-icons branch from 9bdd7d2 to 8bfd613 Compare October 23, 2026 22:51
@jimrandomh

Copy link
Copy Markdown
Owner

Opus 5.5 PR review. I'll probably have time to look more closely and iterate over the weekend.

I'd request changes on PR #54. The PR does fix a real problem, but its fix breaks the case the white tint was added for. I checked the main points against the diff and the file's history; nothing was built or run.

The PR is from A-nony-mous and changes one file, FaceclawMediaNotificationListenerService.kt. It drops setTint(Color.WHITE) from small icons and adds iconIsAlphaTemplate(), a check that decides per icon whether to draw it from its alpha or from its brightness. It also limits the debug-icon dump to debug builds and empties that folder once per process. Line numbers below refer to the PR's version of the file.

Blocking

  1. Coloured small icons that are just shapes go dim or vanish (~507). Since Android 5.0 the system tints small icons and ignores their colours, so many apps fill theirs with a brand colour. With the white tint gone, an icon filled with pure blue fails the hue test and is drawn by brightness, which rounds down to level 0: invisible. Pure red comes out at level 1. Your commit 3532b1f ("Fix certain notification icons being missing") is what added the tint.
  2. The Discord problem comes back (~428). The deleted comment explains why the tint exists: some apps put random noise in the colour channels. That noise now decides the hue test and the brightness of every pixel, so those icons come out speckled again.
  3. It doesn't fully fix the PR's own case (~509). The check treats any colourless icon with at least 5% transparent pixels as a shape to tint. A logo on a white circle (about 21% transparent corners), a rounded square, or a bitmap with 1px of transparent padding all pass, so they still become a solid white block.
  4. Tints built into the drawable now survive (~428). An Icon's own tint and a vector's android:tint (resolved against the listener's light theme) used to be overridden by the white tint. Now a coloured tint sends the icon down the brightness path, and a semi-transparent tint dims it to about 37%.
  5. The large-icon fallback is affected too (~453). When the small icon fails to load, loadNotificationIcon returns the large icon, and the new check runs on that as well. A grayscale round avatar or black-and-white album art is then treated as a shape and sent as a plain lit circle. Before this PR those were always drawn by brightness.

Suggested direction: keep the white tint as the default. Handle only the actual failure, where the alpha has no shape inside it: it is solid apart from rounded corners or a margin. For that case, use brightness or fall back to the large or launcher icon. A colour-based test can't separate "coloured artwork" from "coloured shape meant to be tinted".

Debug dump

  • Old files are never cleaned on release builds (~556). The release-build early return runs before the cleanup, so files already written on release installs stay forever.
  • The stated reason for skipping release builds looks wrong. The PR says nobody can pull these files from a release build. As far as I know, the app's external files folder can still be pulled with adb on many Android versions, and dropping the dump removes the aid for garbled-icon reports from users.
  • The PR overstates the growth problem. Files are named icon-<index>-<pkg>.png and overwrite each other, so the folder grows with the number of distinct (index, app) pairs, not with every refresh. Clearing it once per process only resets that until restart.
  • The dump no longer shows what was sent (~454). It now saves the raw, untinted bitmap, and neither the file name nor the log line records which path the icon took.

Minor

  • Race on cleanup (~564): the @Volatile check-then-set on debugIconsPruned isn't atomic, so one thread can delete a file another just wrote. AtomicBoolean.compareAndSet fixes it.
  • Double pixel pass (~488): every icon is now read pixel by pixel twice, on a path notification-icons.ts says blocks the main thread. A single getPixels into an IntArray would cover both passes.
  • Off-by-one in a doc comment (33 vs 492): TEMPLATE_ALPHA_FLOOR's doc says "at or below", but the code uses <.
  • Unnamed thresholds: the two values that actually decide the result (* 4, * 20) are inline numbers, while the less important ones got named constants.

I can post these as inline comments on the PR with /code-review --comment, or write a shorter summary comment for the author. I haven't posted anything yet.

This branch has not been deployed

No deployments
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