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 alpha count associated constant to Pixel trait #956

Open
arthmis opened this issue May 9, 2019 · 4 comments
Open

Add alpha count associated constant to Pixel trait #956

arthmis opened this issue May 9, 2019 · 4 comments

Comments

@arthmis
Copy link
Contributor

arthmis commented May 9, 2019

I would like to be able to get the number of alpha values of the Pixel type found in an image buffer.

My specific use case for this functionality is:
This is to write generic code over gray scale and color images in an imperative style as opposed to using apply_with_alpha

pub fn invert_mut<I, P, S>(image: &mut I) 
where
    I: GenericImage<Pixel = P>,
    P: Pixel<Subpixel = S> + 'static,
    S: Primitive + 'static,
{
    let channel_count = <P>::CHANNEL_COUNT;
    // example of alpha_count 
    let alpha_count = <P>::ALPHA_COUNT;
    let slice = channel_count - alpha_count;
    let (width, height) = image.dimensions();

    // not using pixels_mut because it isn't well specified
    for y in 0..height {
        for x in 0..width {
            for pixel in image.get_pixel_mut(x, y)[..slice].iter_mut() {
                for value in pixel.data[..slice].iter_mut() {
                    *value = 255 - *value;
                }
            }
        }
    }
}

The alternative using apply_with_alpha looks like this by @HeroicKatora :

pub fn invert_mut<I, P, S>(image: &mut I) 
where
    I: GenericImage<Pixel = P>,
    P: Pixel<Subpixel = S> + 'static,
    S: Primitive + 'static,
{
    let map_color = |x| MAX_VALUE - x;
    let map_alpha = |x| x; //identity transform
    let (width, height) = image.dimensions();
    for y in 0..height {
        for x in 0..width {
            image.get_pixel_mut(x, y)
                .apply_with_alpha(map_color, map_alpha);
        }
    }
}

This might also be useful if someone needs to get the alpha count in the case there is an alpha value for each color channel.

@fintelia
Copy link
Contributor

fintelia commented May 9, 2019

Do you think that the Pixel trait interface is otherwise relatively reasonable? Looking at it, I'm somewhat skeptical that there is really all that much generic code that can be written against it without making a bunch more assumptions. This proposal is a (small) step in the right direction, but much more drastic API changes might be necessary. Given that we're not yet at 1.0, now is the time to get the interface right.

@arthmis
Copy link
Contributor Author

arthmis commented May 9, 2019

I think its overall pretty fine. I think that some of the methods shouldn't be mandatory. For example if it were possible to get the alpha_count then methods like apply_with_alpha wouldn't be necessary, but nice to have. I don't think every implementor for that trait would necessarily want to use that method. I think this could extend to almost all of the methods, but I'm not sure what kind of person would implement a very bare bones Pixel type. I hope this makes sense lol. But just like you said, trying to make it more generic without assumptions could be hard.

@HeroicKatora
Copy link
Member

HeroicKatora commented May 9, 2019

I don't think every implementor for that trait would necessarily want to use that method.

I think this could extend to almost all of the methods, but I'm not sure what kind of person would implement a very bare bones Pixel type.

@lazypassion It would help if there were as many methods with default provided implementations as possible. The methods that you added for example could be provided as default implementations as well. Which others can be default implemented as well?

@arthmis
Copy link
Contributor Author

arthmis commented May 9, 2019

Yes exactly. The rest of the map and apply functions can have defaults as well as the pixel conversion functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants