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

API Proposal: Add blittable Color to System.Numerics #48615

Open
Tracked by #47244
JeremyKuhne opened this issue Feb 22, 2021 · 63 comments · May be fixed by #106575
Open
Tracked by #47244

API Proposal: Add blittable Color to System.Numerics #48615

JeremyKuhne opened this issue Feb 22, 2021 · 63 comments · May be fixed by #106575
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@JeremyKuhne
Copy link
Member

JeremyKuhne commented Feb 22, 2021

Background and Motivation

System.Drawing.Color contains extra metadata that makes it unusable in interop scenarios and inefficient in arrays.

namespace System.Drawing
{
    public readonly struct Color : IEquatable<Color>
    {
        private readonly string? name;
        private readonly long value;
        private readonly short knownColor;
        private readonly short state;
    }
}

Color is also mutable, despite being readonly. If it is constructed from a system KnownColor value, it always looks up the value on every access.

In order to facilitate exchange of raw color data throughout the .NET ecosystem and external libraries we should expose common color types that only contain raw color data.

The intent is to follow up with these base types to provide a basic set of the most common functionality needed around colors as described here: #48615 (comment)

The intent is NOT to start building a general purpose image manipulation library. Libraries such as ImageSharp are the right answer for this.

WinForms and System.Drawing will be able to leverage this for performance, which was the original driver for this request.

// Native Win32 definitions

// GDI
typedef DWORD COLORREF;
// GDI+
typedef DWORD ARGB;

Proposed API

using System;

namespace System.Drawing
{
    partial struct Color : IEquatable<Color>
    {
        public static Color FromArgb(System.Numerics.Colors.Argb<byte> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<byte> argb);

        // ToNumericsArgb? ToArgbNumerics?
        public System.Numerics.Colors.Argb<byte> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<byte>(in Color color);
    }
}

// WPF
namespace System.Windows.Media
{
    public struct Color : IEquatable<Color>, IFormattable
    {
        public static Color FromArgb(System.Numerics.Colors.Argb<byte> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<byte> argb);
        public static Color FromArgb(System.Numerics.Colors.Argb<float> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<float> argb);

        public System.Numerics.Colors.Argb<byte> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<byte>(in Color color);
        public System.Numerics.Colors.Argb<float> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<float>(in Color color);
    }
}

namespace System.Numerics.Colors
{
    public static class Argb
    {
        public static Argb<byte> CreateBigEndian(uint color);
        public static Argb<byte> CreateLittleEndian(uint color);
        public static uint ToUInt32LittleEndian(this Argb<byte> color);
        public static uint ToUInt32BigEndian(this Argb<byte> color);
    }

