View Issue Details

IDProjectCategoryView StatusLast Update
0036442PatchesLCLpublic2019-12-16 00:21
ReporterCudaText manAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.1 (SVN)Product Build 
Target VersionFixed in Version 
Summary0036442: Canvas.PolyBezier fix
DescriptionAccording to Delphi docs, Delphi always paints continuous curves
http://docwiki.embarcadero.com/Libraries/Rio/en/Vcl.Graphics.TCanvas.PolyBezier
but LCL has default value of "Continuous" in PolyBezier() as False. fixed it.

Also added try/finally.
Also added check of minimal count of points in the curve as 4 points (bezier curve has 4 points: start, end, control1, control2).

TagsNo tags attached.
Fixed in Revision62404
LazTarget2.2
Widgetset
Attached Files
  • bez.diff (2,277 bytes)
    Index: lcl/graphics.pp
    ===================================================================
    --- lcl/graphics.pp	(revision 62400)
    +++ lcl/graphics.pp	(working copy)
    @@ -1105,10 +1105,10 @@
                       StartX,StartY,EndX,EndY: Integer); virtual;
         procedure PolyBezier(Points: PPoint; NumPts: Integer;
                              Filled: boolean = False;
    -                         Continuous: boolean = False); virtual; {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
    +                         Continuous: boolean = True); virtual; {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
         procedure PolyBezier(const Points: array of TPoint;
                              Filled: boolean = False;
    -                         Continuous: boolean = False); {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
    +                         Continuous: boolean = True); {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
         procedure Polygon(const Points: array of TPoint;
                           Winding: Boolean;
                           StartIndex: Integer = 0;
    Index: lcl/include/canvas.inc
    ===================================================================
    --- lcl/include/canvas.inc	(revision 62400)
    +++ lcl/include/canvas.inc	(working copy)
    @@ -834,22 +834,25 @@
      ------------------------------------------------------------------------------}
     procedure TCanvas.PolyBezier(const Points: array of TPoint;
       Filled: boolean = False;
    -  Continuous: boolean = False);
    +  Continuous: boolean = True);
     var NPoints, i: integer;
       PointArray: ^TPoint;
     begin
       NPoints:=High(Points)-Low(Points)+1;
    -  if NPoints<=0 then exit;
    +  if NPoints<4 then exit; // Curve must have at least 4 points
       GetMem(PointArray,SizeOf(TPoint)*NPoints);
    -  for i:=0 to NPoints-1 do
    -    PointArray[i]:=Points[i+Low(Points)];
    -  PolyBezier(PointArray, NPoints, Filled, Continuous);
    -  FreeMem(PointArray);
    +  try
    +    for i:=0 to NPoints-1 do
    +      PointArray[i]:=Points[i+Low(Points)];
    +    PolyBezier(PointArray, NPoints, Filled, Continuous);
    +  finally
    +    FreeMem(PointArray);
    +  end;
     end;
     
     procedure TCanvas.PolyBezier(Points: PPoint; NumPts: Integer;
       Filled: boolean = False;
    -  Continuous: boolean = False);
    +  Continuous: boolean = True);
     begin
       Changing;
       RequiredState([csHandleValid, csBrushValid, csPenValid]);
    
    bez.diff (2,277 bytes)
  • polybezier.zip (6,052 bytes)
  • polybezier_lazarus.png (3,829 bytes)
    polybezier_lazarus.png (3,829 bytes)
  • polybezier_delphi.png (2,918 bytes)
    polybezier_delphi.png (2,918 bytes)

Activities

CudaText man

2019-12-15 14:07

reporter  

bez.diff (2,277 bytes)
Index: lcl/graphics.pp
===================================================================
--- lcl/graphics.pp	(revision 62400)
+++ lcl/graphics.pp	(working copy)
@@ -1105,10 +1105,10 @@
                   StartX,StartY,EndX,EndY: Integer); virtual;
     procedure PolyBezier(Points: PPoint; NumPts: Integer;
                          Filled: boolean = False;
-                         Continuous: boolean = False); virtual; {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
+                         Continuous: boolean = True); virtual; {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
     procedure PolyBezier(const Points: array of TPoint;
                          Filled: boolean = False;
-                         Continuous: boolean = False); {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
+                         Continuous: boolean = True); {$IFDEF HasFPCanvas1}reintroduce;{$ENDIF}
     procedure Polygon(const Points: array of TPoint;
                       Winding: Boolean;
                       StartIndex: Integer = 0;
Index: lcl/include/canvas.inc
===================================================================
--- lcl/include/canvas.inc	(revision 62400)
+++ lcl/include/canvas.inc	(working copy)
@@ -834,22 +834,25 @@
  ------------------------------------------------------------------------------}
 procedure TCanvas.PolyBezier(const Points: array of TPoint;
   Filled: boolean = False;
-  Continuous: boolean = False);
+  Continuous: boolean = True);
 var NPoints, i: integer;
   PointArray: ^TPoint;
 begin
   NPoints:=High(Points)-Low(Points)+1;
