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 missing map2/apply2 functions to Pixel trait #2241

Open
jnickg opened this issue May 20, 2024 · 4 comments
Open

Add missing map2/apply2 functions to Pixel trait #2241

jnickg opened this issue May 20, 2024 · 4 comments
Labels
kind: API missing or awkward public APIs

Comments

@jnickg
Copy link

jnickg commented May 20, 2024

Overview

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

  • map2_with_alpha and apply2_with_alpha
  • map2_without_alpha and apply2_without_alpha

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
Copy link
Contributor

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
Copy link
Author

jnickg commented May 26, 2024

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
Copy link
Contributor

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
Copy link
Author

jnickg commented May 27, 2024

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

@Shnatsel Shnatsel added the kind: API missing or awkward public APIs label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: API missing or awkward public APIs
Projects
None yet
Development

No branches or pull requests

3 participants