Change one value + many OnChange events = stack overflow

[WojTec]

Sorry guys, I must ask yet another question, I know I'm bothering you, but I can't fix it my self :(

I'm trying make own color picker.
I have visual picker and edits with values. All componenta has OnChange events to update rest values.

And you know what next: after change one value all OnChange events are handled and finally program crashes with stack overflow.

How it should be to after one change just update rest controls and don't call OnChange from these controls (just on OnChange from control which is actually touched).

[DeddyH]

Maybe you could show us some code?

[wicht]

Do you update any fields' value in any of the OnChange handlers? That could explain the Stack-Overflow. I think you need to detach the OnChange event when one Event is fired, then change the values for the other fields (so OnChange will not be fired), then reassign the Events.

[WojTec]

Sure, this is sample:

Delphi-Quellcode:

OnFormCreate:
UpdateColor(clWhite);

OnHSLChange:
UpdateColor(HSLPicker.SelectedColor);

OnRGBChange:
UpdateColor(RGB(R, G, B));

OnHSLChange:
HSLToRGB()
UpdateColor()

OnCMYKChane:
CMYKToRGB()
UpdateColor()

UpdateColor(const AColor: TColor):
Preview.Color := AColor;
GetRGBValues(Preview.Color, R, G, B);
edtR.Value := R;
// ... get RGB, HSL and CMYK and set edits

HSLPicker.SelectedColor := AColor;

Picker is the one from mbColorLib.

[Furtbichler]

Which programming language?
You should provide sufficient information, complete and easy to understand.
We like to help, but not if we have to solve puzzles in order to do so.

BTW, you have a circle in your dependencies, i.e.
1. if A changes, then also change B
2. if B changes, then also change A

If you change either A or B, you'll get an endless loop.
To avoid this: Only change if values differ:

1. if A changes and B<>A then change B.
2. if B changes and A<>B then change A.

[Christian Seehase]

Hallo,

in UpdateColor you should switch off the OnHSLChange-Event.
If you dont, you receive an endless loop, because you call UpdateColor in HSLPicker.OnChange.

e.g.:

Delphi-Quellcode:

UpdateColor(const AColor: TColor):

var
  neSave : TNotifyEvent;

begin
  neSave := HSLPicker.OnChange;
  try
    HSLPicker.OnChange := nil;
    Preview.Color := AColor;
    GetRGBValues(Preview.Color, R, G, B);
    edtR.Value := R;
    // ... get RGB, HSL and CMYK and set edits

    HSLPicker.SelectedColor := AColor; 
  finally
    HSLPicker.OnChange := neSave;
  end;
end;

[WojTec]

This is what I have:

Delphi-Quellcode:

procedure TMainForm.RGBChange(Sender: TObject);
begin
  UpdateColor(Sender, RGBChange, SetRGBValues(edtR.AsInteger, edtG.AsInteger, edtB.AsInteger));
end;

procedure TMainForm.HSLChange(Sender: TObject);
var
  R, G, B: Byte;
begin
  HSLRangeToRGB(edtH.AsInteger, edtS.AsInteger, edtL.AsInteger, R, G, B);

  UpdateColor(Sender, HSLChange, SetRGBValues(R, G, B));
end;

procedure TMainForm.CMYKChange(Sender: TObject);
var
  R, G, B: Byte;
begin
  CMYKToRGB(edtC.AsInteger, edtM.AsInteger, edtY.AsInteger, edtK.AsInteger, R, G, B);

  UpdateColor(Sender, CMYKChange, SetRGBValues(R, G, B));
end;

procedure TMainForm.ColorPaletteCellClick(Button: TMouseButton;
  Shift: TShiftState; Index: Integer; AColor: TColor; var DontCheck: Boolean);
var
  C: TColor;
begin
  if AColor = clNone then
    C := clBlack
  else
    C := AColor
  ;

  UpdateColor(nil, nil, C);
end;

procedure TMainForm.FavouritePaletteCellClick(Button: TMouseButton;
  Shift: TShiftState; Index: Integer; AColor: TColor; var DontCheck: Boolean);
var
  C: TColor;
begin
  if AColor = clNone then
    C := clBlack
  else
    C := AColor
  ;

  UpdateColor(nil, nil, C);
end;

procedure TMainForm.FormClick(Sender: TObject);
begin
  edtR.OnChange := nil;
  edtR.AsInteger := 0;
  edtR.OnChange := RGBChange;
end;

procedure TMainForm.FormCreate(Sender: TObject);
begin
  Randomize;
  UpdateColor(nil, nil, RandomColor);
end;

procedure TMainForm.HSLPickerChange(Sender: TObject);
begin
  UpdateColor(HSLPicker, HSLPickerChange, HSLPicker.SelectedColor);
end;

procedure TMainForm.HSVPickerChange(Sender: TObject);
begin
  LightnessPicker.Saturation := HSVPicker.Saturation;
  LightnessPicker.Hue := HSVPicker.Hue;
  UpdateColor(HSVPicker, HSVPickerChange, HSVPicker.SelectedColor);
end;

procedure TMainForm.HuePickerChange(Sender: TObject);
begin
  SLPicker.Hue := HuePicker.Hue;
end;

procedure TMainForm.LightnessPickerChange(Sender: TObject);
begin
  HSVPicker.Value := LightnessPicker.Value;
end;

procedure TMainForm.PageControlChange(Sender: TObject);
begin
  UpdateColor(nil, nil, ColorPreview.Color);
end;

procedure TMainForm.SLPickerChange(Sender: TObject);
begin
  UpdateColor(SLPicker, SLPickerChange, SLPicker.SelectedColor);
end;

procedure TMainForm.DisableAutoChange(Sender: TObject; Event: TNotifyEvent);
begin
  HSLPicker.OnChange := nil;

  SLPicker.OnChange := nil;
  HuePicker.OnChange := nil;

  HSVPicker.OnChange := nil;
  LightnessPicker.OnChange := nil;

  edtR.OnChange := nil;
  edtG.OnChange := nil;
  edtB.OnChange := nil;

  edtH.OnChange := nil;
  edtS.OnChange := nil;
  edtL.OnChange := nil;

  edtM.OnChange := nil;
  edtM.OnChange := nil;
  edtY.OnChange := nil;
  edtK.OnChange := nil;

  edtCIEL.OnChange := nil;
  edtCIEa.OnChange := nil;
  edtCIEb.OnChange := nil;
end;

procedure TMainForm.EnableAutoChange;
begin
  HSLPicker.OnChange := HSLPickerChange;

  SLPicker.OnChange := SLPickerChange;
  HuePicker.OnChange := HuePickerChange;

  HSVPicker.OnChange := HSVPickerChange;
  LightnessPicker.OnChange := LightnessPickerChange;

  edtR.OnChange := RGBChange;
  edtG.OnChange := RGBChange;
  edtB.OnChange := RGBChange;

  edtH.OnChange := HSLChange;
  edtS.OnChange := HSLChange;
  edtL.OnChange := HSLChange;

  edtM.OnChange := CMYKChange;
  edtM.OnChange := CMYKChange;
  edtY.OnChange := CMYKChange;
  edtK.OnChange := CMYKChange;

  edtCIEL.OnChange := nil;
  edtCIEa.OnChange := nil;
  edtCIEb.OnChange := nil;
end;

procedure TMainForm.UpdateColor(Sender: TObject; Event: TNotifyEvent; const AColor: TColor);
var
  R, G, B: Byte;
  H, S, L: Int32;
  C, M, Y, K: Byte;
  CIEL, CIEa, CIEb: Double;
begin
  DisableAutoChange(Sender, Event);
  try
    ColorPreview.Color := AColor;

    GetRGBValues(ColorPreview.Color, R, G, B);
    edtR.AsInteger := R;
    edtG.AsInteger := G;
    edtB.AsInteger := B;

    RGBToHSLRange(R, G, B, H, S, L);
    edtH.AsInteger := H;
    edtS.AsInteger := S;
    edtL.AsInteger := L;
    edtH.GuageBeginColor := HSLRangeHueToColor(H);
    edtH.GuageEndColor := edtH.GuageBeginColor;

    edtS.GuageBeginColor := clWhite;
    edtS.GuageEndColor := edtH.GuageBeginColor;

    edtL.GuageBeginColor := clBlack;
    edtL.GuageEndColor := edtH.GuageBeginColor;

    RGBToCMYK(R, G, B, C, M, Y, K);
    edtC.AsInteger := C;
    edtM.AsInteger := M;
    edtY.AsInteger := Y;
    edtK.AsInteger := K;

    RGBToLab(ColorPreview.Color, CIEL, CIEa, CIEb);
    edtCIEL.Value := CIEL;
    edtCIEa.Value := CIEa;
    edtCIEb.Value := CIEb;

    HSLPicker.SelectedColor := AColor;

    SLPicker.SelectedColor := AColor;
    HuePicker.Hue := SLPicker.Hue;

    HSVPicker.SelectedColor := AColor;
    LightnessPicker.Saturation := HSVPicker.Saturation;
    LightnessPicker.Hue := HSVPicker.Hue;
    LightnessPicker.Value := HSVPicker.Value;
  finally
    EnableAutoChange;
  end;
end;

initialization
  MaxHue := 360;
  MaxSat := 100;
  MaxLum := 100;

end.

:(

[Sir Rufo]

1. EnableAutoChange/DisableAutoChange
   IMHO it would be better to use a Flag to dis-/enable the execution of the Event-Code
   
   Delphi-Quellcode:

   procedure TMainForm.RGBChange(Sender: TObject);
   begin
     if not EnabledUpdateFlag then
       Exit;
     UpdateColor(Sender, RGBChange, SetRGBValues(edtR.AsInteger, edtG.AsInteger, edtB.AsInteger));
   end;
   
   procedure TMainForm.UpdateColor(Sender: TObject; Event: TNotifyEvent; const AColor: TColor);
   var
     R, G, B: Byte;
     H, S, L: Int32;
     C, M, Y, K: Byte;
     CIEL, CIEa, CIEb: Double;
   begin
     EnabledUpdateFlag := False;
     try
   
       ...
   
     finally
       EnableUpdateFlag := True;
     end;
   end;

   or like this
   
   Delphi-Quellcode:

   procedure TMainForm.UpdateColor(Sender: TObject; Event: TNotifyEvent; const AColor: TColor);
   var
     R, G, B: Byte;
     H, S, L: Int32;
     C, M, Y, K: Byte;
     CIEL, CIEa, CIEb: Double;
   begin
     if not EnabledUpdateFlag then
       Exit;
   
     EnabledUpdateFlag := False;
     try
   
       ...
   
     finally
       EnableUpdateFlag := True;
     end;
   end;

   Now u don't have to set/reset all the Event-Properties (and u can't miss one of them)
   
   
2. Useless Method-Parameters
   What are the parameters used for ... seems to me useless
   
   Delphi-Quellcode:

   procedure TMainForm.DisableAutoChange(Sender: TObject; Event: TNotifyEvent);
   begin // Sender and Event are never used in this Method, so they are useless!
     HSLPicker.OnChange := nil;
   
     SLPicker.OnChange := nil;
     HuePicker.OnChange := nil;
   
     HSVPicker.OnChange := nil;
     LightnessPicker.OnChange := nil;
   
     edtR.OnChange := nil;
     edtG.OnChange := nil;
     edtB.OnChange := nil;
   
     edtH.OnChange := nil;
     edtS.OnChange := nil;
     edtL.OnChange := nil;
   
     edtM.OnChange := nil;
     edtM.OnChange := nil;
     edtY.OnChange := nil;
     edtK.OnChange := nil;
   
     edtCIEL.OnChange := nil;
     edtCIEa.OnChange := nil;
     edtCIEb.OnChange := nil;
   end;

[WojTec]

I tried with variable too, so why don't worked for me :cry: Now working :D Just one question, values in edits are updated when I moving mouse on picker, but color in box is not updated in same time as these edits. Why?

[Sir Rufo]

Did u check with the Debugger?

Step trough ur code with the Debugger and u will find the "missing link"