7
\$\begingroup\$

This struct is used a lot throughout my programme. This struct is responsible for only things regarding colour, not anything else.

I'm largely concerned of the bitwise operations, and the fact that I store four byte values in a uint but access them with bitwise flags. This is a concept I use throughout the programme. (I have one struct that has a 14-bit field, and two 7-bit fields.)

/// <summary>
/// A structure to represent a generic set of Red, Green, Blue and Alpha to represent the shade to paint an object.
/// </summary>
public struct Color
{
    private uint _PackedValue;

    /// <summary>
    /// Gets or sets the Alpha component of the <see cref="Color"/>.
    /// </summary>
    public byte A { get { return (byte)((_PackedValue & 0xFF000000u) >> 0x18); } set { _PackedValue = (_PackedValue & ~0xFF000000u) | ((uint)value << 0x18); } }

    /// <summary>
    /// Gets or sets the Red component of the <see cref="Color"/>.
    /// </summary>
    public byte R { get { return (byte)((_PackedValue & 0x00FF0000u) >> 0x10); } set { _PackedValue = (_PackedValue & ~0x00FF0000u) | ((uint)value << 0x10); } }

    /// <summary>
    /// Gets or sets the Green component of the <see cref="Color"/>.
    /// </summary>
    public byte G { get { return (byte)((_PackedValue & 0x0000FF00u) >> 0x08); } set { _PackedValue = (_PackedValue & ~0x0000FF00u) | ((uint)value << 0x08); } }

    /// <summary>
    /// Gets or sets the Blue component of the <see cref="Color"/>.
    /// </summary>
    public byte B { get { return (byte)((_PackedValue & 0x000000FFu) >> 0x00); } set { _PackedValue = (_PackedValue & ~0x000000FFu) | ((uint)value << 0x00); } }

    /// <summary>
    /// Creates an instance of a <see cref="Color"/>.
    /// </summary>
    /// <param name="a">The alpha <c>byte</c> component.</param>
    /// <param name="r">The red <c>byte</c> component.</param>
    /// <param name="g">The green <c>byte</c> component.</param>
    /// <param name="b">The blue <c>byte</c> component.</param>
    public Color(byte r, byte g, byte b, byte a)
    {
        _PackedValue = ((uint)a << 0x18) | ((uint)r << 0x10) | ((uint)g << 0x08) | ((uint)b << 0x00);
    }

    /// <summary>
    /// Creates an instance of a <see cref="Color"/> with a default alpha of 255.
    /// </summary>
    /// <param name="r">The red <c>byte</c> component.</param>
    /// <param name="g">The green <c>byte</c> component.</param>
    /// <param name="b">The blue <c>byte</c> component.</param>
    public Color(byte r, byte g, byte b)
    {
        _PackedValue = ((uint)0xFF << 0x18) | ((uint)r << 0x10) | ((uint)g << 0x08) | ((uint)b << 0x00);
    }

    /// <summary>
    /// Creates an instance of a <see cref="Color"/> from a packed value.
    /// </summary>
    /// <param name="packedValue">The original packed value of the <see cref="Color"/>.</param>
    public Color(uint packedValue)
    {
        _PackedValue = packedValue;
    }

    /// <summary>
    /// Returns the packed <c>uint</c> for storage.
    /// </summary>
    /// <returns>A <c>uint</c> representing this <see cref="Color"/> instance.</returns>
    public uint GetPackedValue()
    {
        return _PackedValue;
    }

    /// <summary>
    /// Determines whether two <see cref="Color"/> objects have the same value.
    /// </summary>
    /// <param name="a">The first <see cref="Color"/> object.</param>
    /// <param name="b">The second <see cref="Color"/> object.</param>
    /// <returns>True if the objects have the same values, false otherwise.</returns>
    public static bool operator ==(Color a, Color b)
    {
        return a._PackedValue == b._PackedValue;
    }

    /// <summary>
    /// Determines whether two <see cref="Color"/> objects do not have the same value.
    /// </summary>
    /// <param name="a">The first <see cref="Color"/> object.</param>
    /// <param name="b">The second <see cref="Color"/> object.</param>
    /// <returns>True if the objects do not have the same values, false otherwise.</returns>
    public static bool operator !=(Color a, Color b)
    {
        return a._PackedValue != b._PackedValue;
    }

    /// <summary>
    /// Determines whether an object is a <see cref="Color"/> and is equal to the current <see cref="Color"/> object.
    /// </summary>
    /// <param name="obj">The object to compare.</param>
    /// <returns>True if the object is a <see cref="Color"/> object and has the same value as the current <see cref="Color"/> object.</returns>
    public override bool Equals(object obj)
    {
        return obj is Color && this == (Color)obj;
    }

