View Issue Details

IDProjectCategoryView StatusLast Update
0035618LazarusTAChartpublic2019-05-22 18:04
ReporterMarcin WiazowskiAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN)Product Build61262 
Target VersionFixed in Version 
Summary0035618: TAChart: some cleaning up in Draw() methods
DescriptionIn 0035388:0115881, there is an update_ver2.diff patch attached, which - among others - added "if IsEmpty then exit" statements to TLineSeries.Draw() and TPolarSeries.Draw(). This was needed, because these series weren't working properly with TCalculatedChartSource - TCalculatedChartSource returns XCount = YCount = MaxInt when it is empty, so TLineSeries and TPolarSeries were trying to perform some actions in a loop from 0 to MaxInt-1 (or MaxInt-2). Adding the "if IsEmpty then exit" statements solved the problem.


On the other hand, performing drawing on series, that are not active, is useless (this can occur, because Draw() is a public method) - this was already discussed in 0035207:0114745 (see last lines).


Currently, some series exit their Draw() methods when empty, some other when not active, and some other perform both checks. So I reviewed all the series to make some cleaning up: it's useful to check for "not Active", and it's also good to check for "IsEmpty" in all series, to avoid arising problems like with TCalculatedChartSource (I'm sure that nobody will test every future change, in any of the Draw() methods, by attaching the series to TCalculatedChartSource).


So I'm attaching a patch, which just introduces the same "if IsEmpty or (not Active) then exit" check in almost all series (there are few exceptions from this rule - see below). As a consequence, checks for "Count = 0" were replaced with checks for "IsEmpty"; although this may seem to be equivalent, IsEmpty() is a virtual method, which may be overridden in descendant classes - so there should be no hardcoded checks for just "Count = 0".


Exceptions:

1) In TExpressionSeries.Draw() and TExpressionColorMapSeries.Draw(), "IsEmpty" cannot be determined before the "SetupParser" call. And, directly below the "SetupParser" call, there is an "inherited" call - which calls "IsEmpty" internally.

2) In TFuncSeries.Draw(), "IsEmpty" is not initially called, to handle csDesigning state. But, below, there is an "inherited" call - which calls "IsEmpty" internally.

