image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.86k stars 597 forks source link

Add missing map2/apply2 functions to `Pixel` trait #2241

Open jnickg opened 3 months ago

jnickg commented 3 months ago

Overview

The Pixel trait has many elementary functions for modifying their values, but a few combinations of useful implementations do not exist. Namely:

Good for a first-time contributor, this would make it easier for users to implement their own simple image editing operations, such as image math, where often the alpha channel should be treated differently (e.g. pick self or other alpha value, even when summing RGB channels).

Already implemented in #2239

fintelia commented 3 months ago

The Pixel trait already having a lot of functions is a reason to avoid adding more. Could you share a bit more on what cases these methods would be needed for, and whether there are alternatives ways to implement those?

jnickg commented 3 months ago

The description mentions an example, which is image math (e.g. taking a Laplacian transform and subtracting that from an image's original content, where we want to preserve alpha but operate on the color channels. These seem like elementary operations that are missing, not exactly extra cruft. Is there another way to perform those operations with comparable performance? If so, why are any of the pixel-wise operations present?

fintelia commented 3 months ago

Yes there are other ways to get as good, if not better, performance. The trait exposes the underlying components and if you don't need to be generic over all implementations of Pixel you can rely on additional assumptions about the layout. For best performance, you probably want to operate on directly slices and fine-tune your code until it auto-vectorizes.

From looking closer at the PR, it would actually require a breaking change to add these methods because the Pixel trait isn't sealed. The whole Pixel trait / generic operations over images probably warrants a more substantial overhaul that includes breaking changes. But it would be a lot of work and I don't have the time to work on that

jnickg commented 3 months ago

Can you give a discrete example of how to accomplish map2_without_alpha using existing functions only?