    /// <summary>
    /// Gets the hash code for the current <see cref="Color"/> object.
    /// </summary>
    /// <returns>The hash code that represents the current <see cref="Color"/> object.</returns>
    public override int GetHashCode()
    {
        return _PackedValue.GetHashCode();
    }

    /// <summary>
    /// Implicitly materializes a <see cref="Color.Preset"/> to a <see cref="Color"/>.
    /// </summary>
    /// <param name="preset">The <see cref="Color.Preset"/> to convert.</param>
    /// <returns>The <see cref="Color.Presets"/> value that corrosponds to the <see cref="Color.Preset"/>.</returns>
    /// <remarks>
    /// This method is equivalent to calling <code>Color.Presets[(int)preset];</code>.
    /// </remarks>
    public static implicit operator Color(Color.Preset preset)
    {
        return Presets[(int)preset];
    }

    /// <summary>
    /// A list of <see cref="Color"/> objects that correspond to the <see cref="Color.Preset"/> enumeration values.
    /// </summary>
    public static readonly List<Color> Presets = new List<Color>() {
        new Color(0, 0, 0, 255), // Preset.Black
        new Color(0, 0, 128, 255), // Preset.DarkBlue
        new Color(0, 128, 0, 255), // Preset.DarkGreen
        new Color(0, 128, 128, 255), // Preset.DarkCyan
        new Color(128, 0, 0, 255), // Preset.DarkRed
        new Color(128, 0, 128, 255), // Preset.DarkMagenta
        new Color(128, 128, 0, 255), // Preset.DarkYellow
        new Color(192, 192, 192, 255), // Preset.Gray
        new Color(128, 128, 128, 255), // Preset.DarkGray
        new Color(0, 0, 255, 255), // Preset.Blue
        new Color(0, 255, 0, 255), // Preset.Green
        new Color(0, 255, 255, 255), // Preset.Cyan
        new Color(255, 0, 0, 255), // Preset.Red
        new Color(255, 0, 255, 255), // Preset.Magenta
        new Color(255, 255, 0, 255), // Preset.Yellow
        new Color(255, 255, 255, 255), // Preset.White
    };

    /// <summary>
    /// Provide a list of values to be used to create certain preset colour values.
    /// </summary>
    public enum Preset : int
    {
        Black = 0,
        DarkBlue = 1,
        DarkGreen = 2,
        DarkCyan = 3,
        DarkRed = 4,
        DarkMagenta = 5,
        DarkYellow = 6,
        Gray = 7,
        DarkGray = 8,
        Blue = 9,
        Green = 10,
        Cyan = 11,
        Red = 12,
        Magenta = 13,
        Yellow = 14,
        White = 15,
    }
}

The Color.Preset enumeration exists to provide compatibility directly with the ConsoleColor enumeration. Otherwise, the presets would be in a sub class and be readonly fields. I.e.:

public static readonly Color White = new Color(255, 255, 255, 255);
public static readonly Color Black = new Color(0, 0, 0, 255);
...

I have a lot of classes and structures designed around the idea of "one packed value that has many values inside it." Hopefully, I can get a broad enough answer here on that concept as well to apply to the other structures and classes (dozens of them).

An additional note:

I use the uint _PackedValue field to serialize for network transport, as that field is the most commonly accessed field. (Believe it or not, I rarely actually refer to the A, R, G or B properties, when compared to the _PackedValue field.)

\$\endgroup\$
  • 1
    \$\begingroup\$ Is there a reason your team wrote your own implementation of System.Drawing.Color? Looking at its source code, there's even a decent amount of overlap in the underlying functionality. \$\endgroup\$ – Dan Lyons Jul 30 '15 at 17:49
  • \$\begingroup\$ Something curious, why do you convert byte to uint when byte is already an unsigned 8-bit integer? Why not make the field a byte instead of uint? \$\endgroup\$ – tinstaafl Jul 30 '15 at 17:49
  • \$\begingroup\$ @DanLyons I need to replace some of the functionality, and add new functionality eventually, and I am not allowed to reference the System.Drawing namespace for this project. If I could, I would have used it. \$\endgroup\$ – 410_Gone Jul 30 '15 at 18:10
  • \$\begingroup\$ @tinstaafl The field holds all four byte values, I could make them four single-byte fields, but that would mean that the underlying memory order would depend on the order the fields appear in the structure, which would change how the network serializes them. \$\endgroup\$ – 410_Gone Jul 30 '15 at 18:11
  • \$\begingroup\$ Why don't you just make the enum values equal to the packed value of the corresponding colours? That would save you a bit of code and an enum's raw value is an implementation detail so it doesn't matter if they're a bit odd. It's just a thought (not a recommendation) \$\endgroup\$ – RobH Jul 31 '15 at 8:59
9
\$\begingroup\$

