[SOLVED] Improvement of DoCopyProps routines in fcl-image package

[lagprogramming]

I think DoCopyProps routines in the the fcl-image package have bugs. I added comments to routines, the first routine is the original and the second is the modified one.
procedure TFPCustomFont.DoCopyProps (From:TFPCanvasHelper); ignores Orientation.

Code: Pascal  [Select][+][-]

1. procedure TFPCustomFont.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with from as TFPCustomFont do
4.     begin
5.     self.FName := FName;
6.     self.FSize := FSize;
7.     self.FFPColor := FFPColor;//Replaced by using inherited
8.     self.FFlags := FFlags;//Replaced by using inherited
9.     end;
10. end;

Code: Pascal  [Select][+][-]

1. procedure TFPCustomFont.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with from as TFPCustomFont do
4.     begin
5.     self.FName := FName;
6.     self.FOrientation := FOrientation;//Added Orientation
7.     self.FSize := FSize;
8.     end;
9.   inherited DoCopyProps(From);//Added inherited
10. end;

procedure TFPCustomPen.DoCopyProps (From:TFPCanvasHelper); ignores EndCap and JoinStyle.

Code: Pascal  [Select][+][-]

1. procedure TFPCustomPen.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with From as TFPCustomPen do
4.     begin
5.     self.Style := Style;
6.     self.Width := Width;
7.     self.Mode := Mode;
8.     self.pattern := pattern;
9.     end;
10.   inherited;
11. end;

Code: Pascal  [Select][+][-]

1. procedure TFPCustomPen.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with From as TFPCustomPen do
4.     begin
5.     self.Style := Style;
6.     self.Width := Width;
7.     self.Mode := Mode;
8.     self.Pattern := Pattern;
9.     self.EndCap := EndCap;//Added EndCap
10.     self.JoinStyle := JoinStyle;//Added JoinStyle
11.     end;
12.   inherited DoCopyProps(From);
13. end;
14.  

procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper); ignores Pattern.

Code: Pascal  [Select][+][-]

1. procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with From as TFPCustomBrush do
4.     begin
5.     self.Style := Style;
6.     self.Image := Image;
7.     end;
8.   inherited DoCopyProps(From);
9. end;

Code: Pascal  [Select][+][-]

1. procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   with From as TFPCustomBrush do
4.     begin
5.     self.Style := Style;
6.     self.Image := Image;
7.     self.Pattern := Copy(Pattern, low(Pattern) ,Length(Pattern));//Added Pattern. Copy, not just assign it, in order to make sure it has a single reference.
8.     end;
9.   inherited DoCopyProps(From);
10. end;
11.  

1/2 TFPCustomFont.DoCopyProps uses variable names while TFPCustomPen.DoCopyProps and TFPCustomBrush.DoCopyProps use property names. They should look alike. Either all of them use property names, either all of them use private variables names.

2/2 In procedure TFPCustomFont.DoCopyProps lines self.FFPColor := FFPColor; and self.FFlags := FFlags; have been replaced with inherited DoCopyProps(From);
The problem is that procedure TFPCanvasHelper.DoCopyProps needs to be modified too, and it has no property declared to access variable FFlags, but has two protected routines:

Code: Pascal  [Select][+][-]

1. procedure SetFlags (index:integer; AValue:boolean); virtual;
2. function GetFlags (index:integer) : boolean; virtual;

Private variable FFlags is word and these routines use booleans. Regarding DoCopyProps, I think a FPC developer should look at TFPCanvasHelper code because in addition to FFlags and FFPColor, it has other private variables that may need to be copied.
procedure TFPCanvasHelper.DoCopyProps (From:TFPCanvasHelper); ignores everything except for FPColor.

Code: Pascal  [Select][+][-]

1. procedure TFPCanvasHelper.DoCopyProps (From:TFPCanvasHelper);
2. begin
3.   FPColor := from.FPColor;
4. end;

[AlexTP]

Posted to https://gitlab.com/freepascal.org/fpc/source/-/issues/40362

[lagprogramming]

Maybe the attached patch will speed things up.
Unlike the presented modified code the comments have been removed. Also, TFPCustomBrush.DoCopyProps will simply assign the pattern, not copy it. Because the pattern is an array, it means that modifying individual entries will change the behavior of all brushes using that pattern. I don't know if this is the right behavior but for sure it's better than simply ignoring the pattern, like the official code does in the present.