Skip to content

Add "optimize alternative icons" insight handling#417

Merged
NicoHinderling merged 3 commits intomainfrom
alternative-icon-insight
Oct 15, 2025
Merged

Add "optimize alternative icons" insight handling#417
NicoHinderling merged 3 commits intomainfrom
alternative-icon-insight

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

@NicoHinderling NicoHinderling commented Oct 15, 2025

Closes EME-50

I tried to match the emerge implementation as much as possible. There's a ~6% size difference between the ultimately optimized icons compared to what's produced by the emerge code But it is consistent across all the images and appears to be related to the fact that we are using a different HEIC encoder:

  • in emerge we use Apple's native HEIC encoding via swift (Core Graphics/ImageIO)
  • in launchpad we use libheif

I tried reducing the image quality from 85% to 75% to get the size reduction closer to emerge's but it didn't reduce the size enough to justify an extra 10% loss in quality

@linear
Copy link
Copy Markdown

linear Bot commented Oct 15, 2025

EME-50 Optimize icons

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 15, 2025

Codecov Report

❌ Patch coverage is 89.52381% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.96%. Comparing base (7683d66) to head (9236935).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...aunchpad/size/insights/apple/image_optimization.py 75.60% 9 Missing and 1 partial ⚠️
src/launchpad/artifacts/apple/zipped_xcarchive.py 56.25% 4 Missing and 3 partials ⚠️
...ize/insights/apple/alternate_icons_optimization.py 92.85% 2 Missing and 1 partial ⚠️
tests/unit/test_alternate_icons_optimization.py 98.11% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   79.77%   79.96%   +0.18%     
==========================================
  Files         147      149       +2     
  Lines       11850    12027     +177     
  Branches     1208     1223      +15     
==========================================
+ Hits         9453     9617     +164     
- Misses       1962     1969       +7     
- Partials      435      441       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

size (1024px) before optimization, since they only need quality for homescreen display.
"""

# Icon size constants matching Swift implementation
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ai generated comment? lol

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops

resized, file_info.path, file_info.file_type, resized_size, file_info.size
)
except Exception as exc:
logger.error("Failed to process %s: %s", file_info.path, exc)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use logger.exception, it will automatically capture the exception details. Also avoid using string interpolation in the message so it doesn't create separate issues for us each time.

if res := self._check_heic_minification(img, resized_size):
minify_savings, minified_size = res.savings, res.optimized_size
except Exception as exc:
logger.error("Failed to analyze resized icon: %s", exc)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, logger.exception()

_MAX_WORKERS = 4

@abstractmethod
def _iter_files_to_analyze(self, input: InsightsInput) -> Iterable[FileInfo]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these two methods be combined into something like _find_images() -> List[FileInfo]? The de-duping can be merged into that as well. The current structure here is a bit awkward IMO, the iterator just converts it to a list anyways. It looks like the subclasses are responsible for returning a list of images that should be considered for processing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the _analyze_image_optimization override doesn't fit in nicely here since we lose the ThreadPoolExecutor logic and duplicate a lot of size-checking code. Maybe there could be some kind of overridable "preprocess" or "transform" method that gets called before _analyze_image_optimization so that method doesn't have to be overriden?

@NicoHinderling NicoHinderling force-pushed the alternative-icon-insight branch from fe416bb to c9a7754 Compare October 15, 2025 19:56
@NicoHinderling NicoHinderling force-pushed the alternative-icon-insight branch from c9a7754 to 5870161 Compare October 15, 2025 19:56
@NicoHinderling NicoHinderling force-pushed the alternative-icon-insight branch from 4bf0194 to 9236935 Compare October 15, 2025 20:04
@NicoHinderling NicoHinderling merged commit 2be5cc9 into main Oct 15, 2025
21 checks passed
@NicoHinderling NicoHinderling deleted the alternative-icon-insight branch October 15, 2025 20:20
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