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

Introduce PixelTypeInfo.HasAlpha #1396

Closed
antonfirsov opened this issue Oct 20, 2020 · 9 comments
Closed

Introduce PixelTypeInfo.HasAlpha #1396

antonfirsov opened this issue Oct 20, 2020 · 9 comments

Comments

@antonfirsov
Copy link
Member

antonfirsov commented Oct 20, 2020

Motivation

By introducing a property to observe whether a given pixel type has an alpha, we can then use it in our processors to omit unnecessary alpha premultiplication. Might be also interesting for users working with Image.Identify.

We need to make sure it's value is properly filled:

  • In decoders with Identify implementations (PNG, BMP)
  • For all built-in pixel formats (IPixel-s)

API alternatives

In #1394 (reply in thread) I forgot that extending IPixel is actually a breaking change. Non-breaking variant:

internal enum PixelAlphaRepresentation
{
    None,
    Associated,
    Unassociated
}

public class PixelTypeInfo
{
     // 'null' means unknown/not implemented
     public PixelAlphaRepresentation? AlphaRepresentation { get; }    
}

public PixelOperations<TPixel>
{
     // Return AlphaRepresentation == null by default, but for important pixel types, introduce proper overrides with T4:
     public virtual PixelTypeInfo GetPixelTypeInfo();
}
@JimBobSquarePants
Copy link
Member

I can see us adding ChannelCount to this type at some point so I guess that would be nullable also.

@antonfirsov
Copy link
Member Author

@JimBobSquarePants so you prefer the non-breaking variant? Honestly, now it looks like a safer way to go to me.
Also allows us to extend codecs incrementally to detect PixelTypeInfo details.

@JimBobSquarePants
Copy link
Member

Yeah, it's a safe clever approach. Big fan!

@saucecontrol
Copy link
Contributor

I might be misreading the purpose of this, but did you consider support for pixel formats that are already premultiplied? TIFF and DirectX (among others) support alpha in both forms.

I recall we had a chat about this somewhere, although I've since lost track of it... I set up a generic pixel format descriptor type here which borrows heavily from WIC's pluggable pixel format model and describes alpha using an enum if you're looking for inspiration.

@JimBobSquarePants
Copy link
Member

Oh I remember this stuff yeah, will definitely have to use it as a source of inspiration!

@antonfirsov
Copy link
Member Author

@saucecontrol do I get this right?

  • None: has no alpha
  • Associated: has alpha, pixel is premultiplied
  • Unassociated: has alpha, pixel is not premultiplied

@JimBobSquarePants edited my proposal, let's go with the non-breaking version + enum for PixelAlphaRepresentation.

@saucecontrol
Copy link
Contributor

Yep, that's right. DirectX uses Ignore, Premultiplied, and Straight to describe those values. I'm not sure which is more intuitive.

@JimBobSquarePants
Copy link
Member

I much prefer your naming.

@JimBobSquarePants JimBobSquarePants self-assigned this Nov 7, 2020
@JimBobSquarePants
Copy link
Member

I've started this.

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