An alternative is to create a union, a structure with overlapping values, by adding the StructLayout and FieldOffset attributes. This elegantly allows you to overlay the uint on top of the byte members.

using System.Runtime.InteropServices;

[StructLayout(LayoutKind.Explicit)]
public struct Color
{

    [FieldOffset(0)] private uint value;
    [FieldOffset(0)] private byte b;
    [FieldOffset(1)] private byte g;
    [FieldOffset(2)] private byte r;
    [FieldOffset(3)] private byte a;

    public Color(uint value) : this(0, 0, 0, 0)
    {
        this.value = value;
    }

    public Color(byte r, byte g, byte b) : this(r, g, b, 255)
    {
    }

    public Color(byte r, byte g, byte b, byte a)
    {
        this.value = 0;
        this.r = r;
        this.g = g;
        this.b = b;
        this.a = a;
    }

    public uint Value
    {
        get
        {
            return this.value;
        }
        set
        {
            this.value = value;
        }
    }

    public byte R
    {
        get
        {
            return this.r;
        }
        set
        {
            this.r = value;
        }
    }

    public byte G
    {
        get
        {
            return this.g;
        }
        set
        {
            this.g = value;
        }
    }

    public byte B
    {
        get
        {
            return this.b;
        }
        set
        {
            this.b = value;
        }
    }

    public byte A
    {
        get
        {
            return this.a;
        }
        set
        {
            this.a = value;
        }
    }

}
\$\endgroup\$
  • 2
    \$\begingroup\$ I completely forgot you could do that! \$\endgroup\$ – 410_Gone Jul 31 '15 at 18:25
7
\$\begingroup\$

It is a little bit strange that you only work with the uint in your class. A developer used to the default System.Drawing.Color would use more likely the A, R, G, B properties and less often the uint _PackedValue.

If someone else than you will use this struct, its public so I guess this will happen, it would be much faster to use byte variables too because you wouldn't need to shift the _PackedValue. Sure it would double the memory usage of this struct, but so what nowadays every computer has enough memory installed that it won't be any problem.

As the usage of this uint is mostly for for the storage / network transfer part I would keep this but would work internal by using byte.

If the value of _PackedValue is needed I would calculate it at this point of time.

I also would prefer a property for _PacketValue instead of the GetPackedValue() method.

The constructor which doesn't have the a value would benefit from constructor chaining like

public Color(byte r, byte g, byte b)
    :this(r,g,b,255)
{}

which removes the small code duplication.

That being said I would do it like so

/// <summary>
/// A structure to represent a generic set of Red, Green, Blue and Alpha to represent the shade to paint an object.
/// </summary>
public struct Color
{

    /// <summary>
    /// Gets or sets the Alpha component of the <see cref="Color"/>.
    /// </summary>
    public byte A { get; set; }
    /// <summary>
    /// Gets or sets the Red component of the <see cref="Color"/>.
    /// </summary>
    public byte R { get; set; }

    /// <summary>
    /// Gets or sets the Green component of the <see cref="Color"/>.
    /// </summary>
    public byte G { get; set; }

    /// <summary>
    /// Gets or sets the Blue component of the <see cref="Color"/>.
    /// </summary>
    public byte B { get; set; }

    /// <summary>
    /// Creates an instance of a <see cref="Color"/>.
    /// </summary>
    /// <param name="a">The alpha <c>byte</c> component.</param>
    /// <param name="r">The red <c>byte</c> component.</param>
    /// <param name="g">The green <c>byte</c> component.</param>
    /// <param name="b">The blue <c>byte</c> component.</param>
    public Color(byte r, byte g, byte b, byte a)
    {
        R = r;
        G = g;
        B = b;
        A = a;
    }

    /// <summary>
    /// Creates an instance of a <see cref="Color"/> with a default alpha of 255.
    /// </summary>
    /// <param name="r">The red <c>byte</c> component.</param>
    /// <param name="g">The green <c>byte</c> component.</param>
    /// <param name="b">The blue <c>byte</c> component.</param>
    public Color(byte r, byte g, byte b)
        this(r, g, b, 255)
    {}

    /// <summary>
    /// Creates an instance of a <see cref="Color"/> from a packed value.
    /// </summary>
    /// <param name="packedValue">The original packed value of the <see cref="Color"/>.</param>
    public Color(uint packedValue)
    {
        A = (byte)((packedValue & 0xFF000000u) >> 0x18);
        R = (byte)((packedValue & 0x00FF0000u) >> 0x10);
        G = (byte)((packedValue & 0x0000FF00u) >> 0x08);
        B = (byte)((packedValue & 0x000000FFu) >> 0x00);
    }
    public uint PackedValue
    {
        get 
        {
           return ((uint)a << 0x18) | ((uint)r << 0x10) | ((uint)g << 0x08) | ((uint)b << 0x00);
        }
    }
    /// <summary>
    /// Returns the packed <c>uint</c> for storage.
    /// </summary>
    /// <returns>A <c>uint</c> representing this <see cref="Color"/> instance.</returns>
    public uint GetPackedValue()
    {
        return PackedValue;
    }

