3
\$\begingroup\$

I wrote a record which encapsulates conversion of colors between TColor, RGB values, CMYK values, and HSV/HSB values. It seems to work (at least for the purpose I made it), but I'm not too confident about the rest. Specifically the conversion between RGB and HSV. This record supports implicit conversion with TColor. I originally made it to read the HSV values from a TColor but figured I'd go all out and build something even more useful (CMYK was an added bonus).

Can you find anything wrong with how this works? Any changes I can make to improve its performance? I know there are some places in the code where the formatting is off, I have yet to clean all that up (mostly mix of uppercase/lowercase).

(*
  TColorRec - Helper record for TColor to easily interchange for simplicity of reading not only RGB but also CMYK and HSV.
  Supports directly assigning a TColor using implicit class operators.
*)

type
  TColorRec = record
  private
    FRed: Byte;
    FGreen: Byte;
    FBlue: Byte;
    function GetBrightness: Double;
    function GetHue: Double;
    function GetSaturation: Double;
    procedure SetBrightness(const Value: Double);
    procedure SetHue(const Value: Double);
    procedure SetSaturation(const Value: Double);
    function GetBlack: Integer;
    function GetCyan: Integer;
    function GetMagenta: Integer;
    function GetYellow: Integer;
    procedure SetBlack(const Value: Integer);
    procedure SetCyan(const Value: Integer);
    procedure SetMagenta(const Value: Integer);
    procedure SetYellow(const Value: Integer);
  public
    class operator implicit(Value: TColorRec): TColor;
    class operator implicit(Value: TColor): TColorRec;
    property Red: Byte read FRed write FRed;
    property Green: Byte read FGreen write FGreen;
    property Blue: Byte read FBlue write FBlue;
    property Hue: Double read GetHue write SetHue;
    property Saturation: Double read GetSaturation write SetSaturation;
    property Brightness: Double read GetBrightness write SetBrightness;
    property Cyan: Integer read GetCyan write SetCyan;
    property Magenta: Integer read GetMagenta write SetMagenta;
    property Yellow: Integer read GetYellow write SetYellow;
    property Black: Integer read GetBlack write SetBlack;
  end;

implementation

function RGBToHSV(R, G, B: Byte; var H, S, V: Double): Boolean;
var
  minRGB, maxRGB, delta: Double;
begin
  h := 0.0;
  minRGB := Min(Min(R, G), B);
  maxRGB := Max(Max(R, G), B);
  delta := (maxRGB - minRGB);
  V := maxRGB;
  if (maxRGB <> 0.0) then
    S := 255.0 * delta / maxRGB
  else
    S := 0.0;
  if (S <> 0.0) then begin
    if R = maxRGB then
      H := (G - B) / delta
    else if G = maxRGB then
      H := 2.0 + (B - R) / delta
    else if B = maxRGB then
      H := 4.0 + (R - G) / delta
  end else
    H := -1.0;
  H := h * 60;
  if H < 0.0 then
    H := H + 360.0;

  //S := S * 100 / 255;
  //V := B * 100 / 255;
  S := S / 255;
  V := V / 255;

  Result:= True;
end;

function HSVToRGB(H, S, V: Double; var R, G, B: Byte): Boolean;
var
  i: Integer;
  f, p, q, t: Double;
  procedure CopyOutput(const RV, GV, BV: Double);
  const
    RGBmax = 255;
  begin
    R:= Round(RGBmax * RV);
    G:= Round(RGBmax * GV);
    B:= Round(RGBmax * BV);
  end;
begin
  Assert(InRange(H, 0.0, 1.0));
  Assert(InRange(S, 0.0, 1.0));
  Assert(InRange(V, 0.0, 1.0));
  if S = 0.0 then begin
    // achromatic (grey)
    CopyOutput(B, B, B);
    Result:= True;
    exit;
  end;
  H := H * 6.0; // sector 0 to 5
  i := floor(H);
  f := H - i; // fractional part of H
  p := V * (1.0 - S);
  q := V * (1.0 - S * f);
  t := V * (1.0 - S * (1.0 - f));
  case i of
    0: CopyOutput(V, t, p);
    1: CopyOutput(q, V, p);
    2: CopyOutput(p, V, t);
    3: CopyOutput(p, q, V);
    4: CopyOutput(t, p, V);
    else CopyOutput(V, p, q);
  end;
  Result:= True;
end;

{ TColorRec }

class operator TColorRec.implicit(Value: TColorRec): TColor;
begin
  with Value do
    Result:= RGB(Red, Green, Blue);
end;

class operator TColorRec.implicit(Value: TColor): TColorRec;
begin
  with Result do begin
    FRed:= GetRValue(Value);
    FGreen:= GetGValue(Value);
    FBlue:= GetBValue(Value);
  end;
end;

function TColorRec.GetHue: Double;
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  Result:= H;
end;

function TColorRec.GetSaturation: Double;
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  Result:= S;
end;

function TColorRec.GetBrightness: Double;
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  Result:= V;
end;

function TColorRec.GetCyan: Integer;
begin
  Result:= GetCValue(RGB(FRed, FGreen, FBlue));
end;

function TColorRec.GetMagenta: Integer;
begin
  Result:= GetMValue(RGB(FRed, FGreen, FBlue));
end;

function TColorRec.GetYellow: Integer;
begin
  Result:= GetYValue(RGB(FRed, FGreen, FBlue));