3) In TParametricCurveSeries.Draw(), "IsEmpty" is not initially called, to handle csDesigning state - the "IsEmpty" call is below.
TagsNo tags attached.
Fixed in Revision61268
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • patch.diff (6,282 bytes)
    Index: components/tachart/tacustomseries.pas
    ===================================================================
    --- components/tachart/tacustomseries.pas	(revision 61262)
    +++ components/tachart/tacustomseries.pas	(working copy)
    @@ -890,7 +890,7 @@
     var
       i: Integer;
     begin
    -  if not Active or (Count = 0) then exit;
    +  if IsEmpty or (not Active) then exit;
       with Extent do
         for i := Low(coords) to High(coords) do
           if not IsInfinite(coords[i]) then
    @@ -1952,7 +1952,7 @@
       ysum: Double;
     begin
       if not Marks.IsMarkLabelsVisible or not Marks.AutoMargins then exit;
    -  if Count = 0 then exit;
    +  if IsEmpty then exit;
     
       {FLoBound and FUpBound fields may be outdated here (if axis' range has been
        changed after the last series' painting). FLoBound and FUpBound will be fully
    Index: components/tachart/taexpressionseries.pas
    ===================================================================
    --- components/tachart/taexpressionseries.pas	(revision 61262)
    +++ components/tachart/taexpressionseries.pas	(working copy)
    @@ -537,6 +537,7 @@
     
     procedure TExpressionSeries.Draw(ADrawer: IChartDrawer);
     begin
    +  if (not Active) then exit;
       SetupParser;
       inherited;
     end;
    @@ -672,6 +673,7 @@
     
     procedure TExpressionColorMapSeries.Draw(ADrawer: IChartDrawer);
     begin
    +  if (not Active) then exit;
       SetupParser;
       inherited;
     end;
    Index: components/tachart/tafuncseries.pas
    ===================================================================
    --- components/tachart/tafuncseries.pas	(revision 61262)
    +++ components/tachart/tafuncseries.pas	(working copy)
    @@ -1,4 +1,4 @@
    -{
    +{
     
      Function series for TAChart.
     
    @@ -729,7 +729,7 @@
     var
       R: TRect;
     begin
    -  if not Active then exit;
    +  if (not Active) then exit;
     
       if csDesigning in ComponentState then begin
         with ParentChart do begin
    @@ -819,7 +819,7 @@
       t, ts, ms: Double;
       p, pp: TPoint;
     begin
    -  if not Active then exit;
    +  if (not Active) then exit;
     
       ADrawer.SetBrushParams(bsClear, clTAColor);
       ADrawer.Pen := Pen;
    @@ -1121,7 +1121,7 @@
     var
       i: Integer;
     begin
    -  if IsEmpty then exit;
    +  if IsEmpty or (not Active) then exit;
     
       SetLength(p, Degree + 1);
     
    @@ -2425,7 +2425,7 @@
       if FPaletteMin < FPaletteMax then begin
         cmin := FPaletteMin;
         cmax := FPaletteMax;
    -  end else
    +   end else
       if FPaletteMax < FPaletteMin then begin
         cmin := FPaletteMax;
         cmax := FPaletteMin;
    @@ -2512,7 +2512,7 @@
       scaled_stepX: Integer;
       scaled_stepY: Integer;
     begin
    -  if not (csDesigning in ComponentState) and IsEmpty then exit;
    +  if (not (csDesigning in ComponentState) and IsEmpty) or (not Active) then exit;
     
       ext := ParentChart.CurrentExtent;
       bounds := EmptyExtent;
    Index: components/tachart/tamultiseries.pas
    ===================================================================
    --- components/tachart/tamultiseries.pas	(revision 61262)
    +++ components/tachart/tamultiseries.pas	(working copy)
    @@ -516,7 +516,7 @@
       ext: TDoubleRect;
       nx, ny: Cardinal;
     begin
    -  if IsEmpty then exit;
    +  if IsEmpty or (not Active) then exit;
       if not RequestValidChartScaling then exit;
     
       ADrawer.Pen := BubblePen;
    @@ -876,7 +876,7 @@
       center: Double;
     begin
       if not Marks.IsMarkLabelsVisible or not Marks.AutoMargins then exit;
    -  if Count = 0 then exit;
    +  if IsEmpty then exit;
       if not RequestValidChartScaling then exit;
     
       FindExtentInterval(ParentChart.CurrentExtent, Source.IsSortedByXAsc);
    @@ -984,8 +984,8 @@
       i: Integer;
       nx, ny: Cardinal;
     begin
    -  if IsEmpty then
    -    exit;
    +  if IsEmpty or (not Active) then exit;
    +
       if FWidthStyle = bwsPercentMin then
         UpdateMinXRange;
     
    @@ -1432,8 +1432,9 @@
       p: TPen;
       nx, ny: Cardinal;
     begin
    +  if IsEmpty or (not Active) then exit;
       my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
    -  if IsEmpty or (my >= Source.YCount) then exit;
    +  if my >= Source.YCount then exit;
     
       ext2 := ParentChart.CurrentExtent;
       ExpandRange(ext2.a.X, ext2.b.X, 1.0);
    @@ -1807,6 +1808,7 @@
       p1, p2: TDoublePoint;
       lPen: TPen;
     begin
    +  if IsEmpty or (not Active) then exit;
       with Extent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
    Index: components/tachart/taradialseries.pas
    ===================================================================
    --- components/tachart/taradialseries.pas	(revision 61262)
    +++ components/tachart/taradialseries.pas	(working copy)
    @@ -627,7 +627,7 @@
       prevLabelPoly: TPointArray = nil;
       ps: TPieSlice;
     begin
    -  if IsEmpty then exit;
    +  if IsEmpty or (not Active) then exit;
     
       Marks.SetAdditionalAngle(0);
       Measure(ADrawer);
    @@ -1263,8 +1263,7 @@
     var
       j: Integer;
     begin
    -  if IsEmpty then exit;
    -
    +  if IsEmpty or (not Active) then exit;
       originPt := ParentChart.GraphToImage(DoublePoint(OriginX, OriginY));
       fill := FFilled and (FBrush.Style <> bsClear);
       SetLength(pts, Count + 1);  // +1 for origin
    Index: components/tachart/taseries.pas
    ===================================================================
    --- components/tachart/taseries.pas	(revision 61262)
    +++ components/tachart/taseries.pas	(working copy)
    @@ -469,8 +469,7 @@
       ext: TDoubleRect;
       i: Integer;
     begin
    -  if IsEmpty then exit;
    -
    +  if IsEmpty or (not Active) then exit;
       with Extent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
    @@ -855,6 +854,7 @@
       end;
     
     begin
    +  if IsEmpty or (not Active) then exit;
       with Extent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
    @@ -946,6 +946,7 @@
     var
       p: Integer;
     begin
    +  if IsEmpty or (not Active) then exit;
       if Pen.Style = psClear then exit;
     
       ADrawer.SetBrushParams(bsClear, clTAColor);
    @@ -1227,7 +1228,7 @@
       ofs, y: Double;
       zero: Double;
     begin
    -  if IsEmpty then exit;
    +  if IsEmpty or (not Active) then exit;
     
       if BarWidthStyle = bwPercentMin then
         UpdateMinXRange;
    @@ -2132,7 +2133,7 @@
     var
       j, k: Integer;
     begin
    -  if IsEmpty then exit;
    +  if IsEmpty or (not Active) then exit;
     
       ext := ParentChart.CurrentExtent;
       ext2 := ext;
    @@ -2280,6 +2281,7 @@
     var
       ic: IChartTCanvasDrawer;
     begin
    +  if IsEmpty or (not Active) then exit;
       if Supports(ADrawer, IChartTCanvasDrawer, ic) and Assigned(FOnDraw) then
         FOnDraw(ic.Canvas, FChart.ClipRect);
     end;
    
    patch.diff (6,282 bytes)

Activities

Marcin Wiazowski

2019-05-22 00:01

reporter  

patch.diff (6,282 bytes)
Index: components/tachart/tacustomseries.pas
===================================================================
--- components/tachart/tacustomseries.pas	(revision 61262)
+++ components/tachart/tacustomseries.pas	(working copy)
@@ -890,7 +890,7 @@
 var
   i: Integer;
 begin
-  if not Active or (Count = 0) then exit;
+  if IsEmpty or (not Active) then exit;
   with Extent do
     for i := Low(coords) to High(coords) do
       if not IsInfinite(coords[i]) then
@@ -1952,7 +1952,7 @@
   ysum: Double;
 begin
   if not Marks.IsMarkLabelsVisible or not Marks.AutoMargins then exit;
-  if Count = 0 then exit;
+  if IsEmpty then exit;
 
   {FLoBound and FUpBound fields may be outdated here (if axis' range has been
    changed after the last series' painting). FLoBound and FUpBound will be fully
Index: components/tachart/taexpressionseries.pas
===================================================================
--- components/tachart/taexpressionseries.pas	(revision 61262)
+++ components/tachart/taexpressionseries.pas	(working copy)
@@ -537,6 +537,7 @@
 
 procedure TExpressionSeries.Draw(ADrawer: IChartDrawer);
 begin
+  if (not Active) then exit;
   SetupParser;
   inherited;
 end;
@@ -672,6 +673,7 @@
 
 procedure TExpressionColorMapSeries.Draw(ADrawer: IChartDrawer);
 begin
+  if (not Active) then exit;
   SetupParser;
   inherited;
 end;
Index: components/tachart/tafuncseries.pas
===================================================================
--- components/tachart/tafuncseries.pas	(revision 61262)
+++ components/tachart/tafuncseries.pas	(working copy)
@@ -1,4 +1,4 @@
-{
+{
 
  Function series for TAChart.
 
@@ -729,7 +729,7 @@
 var
   R: TRect;
 begin
-  if not Active then exit;
+  if (not Active) then exit;
 
   if csDesigning in ComponentState then begin
     with ParentChart do begin
@@ -819,7 +819,7 @@
   t, ts, ms: Double;
   p, pp: TPoint;
 begin
-  if not Active then exit;
+  if (not Active) then exit;
 
   ADrawer.SetBrushParams(bsClear, clTAColor);
   ADrawer.Pen := Pen;
@@ -1121,7 +1121,7 @@
 var
   i: Integer;
 begin
-  if IsEmpty then exit;
+  if IsEmpty or (not Active) then exit;
 
   SetLength(p, Degree + 1);
 
@@ -2425,7 +2425,7 @@
   if FPaletteMin < FPaletteMax then begin
     cmin := FPaletteMin;
     cmax := FPaletteMax;
-  end else
+   end else
   if FPaletteMax < FPaletteMin then begin
     cmin := FPaletteMax;
     cmax := FPaletteMin;
@@ -2512,7 +2512,7 @@
   scaled_stepX: Integer;
   scaled_stepY: Integer;
 begin
-  if not (csDesigning in ComponentState) and IsEmpty then exit;
+  if (not (csDesigning in ComponentState) and IsEmpty) or (not Active) then exit;
 
   ext := ParentChart.CurrentExtent;
   bounds := EmptyExtent;
Index: components/tachart/tamultiseries.pas
===================================================================
--- components/tachart/tamultiseries.pas	(revision 61262)
+++ components/tachart/tamultiseries.pas	(working copy)
@@ -516,7 +516,7 @@
   ext: TDoubleRect;
   nx, ny: Cardinal;
 begin
-  if IsEmpty then exit;
+  if IsEmpty or (not Active) then exit;
   if not RequestValidChartScaling then exit;
 
   ADrawer.Pen := BubblePen;
@@ -876,7 +876,7 @@
   center: Double;
 begin
   if not Marks.IsMarkLabelsVisible or not Marks.AutoMargins then exit;
-  if Count = 0 then exit;
+  if IsEmpty then exit;
   if not RequestValidChartScaling then exit;
 
   FindExtentInterval(ParentChart.CurrentExtent, Source.IsSortedByXAsc);
@@ -984,8 +984,8 @@
   i: Integer;
   nx, ny: Cardinal;
 begin
-  if IsEmpty then
-    exit;
+  if IsEmpty or (not Active) then exit;
+
   if FWidthStyle = bwsPercentMin then
     UpdateMinXRange;
 
@@ -1432,8 +1432,9 @@
   p: TPen;
   nx, ny: Cardinal;
 begin
+  if IsEmpty or (not Active) then exit;
   my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
-  if IsEmpty or (my >= Source.YCount) then exit;
+  if my >= Source.YCount then exit;
 
   ext2 := ParentChart.CurrentExtent;
   ExpandRange(ext2.a.X, ext2.b.X, 1.0);
@@ -1807,6 +1808,7 @@
   p1, p2: TDoublePoint;
   lPen: TPen;
 begin
+  if IsEmpty or (not Active) then exit;
   with Extent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
Index: components/tachart/taradialseries.pas
===================================================================
--- components/tachart/taradialseries.pas	(revision 61262)
+++ components/tachart/taradialseries.pas	(working copy)
@@ -627,7 +627,7 @@
   prevLabelPoly: TPointArray = nil;
   ps: TPieSlice;
 begin
-  if IsEmpty then exit;
+  if IsEmpty or (not Active) then exit;
 
   Marks.SetAdditionalAngle(0);
   Measure(ADrawer);
@@ -1263,8 +1263,7 @@
 var
   j: Integer;
 begin
-  if IsEmpty then exit;
-
+  if IsEmpty or (not Active) then exit;
   originPt := ParentChart.GraphToImage(DoublePoint(OriginX, OriginY));
   fill := FFilled and (FBrush.Style <> bsClear);
   SetLength(pts, Count + 1);  // +1 for origin
Index: components/tachart/taseries.pas
===================================================================
--- components/tachart/taseries.pas	(revision 61262)
+++ components/tachart/taseries.pas	(working copy)
@@ -469,8 +469,7 @@
   ext: TDoubleRect;
   i: Integer;
 begin
-  if IsEmpty then exit;
-
+  if IsEmpty or (not Active) then exit;
   with Extent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
@@ -855,6 +854,7 @@
   end;
 
 begin
+  if IsEmpty or (not Active) then exit;
   with Extent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
@@ -946,6 +946,7 @@
 var
   p: Integer;
 begin
+  if IsEmpty or (not Active) then exit;
   if Pen.Style = psClear then exit;
 
   ADrawer.SetBrushParams(bsClear, clTAColor);
@@ -1227,7 +1228,7 @@
   ofs, y: Double;
   zero: Double;
 begin
-  if IsEmpty then exit;
+  if IsEmpty or (not Active) then exit;
 
   if BarWidthStyle = bwPercentMin then
     UpdateMinXRange;
@@ -2132,7 +2133,7 @@
 var
   j, k: Integer;
 begin
-  if IsEmpty then exit;
+  if IsEmpty or (not Active) then exit;
 
   ext := ParentChart.CurrentExtent;
   ext2 := ext;
@@ -2280,6 +2281,7 @@
 var
   ic: IChartTCanvasDrawer;
 begin
+  if IsEmpty or (not Active) then exit;
   if Supports(ADrawer, IChartTCanvasDrawer, ic) and Assigned(FOnDraw) then
     FOnDraw(ic.Canvas, FChart.ClipRect);
 end;
patch.diff (6,282 bytes)

wp

2019-05-22 11:33

reporter   ~0116328

Applied, thanks

Marcin Wiazowski

2019-05-22 18:04

reporter   ~0116343

Thanks!

Issue History

Date Modified Username Field Change
2019-05-22 00:01 Marcin Wiazowski New Issue
2019-05-22 00:01 Marcin Wiazowski File Added: patch.diff
2019-05-22 00:56 wp Assigned To => wp
2019-05-22 00:56 wp Status new => assigned
2019-05-22 11:33 wp Status assigned => resolved
2019-05-22 11:33 wp Resolution open => fixed
2019-05-22 11:33 wp Fixed in Revision => 61268
2019-05-22 11:33 wp LazTarget => -
2019-05-22 11:33 wp Widgetset Win32/Win64 => Win32/Win64
2019-05-22 11:33 wp Note Added: 0116328
2019-05-22 18:04 Marcin Wiazowski Status resolved => closed
2019-05-22 18:04 Marcin Wiazowski Note Added: 0116343