    public readonly struct Argb<T> : IEquatable<Argb<T>>, IFormattable, ISpanFormattable where T : struct
    {
        public T A { get; }
        public T R { get; }
        public T G { get; }
        public T B { get; }

        public Argb(T a, T r, T g, T b);
        public Argb(ReadOnlySpan<T> values);
        public void CopyTo(Span<T> destination);
        public bool Equals(Argb<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
        // whatever ISpanFormattable says
        public Rgba<T> ToRgba();
    }

    public static class Rgba
    {
        public static Rgba<byte> CreateLittleEndian(uint color);
        public static Rgba<byte> CreateBigEndian(uint color);
        public static uint ToUInt32LittleEndian(this Rgba<byte> color);
        public static uint ToUInt32BigEndian(this Rgba<byte> color);
    }

    public readonly struct Rgba<T> : IEquatable<Rgba<T>>, IFormattable, ISpanFormattable where T : struct
    {
        public T R { get; }
        public T G { get; }
        public T B { get; }
        public T A { get; }

        public Rgba(T r, T g, T b, T a);
        public Rgba(ReadOnlySpan<T> values);
        public void CopyTo(Span<T> destination);
        public bool Equals(Rgba<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
        // whatever ISpanFormattable says
        public Argb<T> ToArgb();
    }
}

Original Proposal

    public readonly struct ColorArgb
    {
        // Layout matches GDI COLORREF and GDI+ ARGB
        public uint Value { get; }
        public byte A => (byte)(Value >> 24);
        public byte R => (byte)(Value >> 16);
        public byte G => (byte)(Value >> 8);
        public byte B => (byte)(Value);

        public ColorArgb(uint value) => Value = value;
        public ColorArgb(byte r, byte g, byte b) : this(r, g, b, byte.MaxValue) { }
        public ColorArgb(byte r, byte g, byte b, byte a) => Value = (uint)(a << 24 | r << 16 | g << 8 | b);
        public ColorArgb(KnownColor knownColor) => Value = KnownColorTable.KnownColorToArgb(knownColor);

        public ColorArgb(Color color) => Value = (uint)color.ToArgb();

        // Color has these 3 methods.
        public float GetHue() => 0;
        public float GetSaturation() => 0;
        public float GetBrightness() => 0;

        // Frequently checked as only GDI+ supports transparency.
        public bool HasTransparency => A != byte.MaxValue;
        public bool FullyTransparent => A == 0;

        public static implicit operator Color(ColorArgb color) => Color.FromArgb((int)color.Value);
        public static implicit operator ARGB(ColorArgb color) => (ARGB)color.Value;
        public static implicit operator ColorArgb(ARGB argb) => new ColorArgb((uint)argb);
        public static implicit operator ColorArgb(Color color) => new ColorArgb(color);
        public static implicit operator ColorArgb(KnownColor knownColor) => new ColorArgb(knownColor);
    }

    // For ease of use in interop definitions (P/Invoke, function pointers, COM)
    // (as close as we get to a typedef in C#)
    public enum ARGB : uint { }

Alternative Designs

We could drop a System.Drawing specific color type in System.Drawing for this purpose (the original proposal). Discussing this with @pgovind and @tannergooding we feel there is value in defining more broadly available color types in System.Numerics.

Using Vector4 is possible for this purpose, but that makes data interchange and just general usage difficult. (Was this ARGB or RGBA, etc?) The intent is also to add additional methods in the future that are dependent on specific layout (such as conversion to HSL/HSV, etc.).

Risks

No known risks.

@JeremyKuhne JeremyKuhne added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 22, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Drawing untriaged New issue has not been triaged by the area owner labels Feb 22, 2021
@ghost
Copy link

ghost commented Feb 22, 2021

Tagging subscribers to this area: @safern, @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

System.Drawing.Color contains extra metadata that makes it unusable in interop scenarios and inefficient in arrays.

namespace System.Drawing
{
    public readonly struct Color : IEquatable<Color>
    {
        private readonly string? name; // Do not rename (binary serialization)
        private readonly long value; // Do not rename (binary serialization)
        private readonly short knownColor; // Do not rename (binary serialization)
        private readonly short state; // Do not rename (binary serialization)
    }
}

Color is also mutable, despite being readonly. If it is constructed from a system KnownColor value, it always looks up the value on every access.

In order to facilitate GDI and GDI+ interop scenarios and creation of more performant overloads for System.Drawing and Windows Forms, we should expose a type only contains the raw color data.

// Native definitions

// GDI
typedef DWORD COLORREF;
// GDI+
typedef DWORD ARGB;

Proposed API

    public readonly struct ColorArgb
    {
        // Layout matches GDI COLORREF and GDI+ ARGB
        public uint Value { get; }
        public byte A => (byte)(Value >> 24);
        public byte R => (byte)(Value >> 16);
        public byte G => (byte)(Value >> 8);
        public byte B => (byte)(Value);

        public ColorArgb(uint value) => Value = value;
        public ColorArgb(byte r, byte g, byte b) : this(r, g, b, byte.MaxValue) { }
        public ColorArgb(byte r, byte g, byte b, byte a) => Value = (uint)(a << 24 | r << 16 | g << 8 | b);
        public ColorArgb(KnownColor knownColor) => Value = KnownColorTable.KnownColorToArgb(knownColor);

        public ColorArgb(Color color) => Value = (uint)color.ToArgb();

        // Color has these 3 methods.
        public float GetHue() => 0;
        public float GetSaturation() => 0;
        public float GetBrightness() => 0;

        // Frequently checked as only GDI+ supports transparency.
        public bool HasTransparency => A != byte.MaxValue;
        public bool FullyTransparent => A == 0;

        public static implicit operator Color(ColorArgb color) => Color.FromArgb((int)color.Value);
        public static implicit operator ARGB(ColorArgb color) => (ARGB)color.Value;
        public static implicit operator ColorArgb(ARGB argb) => new ColorArgb((uint)argb);
        public static implicit operator ColorArgb(Color color) => new ColorArgb(color);
        public static implicit operator ColorArgb(KnownColor knownColor) => new ColorArgb(knownColor);
    }

    // For ease of use in interop definitions (P/Invoke, function pointers, COM)
    // (as close as we get to a typedef in C#)
    public enum ARGB : uint { }

Alternative Designs

We could make a more generic set of color structs to handle all typical color usage scenarios, but that isn't the intent here. #32418 tracks such a request. This type is specifically to address the existing space that System.Drawing exists in, and as such lives in System.Drawing.

Risks

No known risks.

Author: JeremyKuhne
Assignees: -
Labels:

api-suggestion, area-System.Drawing, untriaged

Milestone: -

@charlesroddie
Copy link

Wow what horrors. ColorArgb is clearly better for the reasons given. But then Color8 in the linked thread solves the same problem. So apart from the namespace it's the same proposal.

@aaronfranke
Copy link

I dislike this proposal for a few reasons:

  • The name does not give any information about color depth, while Color8 does.

  • I think we should be using and encouraging RGBA instead of ARGB. RGBA is the most used format, while ARGB is mostly a just thing in Microsoft products.

@JeremyKuhne
Copy link
Member Author

So apart from the namespace it's the same proposal.

@charlesroddie Yes, it is very close. This is the native color, and this is specifically for direct interop. Color8 is not the same format and I don't want to muddy that space with a layout that doesn't match the "standard".

@aaronfranke

The name does not give any information about color depth, while Color8 does.

True, and I think Color8 is a much better "new" color. This is explicitly for perf and direct interop scenarios.

I think we should be using and encouraging RGBA instead of ARGB.

I agree, but it doesn't address the specific interop need here.

I want to be super clear- this is not intended to be the "new" Color. This is meant to allow us to have a shared native definition for GDI and GDI+ scenarios, which is what System.Drawing is specifically written to. We can take multiple steps to push people to use the "new" Color types when they become available. Type naming, subnamespacing, hiding from intellisense, etc. are options we can lean on.

@charlesroddie
Copy link

charlesroddie commented Apr 6, 2021

This is meant to allow us to have a shared native definition for GDI and GDI+ scenarios, which is what System.Drawing is specifically written to.

Sorry I didn't see the interop part. If it's a Windows-specific feature then I don't think it's a candidate for dotnet/runtime. Perhaps a WindowsGraphicsPrimitives library that WPF/WinForms can depend on? Or a GraphicsPrimitives library that does the same thing for a broader set of consumers?

@JeremyKuhne
Copy link
Member Author

If it's a Windows-specific feature then I don't think it's a candidate for dotnet/runtime.

Well, it is Windows specific in the sense that it is GDI/GDI+ specific, which is what System.Drawing is a projection/mapping of. So generally I agree with you, but this assembly is really a legacy Windows thing we're carrying and not the future of drawing/graphics support for .NET I think.

@safern
Copy link
Member

safern commented Apr 8, 2021

Given that this is needed by Winforms to improve perf and System.Drawing.Common could benefit from it, let's mark is as api-ready-for-review and discuss it in the review meeting.

@safern safern added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Apr 8, 2021
@safern safern added this to the 6.0.0 milestone Apr 8, 2021
@aaronfranke
Copy link

aaronfranke commented Apr 8, 2021

If this is reviewed, then #32418 should also be reviewed. Since so many things need a Color type, it makes sense to include one (or two) in .NET for everyone to use, including Winforms and System.Drawing and Maui and much more.

@bartonjs
Copy link
Member

In API Review we feel that this is just setting us up for an API bifurcation between Color and ColorArgb, and that there doesn't seem to be a compelling scenario for that bifurcation.

With a compelling scenario we could have a different discussion.

@bartonjs
Copy link
Member

bartonjs commented May 25, 2021

Video

It seems like there may be scenarios, but we should reconcile the layering (above or below Color) and determine if we need this, or a companion, for MAUI.

@bartonjs bartonjs reopened this May 25, 2021
@bartonjs bartonjs added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 25, 2021
@JeremyKuhne JeremyKuhne changed the title API Proposal: Add blittable Color to System.Drawing API Proposal: Add blittable Color to System.Numerics May 27, 2021
@JeremyKuhne
Copy link
Member Author

Updated the proposal based on offline discussions with @pgovind and @tannergooding.

@ghost
Copy link

ghost commented May 28, 2021

Tagging subscribers to this area: @tannergooding, @pgovind
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

System.Drawing.Color contains extra metadata that makes it unusable in interop scenarios and inefficient in arrays.

namespace System.Drawing
{
    public readonly struct Color : IEquatable<Color>
    {
        private readonly string? name; // Do not rename (binary serialization)
        private readonly long value; // Do not rename (binary serialization)
        private readonly short knownColor; // Do not rename (binary serialization)
        private readonly short state; // Do not rename (binary serialization)
    }
}

Color is also mutable, despite being readonly. If it is constructed from a system KnownColor value, it always looks up the value on every access.

In order to facilitate GDI and GDI+ interop scenarios and creation of more performant overloads for System.Drawing and Windows Forms, we should expose a type only contains the raw color data.

// Native definitions

// GDI
typedef DWORD COLORREF;
// GDI+
typedef DWORD ARGB;

Proposed API

using System;

namespace System.Drawing
{
    public readonly struct Color : IEquatable<Color>
    {
        public static implicit operator Color(System.Numerics.Colors.Argb<byte> argb);
        public static explicit operator System.Numerics.Colors.Argb<byte>(in Color color);
        public static implicit operator Color(System.Numerics.Colors.Rgba<byte> rgba);
        public static explicit operator System.Numerics.Colors.Rgba<byte>(in Color color);
    }
}

namespace System.Numerics.Colors
{
    public static class Argb
    {
        public static Argb<byte> CreateLittleEndian(uint color);
        public static uint ToUInt32LittleEndian(this Argb<byte> color);
    }

    public readonly struct Argb<T> : IEquatable<Argb<T>>, IFormattable where T : struct
    {
        public T A { get; }
        public T R { get; }
        public T G { get; }
        public T B { get; }

        public Argb(T a, T r, T g, T b);
        public Argb(ReadOnlySpan<T> values);
        public readonly void CopyTo(Span<T> destination);
        public bool Equals(Argb<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
    }

    public static class Rgba
    {
        public static Rgba<byte> CreateLittleEndian(uint color);
        public static uint ToUInt32LittleEndian(this Rgba<byte> color);
    }

    public readonly struct Rgba<T> : IEquatable<Rgba<T>>, IFormattable where T : struct
    {
        public T R { get; }
        public T G { get; }
        public T B { get; }
        public T A { get; }

        public Rgba(T r, T g, T b, T a);
        public Rgba(ReadOnlySpan<T> values);
        public readonly void CopyTo(Span<T> destination);
        public bool Equals(Rgba<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
    }
}

Original Proposal

``` C# public readonly struct ColorArgb { // Layout matches GDI COLORREF and GDI+ ARGB public uint Value { get; } public byte A => (byte)(Value >> 24); public byte R => (byte)(Value >> 16); public byte G => (byte)(Value >> 8); public byte B => (byte)(Value);
    public ColorArgb(uint value) => Value = value;
    public ColorArgb(byte r, byte g, byte b) : this(r, g, b, byte.MaxValue) { }
    public ColorArgb(byte r, byte g, byte b, byte a) => Value = (uint)(a << 24 | r << 16 | g << 8 | b);
    public ColorArgb(KnownColor knownColor) => Value = KnownColorTable.KnownColorToArgb(knownColor);

    public ColorArgb(Color color) => Value = (uint)color.ToArgb();

    // Color has these 3 methods.
    public float GetHue() => 0;
    public float GetSaturation() => 0;
    public float GetBrightness() => 0;

    // Frequently checked as only GDI+ supports transparency.
    public bool HasTransparency => A != byte.MaxValue;
    public bool FullyTransparent => A == 0;

    public static implicit operator Color(ColorArgb color) => Color.FromArgb((int)color.Value);
    public static implicit operator ARGB(ColorArgb color) => (ARGB)color.Value;
    public static implicit operator ColorArgb(ARGB argb) => new ColorArgb((uint)argb);
    public static implicit operator ColorArgb(Color color) => new ColorArgb(color);
    public static implicit operator ColorArgb(KnownColor knownColor) => new ColorArgb(knownColor);
}

// For ease of use in interop definitions (P/Invoke, function pointers, COM)
// (as close as we get to a typedef in C#)
public enum ARGB : uint { }
</details>

## Alternative Designs

We could drop a System.Drawing specific color type in System.Drawing for this purpose (the original proposal). Discussing this with @pgovind and @tannergooding we feel there is value in defining more broadly available color types in System.Numerics.

Using Vector4 is _possible_ for this purpose, but that makes data interchange and just general usage difficult. (Was this ARGB or RGBA, etc?) The intent is also to add additional methods in the future that are dependent on specific layout (such as conversion to HSL/HSV, etc.).

## Risks

No known risks.


<table>
  <tr>
    <th align="left">Author:</th>
    <td>JeremyKuhne</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`api-needs-work`, `api-suggestion`, `area-System.Drawing`, `area-System.Numerics`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>6.0.0</td>
  </tr>
</table>
</details>

@scalablecory
Copy link
Contributor

The typical pattern is that sRGB is sRGB and that RGB/RGBA is "the full color space".

I'm not familiar with the term "full color space" -- is it a misnomer for "any color space"?

What you're describing here isn't how I would expect the type to be used.

Anywhere I see RGB<byte>, I'd expect to pass compressed sRGB.

Anywhere I see RGB<float>, I'd expect to pass linear sRGB. Except with System.Windows.Media.Color, where I'd expect to pass linear scRGB.

@tannergooding
Copy link
Member

tannergooding commented Jul 24, 2021

sRGB is almost never the normal definition of "color" values in the types of APIs that this is covering.

Instead it is typically CIE 1931 RGB (linear RGB) which is the "full color space", that is the visible color spectrum irrespective of the limitations of monitors or other display technology:
image (image from wikipedia)

This along with CIE 1931 XYZ are the two most common color spaces and are often used and cited as the intermediary formats on which other color spaces such as sRGB (IEC 61966-2-1:1999) are built.

Edit: I think I may have gotten RGB and XYZ mixed up on which is the "full space", I think XYZ is the unlimited gamut that can define "any color", but the general point remains the same.

@Nirmal4G
Copy link

Nirmal4G commented Aug 4, 2021

If we're having trouble with the naming, I have a few suggestions.

  1. To + ArgbColor<byte>/RgbaColor<byte>
  2. To + ColorARGB<byte>/ColorRGBA<byte>
  3. To + AlphaFirstColor<byte>/AlphaLastColor<byte>
  4. To + ColorAlphaFirst<byte>/ColorAlphaLast<byte>

This is similar to naming of UTF8String and Http~ types. I personally recommend either 2 or 4 (If we're okay with the verbose naming)

@Nirmal4G
Copy link

Nirmal4G commented Aug 4, 2021

Unrelated to the issue... The original proposal in the description is not rendered properly.

To render the details/spoiler tag properly, just leave an empty line between the tags, like so...

<details>
<summary>Title text here...</summary>

<!-- insert markdown here... -->

</details>

@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Jan 26, 2022
@terrajobst
Copy link
Member

terrajobst commented Mar 8, 2022

Video

  • Looks good as proposed
using System;

namespace System.Drawing
{
    partial struct Color : IEquatable<Color>
    {
        public static Color FromArgb(System.Numerics.Colors.Argb<byte> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<byte> argb);

        // ToNumericsArgb? ToArgbNumerics?
        public System.Numerics.Colors.Argb<byte> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<byte>(in Color color);
    }
}

// WPF
namespace System.Windows.Media
{
    public struct Color : IEquatable<Color>, IFormattable
    {
        public static Color FromArgb(System.Numerics.Colors.Argb<byte> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<byte> argb);
        public static Color FromArgb(System.Numerics.Colors.Argb<float> argb);
        public static implicit operator Color(System.Numerics.Colors.Argb<float> argb);

        public System.Numerics.Colors.Argb<byte> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<byte>(in Color color);
        public System.Numerics.Colors.Argb<float> ToArgbValue();
        public static explicit operator System.Numerics.Colors.Argb<float>(in Color color);
    }
}

namespace System.Numerics.Colors
{
    public static class Argb
    {
        public static Argb<byte> CreateBigEndian(uint color);
        public static Argb<byte> CreateLittleEndian(uint color);
        public static uint ToUInt32LittleEndian(this Argb<byte> color);
        public static uint ToUInt32BigEndian(this Argb<byte> color);
    }
    public readonly struct Argb<T> : IEquatable<Argb<T>>, IFormattable, ISpanFormattable where T : struct
    {
        public T A { get; }
        public T R { get; }
        public T G { get; }
        public T B { get; }

        public Argb(T a, T r, T g, T b);
        public Argb(ReadOnlySpan<T> values);
        public void CopyTo(Span<T> destination);
        public bool Equals(Argb<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
        // whatever ISpanFormattable says
        public Rgba<T> ToRgba();
    }
    public static class Rgba
    {
        public static Rgba<byte> CreateLittleEndian(uint color);
        public static Rgba<byte> CreateBigEndian(uint color);
        public static uint ToUInt32LittleEndian(this Rgba<byte> color);
        public static uint ToUInt32BigEndian(this Rgba<byte> color);
    }
    public readonly struct Rgba<T> : IEquatable<Rgba<T>>, IFormattable, ISpanFormattable where T : struct
    {
        public T R { get; }
        public T G { get; }
        public T B { get; }
        public T A { get; }

        public Rgba(T r, T g, T b, T a);
        public Rgba(ReadOnlySpan<T> values);
        public void CopyTo(Span<T> destination);
        public bool Equals(Rgba<T> other);
        public string ToString(string format, IFormatProvider formatProvider);
        // whatever ISpanFormattable says
        public Argb<T> ToArgb();
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 8, 2022
@deeprobin
Copy link
Contributor

I think we can label this "easy". I think new contributors would do well with this issue.

@tannergooding
Copy link
Member

This one is being primarily driven by WinForms and need in other areas. It's going to likely be done by that team to ensure it and its usages are going in at the same time/release

@rickbrew
Copy link
Contributor

rickbrew commented Mar 11, 2022

So why are they named Rgba and Argb ? I thought the standard, as per WIC, was BGRA and RGBA? Both of those have A in the same position (byte index 3). I'm not familiar with moving A to byte index 0.

Argb<T> in particular does not appear compatible with Win32/WIC-style BGRA ordering. This means that to use Argb<T>, swizzling will always be required, which has overhead and will limit throughput in a very common scenario, including WinForms.

@tannergooding
Copy link
Member

Argb in particular does not appear compatible with Win32/WIC-style BGRA ordering.

This is a complex space and depends on native surface format of a given OS vs what is used everywhere.

The native Win32 surface format is often referred to as B8G8R8A8 but that is most often carried about as a typedef of uint32_t and so whether it matches a Bgra<T> or Argb<T> struct can depend on endianness among other factors.

It may be that we want/need to eventually expose Bgra<T> as well, but that doesn't necessarily fit with the immediate needs of WinForms, WPF, or common usages in games.

The conversion between formats can also be done as part of the copy or write operation in a majority of these cases, leading to no real additional cost in terms of the operation.

@rickbrew
Copy link
Contributor

rickbrew commented Mar 11, 2022

I disagree -- the swizzling definitely has a cost, especially when it has to be done on both ends of an operation, especially if has to be done multiple times as a buffer is passed around internally and has various operations performed on it that only operate on the swizzled format. So in my app/library, imagine I'm passing a BGRA bitmap from A to B to C, and each one wants to execute a filter on the bitmap that is supported by these primitives (but only in ARGB format!). That means I have to swizzle 6 times to get the job done, unless I add a way for the components to communicate the component ordering of the buffer. This affects architecture of apps and libraries, which may not be malleable enough to reasonably accommodate this.

It may be that we want/need to eventually expose Bgra as well, but that doesn't necessarily fit with the immediate needs of WinForms, WPF, or common usages in games.

That sounds completely contradictory. You're stating that a key target scenario here is WinForms and WPF, which will be dealing with BGRA because that's what the platform (Win32) uses, so therefore BGRA doesn't fit with their immediate needs? 😕 Paint.NET would also be hamstrung, performance-wise, if I wanted to use these primitives -- I have seen good performance improvements by optimizing and eliminating things like swizzling, copying, and format conversions.

To be blunt, if BGRA isn't supported, these primitives are completely useless for Paint.NET and for a large number of Windows/desktop scenarios. The performance cost of swizzling is not super high but it does lower the upper bound of performance, especially for real-time/interactive scenarios.

@tannergooding
Copy link
Member

the swizzling definitely has a cost, especially when it has to be done on both ends of an operation, especially if has to be done multiple times as a buffer is passed around internally and has various operations performed on it that only operate on the swizzled format.

I think you're misunderstanding. In terms of basic operations, the swizzle is performed as part of the load/store. So if you require a copy, then you can convert as part of that copy at no-additional cost (there is no instruction latency difference between a load or a load + swizzle). This is true of both individual pixel read-writes and more vectorized algorithms. You can achieve theoretically higher throughput for a 1-to-1 copy using non-temporal loads/stores, but that's something a bit different and isn't as common.

If you are having to swizzle, do an operation, and then re-swizzle; then yes there can be a cost for vectorized code since you can't do the swizzle as part of a vectorized store.

You're stating that a key target scenario here is WinForms and WPF, which will be dealing with BGRA because that's what the platform (Win32) uses, so therefore BGRA doesn't fit with their immediate needs

This is a complex area and what the docs describe vs how its implemented can differ a bit.

  • COLORREF is a uint32 and is 0x00BBGGRR
    • This is equivalent to struct COLORREF { byte R, G, B, A; }, where A is always 0, on a little-endian platform
    • This is described in docs as RGB
  • Gdiplus.Color is a uint32 and is 0xAARRGGBB.
    • This is equivalent to struct Color { byte B, G, R, A; } on a little-endian platform
    • This is described in docs as RGBA
    • The ordering in the struct, however, is BGRA
  • System.Windows.Media.Color is struct Color { byte a, r, g, b; } or struct Color { float a, r, g, b; }
    • This is equivalent to 0xBBGGRRAA on a little endian platform
    • This is described in the docs as ARGB
  • CSS and other standard colors are or in the form #RRGGBBAA
    • These are described in the docs as RGB and RGBA
  • DirectX Math are interpreted as struct Color { float r, g, b, a; }
    • These are described in the docs as RGBA
    • Most games likewise order their surface formats and other bits as RGBA
  • Direct2D exposes ColorF which is a uint32 and is 0xAARRGGBB
    • Direct2D recommends targeting B8G8R8A8 as this ends up matching the layout of ColorF and Gdiplus.Color on a little-endian system

The actual underlying format for GDI+ and the recommended format for D2D1 is really equivalent to BGRA since Windows only supports little-endian systems (however, it would be ARGB on a big-endian system if ever supported in the future). However, most higher level libraries are working with RGBA or ARGB instead and then "normalize" to the actual surface format in the compositor or elsewhere. This is particularly relevant when the surface format for other platforms/systems can differ as well and so picking one major format to standardize on for the "core" functionality is desirable. This does sometimes have a cost for converting, but those are often relegated to "one time" operations that happen as part of the start/end of the transform pipeline.

  • Also noting that BGRA is strictly just the "native" surface format. If you have a monitor that supports it, you are completely able to opt into other formats as well including HDR, 10 or 12-bit color formats, etc. In this case, the DWM will handle fixing up the difference for apps that don't support this (and it can lead to a noticeable "washed out" appearance in some cases).

@rickbrew
Copy link
Contributor

Hmm, well that seems to clear up the confusion on my part (along with the conversation we had on Discord).

@dakersnar dakersnar modified the milestones: 7.0.0, Future Aug 8, 2022
@AlexRadch AlexRadch linked a pull request Aug 17, 2024 that will close this issue
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 17, 2024
@AlexRadch
Copy link
Contributor

AlexRadch commented Aug 18, 2024

I implemented this API in PR #106575. I think that the methods CreateBigEndian(uint color) and CreateLittleEndian(uint color) have inappropriate and confusing names. These methods create a color from an integer value with a big-endian/little-endian byte order.

I suggest renaming them to FromBigEndian and FromLittleEndian. Such names are more consistent with reverse methods ToUInt32BigEndian and ToUInt32LittleEndian. It is convenient for reverse methods to have reverse names.

@PavielKraskouski
Copy link

Will it be possible to convert float colors to vector types and back? For example, when reading normal maps, colors need to be converted to vector.

@michael-hawker
Copy link

Does CMYK factor as an alternate schema/form in here at all as something to think about? Just trying to think beyond the base cases here and what could be useful to other industries/use-cases beyond the traditional graphics rendering.

@rickbrew
Copy link
Contributor

rickbrew commented Dec 6, 2024

Worth noting is that colors composed of different primitive types, e.g. byte vs float, often use different ranges for their values. An Rgba<byte> will use the full [0, 255] range of a byte whereas an Rgba<float> would use a normalized range of [0, 1]. Values outside of that range are either invalid or used for things like HDR. So this design should consider that a "primitive schema" might be necessary in the future, particularly if generic conversions between e.g. Rgba<byte> and Rgba<float> is desired functionality.

Put another way, we can't just constrain to INumber<T>, IMinMaxValue<T> and use IMinMaxValue<T>.MinValue/MaxValue for the range of color components.

Some imaging toolkits, notably WIC, also have a default policy that pixel formats using unsigned integers (e.g. Rgba<byte>) use companded gamma (e.g. sRGB ~2.2ɣ), while floating point formats (Rgba<float>) use linear gamma (1.0ɣ). This gets into color spaces, color profiles, and color "contexts", which is a huge mess of complexity on its own, but is worth keeping an eye on so that these APIs don't get painted in a corner with respect to future expansion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.