-  if NPoints<=0 then exit;
+  if NPoints<4 then exit; // Curve must have at least 4 points
   GetMem(PointArray,SizeOf(TPoint)*NPoints);
-  for i:=0 to NPoints-1 do
-    PointArray[i]:=Points[i+Low(Points)];
-  PolyBezier(PointArray, NPoints, Filled, Continuous);
-  FreeMem(PointArray);
+  try
+    for i:=0 to NPoints-1 do
+      PointArray[i]:=Points[i+Low(Points)];
+    PolyBezier(PointArray, NPoints, Filled, Continuous);
+  finally
+    FreeMem(PointArray);
+  end;
 end;
 
 procedure TCanvas.PolyBezier(Points: PPoint; NumPts: Integer;
   Filled: boolean = False;
-  Continuous: boolean = False);
+  Continuous: boolean = True);
 begin
   Changing;
   RequiredState([csHandleValid, csBrushValid, csPenValid]);
bez.diff (2,277 bytes)

Juha Manninen

2019-12-15 22:04

developer   ~0119862

I don't see any mention of "continuous curve" in the Embarcadero's page.

CudaText man

2019-12-15 22:44

reporter   ~0119867

LCL docs
If the Continuous flag is TRUE then each subsequent curve requires three more points, using the end-point of the previous Curve as its starting point, the first and second points being used as its control points, and the third point its end-point.

If the continuous flag is set to FALSE, then each subsequent Curve requires 4 additional points, which are used exactly as in the first curve.
https://lazarus-ccr.sourceforge.io/docs/lcl/graphics/tcanvas.polybezier.html

----
Delphi docs
 Each subsequent curve in the sequence needs exactly three more points: the ending point of the previous curve is used as the starting point, the next two points in the sequence are control points, and the third is the ending point.
http://docwiki.embarcadero.com/Libraries/Rio/en/Vcl.Graphics.TCanvas.PolyBezier

==> it means LCL must have Continous as True.

wp

2019-12-15 23:22

developer   ~0119868

I am attaching a Lazarus/Delphi project which shows the difference. It draws a bezier curve for the same points on two Paintboxes, the left one with the Lazarus default Continuous = false, the right one with Continous = true. The two curves show marked differences, the left one has a discontinuous kink between the first and second segment (points #0 to 3 and points 0000004 to 7), the right one has a smooth joint.

Running on Delphi, we notice that the Continuous parameter is missing, Delphi draws the Bezier only in the Continuous mode, as can be seen in the screen shot.

Therefore we have a different behaviour of Lazarus default compared with Delphi here.

polybezier.zip (6,052 bytes)
polybezier_lazarus.png (3,829 bytes)
polybezier_lazarus.png (3,829 bytes)
polybezier_delphi.png (2,918 bytes)
polybezier_delphi.png (2,918 bytes)

Juha Manninen

2019-12-16 00:16

developer   ~0119870

Ok, I learned something new again. I thought all Bezier curves are continuous. The non-continuous variety looks quite useless to me. :)

wp

2019-12-16 00:19

developer   ~0119871

Last edited: 2019-12-16 00:21

View 2 revisions

Applied, thanks. Test and close if ok.

I will not ask for back-porting to fixes because the new behavior may break old Lazarus code. An incompatibility note was added to the v2.2 release notes (https://wiki.freepascal.org/Lazarus_2.2.0_release_notes#TCanvas.PolyBezier_default_behavior).

@Juha: I have the same impression...

Issue History

Date Modified Username Field Change
2019-12-15 14:07 CudaText man New Issue
2019-12-15 14:07 CudaText man File Added: bez.diff
2019-12-15 22:04 Juha Manninen Note Added: 0119862
2019-12-15 22:44 CudaText man Note Added: 0119867
2019-12-15 23:22 wp File Added: polybezier.zip
2019-12-15 23:22 wp File Added: polybezier_lazarus.png
2019-12-15 23:22 wp File Added: polybezier_delphi.png
2019-12-15 23:22 wp Note Added: 0119868
2019-12-15 23:38 wp Assigned To => wp
2019-12-15 23:38 wp Status new => assigned
2019-12-15 23:38 wp Status assigned => confirmed
2019-12-15 23:38 wp LazTarget => -
2019-12-16 00:16 Juha Manninen Note Added: 0119870
2019-12-16 00:19 wp Status confirmed => resolved
2019-12-16 00:19 wp Resolution open => fixed
2019-12-16 00:19 wp Fixed in Revision => 62404
2019-12-16 00:19 wp LazTarget - => 2.2
2019-12-16 00:19 wp Note Added: 0119871
2019-12-16 00:21 wp Note Edited: 0119871 View Revisions