    /// <summary>
    /// Determines whether two <see cref="Color"/> objects have the same value.
    /// </summary>
    /// <param name="a">The first <see cref="Color"/> object.</param>
    /// <param name="b">The second <see cref="Color"/> object.</param>
    /// <returns>True if the objects have the same values, false otherwise.</returns>
    public static bool operator ==(Color a, Color b)
    {
        return a.PackedValue == b.PackedValue;
    }

    /// <summary>
    /// Determines whether two <see cref="Color"/> objects do not have the same value.
    /// </summary>
    /// <param name="a">The first <see cref="Color"/> object.</param>
    /// <param name="b">The second <see cref="Color"/> object.</param>
    /// <returns>True if the objects do not have the same values, false otherwise.</returns>
    public static bool operator !=(Color a, Color b)
    {
        return a.PackedValue != b.PackedValue;
    }

    /// <summary>
    /// Determines whether an object is a <see cref="Color"/> and is equal to the current <see cref="Color"/> object.
    /// </summary>
    /// <param name="obj">The object to compare.</param>
    /// <returns>True if the object is a <see cref="Color"/> object and has the same value as the current <see cref="Color"/> object.</returns>
    public override bool Equals(object obj)
    {
        return obj is Color && this == (Color)obj;
    }

    /// <summary>
    /// Gets the hash code for the current <see cref="Color"/> object.
    /// </summary>
    /// <returns>The hash code that represents the current <see cref="Color"/> object.</returns>
    public override int GetHashCode()
    {
        return PackedValue.GetHashCode();
    }

    /// <summary>
    /// Implicitly materializes a <see cref="Color.Preset"/> to a <see cref="Color"/>.
    /// </summary>
    /// <param name="preset">The <see cref="Color.Preset"/> to convert.</param>
    /// <returns>The <see cref="Color.Presets"/> value that corrosponds to the <see cref="Color.Preset"/>.</returns>
    /// <remarks>
    /// This method is equivalent to calling <code>Color.Presets[(int)preset];</code>.
    /// </remarks>
    public static implicit operator Color(Color.Preset preset)
    {
        return Presets[(int)preset];
    }

    /// <summary>
    /// A list of <see cref="Color"/> objects that correspond to the <see cref="Color.Preset"/> enumeration values.
    /// </summary>
    public static readonly List<Color> Presets = new List<Color>() {
        new Color(0, 0, 0, 255), // Preset.Black
        new Color(0, 0, 128, 255), // Preset.DarkBlue
        new Color(0, 128, 0, 255), // Preset.DarkGreen
        new Color(0, 128, 128, 255), // Preset.DarkCyan
        new Color(128, 0, 0, 255), // Preset.DarkRed
        new Color(128, 0, 128, 255), // Preset.DarkMagenta
        new Color(128, 128, 0, 255), // Preset.DarkYellow
        new Color(192, 192, 192, 255), // Preset.Gray
        new Color(128, 128, 128, 255), // Preset.DarkGray
        new Color(0, 0, 255, 255), // Preset.Blue
        new Color(0, 255, 0, 255), // Preset.Green
        new Color(0, 255, 255, 255), // Preset.Cyan
        new Color(255, 0, 0, 255), // Preset.Red
        new Color(255, 0, 255, 255), // Preset.Magenta
        new Color(255, 255, 0, 255), // Preset.Yellow
        new Color(255, 255, 255, 255), // Preset.White
    };

    /// <summary>
    /// Provide a list of values to be used to create certain preset colour values.
    /// </summary>
    public enum Preset : int
    {
        Black = 0,
        DarkBlue = 1,
        DarkGreen = 2,
        DarkCyan = 3,
        DarkRed = 4,
        DarkMagenta = 5,
        DarkYellow = 6,
        Gray = 7,
        DarkGray = 8,
        Blue = 9,
        Green = 10,
        Cyan = 11,
        Red = 12,
        Magenta = 13,
        Yellow = 14,
        White = 15,
    }
}  

I would like to suggest, if you will use this version, to also change the operators, Equals() and GetHashcode() methods.

\$\endgroup\$
  • \$\begingroup\$ Just an FYI, I had to add : this() to call the default constructor on both the public Color(byte r, byte g, byte b, byte a) and public Color(uint packedValue) methods. Because it's a struct the compiler didn't recognize that the properties set the fields. \$\endgroup\$ – 410_Gone Jul 31 '15 at 14:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.