Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for more modern image format like JPEG XL #899

Closed
wants to merge 13 commits into from

Conversation

mackoj
Copy link
Contributor

@mackoj mackoj commented Sep 4, 2024

Important

  • This PR introduces a new dependency, which is typically not preferred, but the aim here is to experiment rather than deliver a polished, production-ready implementation.

The primary goal of this PR is to spark a discussion about whether we should change the default image format for swift-snapshot-testing. This PR adds support for multiple image formats, and while it does not set a new default, it allows users to choose and add their preferred image format as a dependency to their test target.

As discussed in this GitHub discussion, this PR introduces basic support for JPEG XL.

Context:
In our company, our snapshots folder contains 1,000 test snapshot images totaling 202.1 MB when using PNG files. With JPEG XL, the size is reduced to 60.7 MB, representing a 70% reduction.

Implementation Details:
The implementation supports multiple image encoders, including the current default PNG encoder and the new JPEG XL encoder. This design allows for the easy addition of other encoders, such as .heic, .webp or other formats, which could be useful for benchmarking and making an informed decision about the default file format.

Key files include:

  • Sources/ImageSerializer/ImageSerializer.swift
  • Sources/SnapshotTesting/Snapshotting/ImageCoder.swift

A new ImageFormat enum has been added to allow users to select their desired image format:

public enum ImageFormat: String {
  case jxl
  case png
  case heic
  case webp

  public static var defaultValue = ImageFormat.png
}

Notes:

  • Due to the new dependency, the minimum macOS and iOS versions have been raised. This dependency currently does not support tvOS, but this could be addressed in the future by integrating directly with libjxl instead of JxlCoder.

What’s Missing:

  • Add support for withSnapshotTesting
  • Add Tests
  • Replace JxlCoder with libjxl
  • Extract JPEG XL-related code into a separate target. This will ensure that for most users, nothing changes (no new dependencies, .png remains the default format). Users who prefer smaller snapshots can add a target called JPEGXLSnapshot (containing everything related to .jxl) to their test target and switch the default image renderer.
  • Improve the API and usage

Additional Considerations:
What else should we test besides snapshot size? Should we measure read and write times, or check if images work natively on the OS? Should we also consider including formats like .avif or .webp?

Usage:
To use the JPEG XL image serializer, add JPEGXLImageSerializer as a dependency to your test target. The setup is simple; it works like record, but you need to set the image format globally:

imageFormat = .jxl

Alternatively, you can specify the image format for each image function as the last argument:

assertSnapshot(of: label, as: .image(precision: 0.9, format: .jxl))

Feedback Request:
I’m seeking feedback on:

  • The space savings in your Snapshots folder when using .jxl (JPEG XL) compared to .png.
  • The impact on test run times using the new encoder versus the classic PNG encoder.

Please include details about your OS, toolchain, and machine setup to provide better context for the results.

Thank you for your time and feedback!

moritzsternemann and others added 5 commits September 1, 2020 13:49
* Add scale parameter to image diffing strategies

* Use updated diffing in CALayer, UIView, UIViewController and SwiftUIView strategies

* Pass scale through to CGPath and UIBezierPath strategies

* Re-generate project file
@mackoj
Copy link
Contributor Author

mackoj commented Sep 4, 2024

After opening the PR I saw that my base branch was the last version of master because now the default branch is main so I will need to fix the issues.

@mackoj mackoj changed the title Feat Add basic support for JPEG XL Feat add support for JPEG XL / HEIC Sep 7, 2024
@mackoj mackoj changed the title Feat add support for JPEG XL / HEIC Feat add support for JPEG XL and HEIC Sep 7, 2024
@mackoj mackoj changed the title Feat add support for JPEG XL and HEIC Feat add support for JPEG XL / HEIC and WEBP Sep 7, 2024
@mackoj mackoj changed the title Feat add support for JPEG XL / HEIC and WEBP Feat add support for JPEG XL Sep 7, 2024
@mackoj mackoj changed the title Feat add support for JPEG XL Add support for more modern image format like JPEG XL Sep 7, 2024
@mackoj mackoj marked this pull request as ready for review September 7, 2024 22:17
@mbrandonw
Copy link
Member

Hi @mackoj, thanks for taking the time to explore this! It's certainly interesting, but unfortunately it's not really something we are looking to bring into the library anytime soon. First of all, as it is designed now it is a breaking change for people, and so it could only go in a 2.0. But also, ideally, this kind of snapshot strategy could be released as its own standalone library that people could depend on if they want this functionality. Is there any reason this code can't be extracted out of the repo?

@stephencelis
Copy link
Member

@mackoj Thanks for looking into this and sharing! We're looking at minimizing changes to strategies these days with an eye toward a more plugin-izable future in 2.0, so we were wondering if it's possible to extract these changes to a plugin, instead? And then we can link to your library in the README for better discoverability.

@mackoj
Copy link
Contributor Author

mackoj commented Sep 10, 2024

Thanks for taking the time to respond, @mbrandonw and @stephencelis. I understand the responsibility you have to avoid breaking everyone's code, and in its current form, this PR does break a lot of things. However, I see it more as an opportunity to experiment and evaluate the benefits of using another format rather than something to add to swift-snapshot-testing in its current state.

I’m hesitant to fork swift-snapshot-testing like SnapshotTestingHEIC did because that would prevent combining multiple plugins. Since I’m already using swiftui-preview-snapshots, that’s not a viable path for me as I need swiftui-preview-snapshots to automatically use the new file format.

The goal after this experiment is to attempt something smaller but with the intention of getting it merged. For example, a plugin API for image output could be a good starting point. Creating a way to support different image outputs while keeping the default one intact could be beneficial, especially if it doesn’t break the existing API. This approach would enable the creation of plugins that could work specifically for this purpose, serving as a great first step toward making swift-snapshot-testing more extensible.

I did try a plugin API here #904.

@mackoj mackoj mentioned this pull request Sep 10, 2024
4 tasks
@mackoj mackoj closed this Sep 11, 2024
@mbrandonw
Copy link
Member

Hi @mackoj, sorry for the confusion, but we were talking about extracting this out into its own library separate from swift-snapshot-testing. Your library would depend on ours and vend this new snapshot strategy. And then people who want this behavior would depend on your package.

@mackoj
Copy link
Contributor Author

mackoj commented Sep 11, 2024

Hi @mbrandonw, thanks for your response!

I understand the focus on strategies, but in certain cases, such as image export, a plugin API can provide significant flexibility across various tools. For example, I use both AccessibilitySnapshot and swiftui-preview-snapshots. If I want to reduce git-lfs usage, I might turn to SnapshotTestingHEIC, but then I hit a roadblock since these tools don’t share a unified strategy for image export.

Without a plugin API, I’d need to build custom solutions to make these tools work together, which is difficult when iterating quickly or experimenting with multiple strategies on a team. This leads to maintenance overhead, and overloading different frameworks becomes unwieldy.

A plugin API in swift-snapshot-testing would streamline this process by offering a shared interface for cross-cutting concerns like image encoding and decoding, without disrupting existing users. It provides a minimal API surface that allows plugins to handle specific tasks like image saving, keeping the core strategies intact.

In my PR, I’ve aimed to keep changes minimal and non-intrusive, ensuring it just introduces this added flexibility without altering the current usage.

I’d really appreciate if you could take a look! #904

@mackoj mackoj mentioned this pull request Sep 15, 2024
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.

5 participants