Misleading docs re: double ranges

[ividyon]

Color property contains nested properties you may bind to or use to retrieve the color in code-behind: Color.A: Current Alpha, a double ranging from 0 to 255 Color.RGB_R, Color.RGB_G, Color.RGB_B: Dimensions of the RGB color space, each is a 0-255 double Color.HSV_H: Hue in the HSV color space, a 0-360 double Color.HSV_S: Saturation in the HSV color space, a 0-100 double Color.HSV_V: Value in the HSV color space, a 0-100 double Color.HSL_H: Hue in the HSL color space, a 0-360 double Color.HSL_S: Saturation in the HSL color space, a 0-100 double Color.HSL_L: Lightness in the HSL color space, a 0-100 double

Reading this, I assumed "A double ranging from 0 to 255" to mean that the value in the number can be between 0.0 and 255.0.

In my application, I have colors and alpha saved as float ranging from 0 to 1. Reading the README for this color picker, my assumption was that I had to multiply these values by 255 to receive the values the color picker wants.

I could achieve that with the following:

        var color = new Color(); // Windows.Media
        color.ScA = W;
        color.ScR = X;
        color.ScG = Y;
        color.ScB = Z;
        var colorState = new ColorState();
        colorState.SetARGB(color.A, color.R, color.G, color.B);
        _colorState = colorState;

This would feed my 0-1 floats into a Color class, then spit them out as 0-255 bytes. This is what I assumed the ColorState wants, based on the description.

Yet in fact, I had to do this:

        var colorState = new ColorState();
        colorState.SetARGB(W, X, Y, Z);
        _colorState = colorState;

because, it seems, the color state actually expects a double from 0 to 1, not from 0 to 255.

[Equbuxu]

Hi! You are right in saying that ColorState expects a double from 0 to 1. The part of the docs that states "a double ranging from 0 to 255" applies to a different property, Color.A. In general, we intended SelectedColor and Color (not ColorState) properties to be used for getting or setting the displayed color.

The precise behavior of the internals of the ColorState struct is not documented, because you are not expected to edit it manually, the struct is there to contain the complete state needed to transfer all the slider positions and textbox texts from one control to another. In other words, it's for bindings.

[ividyon]

Oh, I see! It's a great tool. I wonder if I'm using it wrong?

For me it's intended as a "helper" widget to help picking some otherwise obnoxiously manual-labor values, but not to be the only widget. It's auxiliary in nature. So I needed to connected it to the "real" values properly, which is why I've done it as seen above, to bind a ColorState to the picker, and synchronize that ColorState with my real numbers.

I had to do it this way to set a "default value" for the color picker, based on the data that comes in from elsewhere.

Did I do it in an unintended manner?

    public ColorState ColorState
    {
        get => _colorState;
        set
        {
            _colorState = value;
            X = (float)value.RGB_R;
            Y = (float)value.RGB_G;
            Z = (float)value.RGB_B;
            W = (float)value.A;
        }
    }

    public float X
    {
        get => Value.X;
        set
        {
            if (Value.X.Equals(value)) return;
            OnPropertyChanging();
            OnPropertyChanging(nameof(ColorState));
            Value = Value with { X = value };
            OnPropertyChanged();
            OnPropertyChanged(nameof(ColorState));
        }
    }

    // ...

    public Vec4PropertyStepViewModel(FxrModel.Property.PropertyStep<Vector4> step, Vec4PropertyViewModel prop) :
        base(step, prop)
    {
        var colorState = new ColorState();
        colorState.SetARGB(W, X, Y, Z);
        _colorState = colorState;
    }

<colorPicker:PortableColorPicker HorizontalAlignment="Center" ShowAlpha="True" Style="{StaticResource DefaultColorPickerStyle}" Width="20" Height="20" Margin="{adonisUi:Space 1, Top=0}"
                                                                     ColorState="{Binding ColorState, Mode=TwoWay}" />

[ividyon]

The thing is that this does work fine, and ColorState is used immediately in an example of how to bind the element to data, so I feel this discrepancy should be pointed out somehow.

The Media Color class does this by prefixing the 0-1 range colors with s, presumably for scalar. Here, it could at least use a mention in the README.

[Equbuxu]

Hi again, sorry for the delayed response, overall there is nothing fundamentally wrong with your approach, it should work fine even though I didn't really expect anyone to use ColorState this way. Using SelectedColor to change the color seemed a lot more intuitive to me. And actually, is there any reason why you chose ColorState specifically? It seems like you code shouldn't break if you switch to using SelectedColor with a 0-1 to 0-255 conversion.

As for the docs, you are right, a clearer wording would be good, especially considering that the example uses ColorState heavily (albeit in an intended way)

[ividyon]

Basically, I use it because I glimpsed at "example usages" in the README here to learn how to use the lib, and the example binds to ColorState, so yeah! That was just the obvious way to do it, and SelectedColor is not recommended or used in any example at all, just listed.