end;

function TColorRec.GetBlack: Integer;
begin
  Result:= GetKValue(RGB(FRed, FGreen, FBlue));
end;

procedure TColorRec.SetBrightness(const Value: Double);
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  V:= Value;
  HSVToRGB(H, S, V, FRed, FGreen, FBlue);
end;

procedure TColorRec.SetHue(const Value: Double);
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  H:= Value;
  HSVToRGB(H, S, V, FRed, FGreen, FBlue);
end;

procedure TColorRec.SetSaturation(const Value: Double);
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  S:= Value;
  HSVToRGB(H, S, V, FRed, FGreen, FBlue);
end;

procedure TColorRec.SetCyan(const Value: Integer);
begin
  Self:= CMYK(Value, Magenta, Yellow, Black);
end;

procedure TColorRec.SetMagenta(const Value: Integer);
begin
  Self:= CMYK(Cyan, Value, Yellow, Black);
end;

procedure TColorRec.SetYellow(const Value: Integer);
begin
  Self:= CMYK(Cyan, Magenta, Value, Black);
end;

procedure TColorRec.SetBlack(const Value: Integer);
begin
  Self:= CMYK(Cyan, Magenta, Yellow, Value);
end;
\$\endgroup\$
  • \$\begingroup\$ I've already began my own notes: 1) RGB/HSV conversion function results are not assigned as they should be, 2) I should add an Alpha channel to RGB since technically it's ARGB \$\endgroup\$ – Jerry Dodge Feb 1 '15 at 1:21
  • 1
    \$\begingroup\$ I would consider this to be the (real) record helper (if the targeted Delphi version supports it). \$\endgroup\$ – TLama Feb 2 '15 at 7:32
  • \$\begingroup\$ Double variables in hsv routine are overkill. use single. \$\endgroup\$ – Marco van de Voort Apr 13 '17 at 8:40
3
\$\begingroup\$

What is the purpose of these conversions? How are they going to be used? I ask because I typically have 2 use cases for HSV colors:

  1. converting an image into the color space for some app or library that works in that space
  2. converting an individual pixel into HSV (or whatever) to do a particular manipulation on it, then converting back to RGB

In the first case, there are generally constraints, such as that the resulting image must have 8-bits per channel. In the second case, I usually want floating point precision for doing the color manipulation, though. It seems very odd to have CMYK be Integers instead of Bytes like RGB. What's the reasoning there?

The answer to the above question (the purpose of the conversions) will determine what format you should return the values in. Right now, the HSV conversion takes 8-bit unsigned input values but returns double-precision floats. That's useful for #2. Is there any reason the input RGB values couldn't also be double-precision floats?

S and V are in the range 0-1, but H is 0-360. It's been years since I used Delphi, but most math functions in most other languages take radians. Are you sure you want hue to be in degrees? I have seen implementations where H is 0-1 (representing 0° to 360°) for consistency, but it's a pain to pass to math functions. In the HSVToRGB() function you assert that they're in the 0-1 range, which H definitely won't be!

If you do want S to be in the 0-1 range, then don't multiply it by 255 here:

if (maxRGB <> 0.0) then
    S := 255.0 * delta / maxRGB

and then divide by 255 here:

  S := S / 255;

Also, why is RGBToHSV()a function? It always returns True. Same with HSVToRGB().

The procedure name CopyOutput() is confusing. You aren't copying any values. You're scaling the inputs, so I'd call it ScaleComponents() or something like that. In this case:

if S = 0.0 then begin
    // achromatic (grey)
    CopyOutput(B, B, B);
    Result:= True;
    exit;
end;

You're using the blue channel, which hasn't been set yet, to set R, G, and B. Shouldn't that be:

CopyOutput(V, V, V);

Further down in HSVToRGB() you write:

H := H * 6.0; // sector 0 to 5

If H is in the 0-360 range, it should be:

H := H / 60.0;

It seems wasteful to have setters and getters for single channels but not have them for all 3 (or 4) channels at a time. For example, if a caller is asking for Hue, it's likely they'll ask for Saturation and Value, too. So it would be nice to have some way to convert all 3 at once. Otherwise, the conversion is done 3 times and there's 3 times the function-call overhead.

For the methods SetHue(), SetSaturation(), and SetBrightness(), I wouldn't call the argument Value since it can be confused with the V component of HSV. I would name the argument NewHue, NewSaturation and NewBrightness.

It looks like you're using inherited conversions for CMYK. Am I understanding that correctly? Since I don't know the TColor class, I don't have many thoughts on that. If it's currently working for you, then I guess it probably works OK.

\$\endgroup\$
2
\$\begingroup\$

Putting this statement before S := S / 255, makes the record consistent for handling HSV:

H := H/360.0; //for making the 0..360 degrees map to 0..1 as required in the assert

Additionally, these setter,getter can help working with degree values of Hue which is standard for web colors:

procedure TColorRec.SetHueDegrees(const Value: Double);
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  H:= Value/360;   //Assuming H is always in the range 0..1 (for 0..360)
  HSVToRGB(H, S, V, FRed, FGreen, FBlue);
end;

function TColorRec.GetHueDegrees: Double;
var
  H, S, V: Double;
begin
  RGBToHSV(FRed, FGreen, FBlue, H, S, V);
  Result:= H*360;      //Assuming H is always in the range 0..1 (for 0..360)
end;
\$\endgroup\$

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.