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

Forum > FPC development

(1/1)

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 [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomFont.DoCopyProps (From:TFPCanvasHelper);begin with from as TFPCustomFont do begin self.FName := FName; self.FSize := FSize; self.FFPColor := FFPColor;//Replaced by using inherited self.FFlags := FFlags;//Replaced by using inherited end;end;
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomFont.DoCopyProps (From:TFPCanvasHelper);begin with from as TFPCustomFont do begin self.FName := FName; self.FOrientation := FOrientation;//Added Orientation self.FSize := FSize; end; inherited DoCopyProps(From);//Added inheritedend;

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

--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomPen.DoCopyProps (From:TFPCanvasHelper);begin with From as TFPCustomPen do begin self.Style := Style; self.Width := Width; self.Mode := Mode; self.pattern := pattern; end; inherited;end;
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomPen.DoCopyProps (From:TFPCanvasHelper);begin with From as TFPCustomPen do begin self.Style := Style; self.Width := Width; self.Mode := Mode; self.Pattern := Pattern; self.EndCap := EndCap;//Added EndCap self.JoinStyle := JoinStyle;//Added JoinStyle end; inherited DoCopyProps(From);end;
procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper); ignores Pattern.

--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper);begin with From as TFPCustomBrush do begin self.Style := Style; self.Image := Image; end; inherited DoCopyProps(From);end;
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCustomBrush.DoCopyProps (From:TFPCanvasHelper);begin with From as TFPCustomBrush do begin self.Style := Style; self.Image := Image; 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. end; inherited DoCopyProps(From);end;
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 [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure SetFlags (index:integer; AValue:boolean); virtual;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 [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFPCanvasHelper.DoCopyProps (From:TFPCanvasHelper);begin FPColor := from.FPColor;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.

Navigation

[0] Message Index