View Issue Details

IDProjectCategoryView StatusLast Update
0035313LazarusTAChartpublic2019-04-06 03:23
ReporterMarcin WiazowskiAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN)Product Build60823 
Target Version2.2Fixed in Version 
Summary0035313: TAChart: added caching for cumulative and list extents
DescriptionCurrently, only basic extent is cached, as can be seen in TCustomChartSource.BasicExtent(). So I'm attaching a patch, that adds caching also for cumulative and list extents (that are used by multi-value data sources).

To make analysis of changes introduced by the patch easier, I'm attaching in fact two files:




* The attached patch_partial.diff introduces some initial code rearrangements:


1) Whenever I can see "BasicExtent" in the code, I immediately know what it is; and when I can see just "Extent", I'm not sure with which inheritance level I'm dealing with. So, to make the code more understandable, I decided to rename TCustomChartSource strict protected fields from:

  FExtent: TDoubleRect;
  FExtentIsValid: Boolean;

to:

  FBasicExtent: TDoubleRect;
  FBasicExtentIsValid: Boolean;

This change of course can be reverted, but I find it quite useful. It impacts only TCustomChartSource and TListChartSource.


2) Some lacking code was added to SetDataItemDefaults() procedure.


3) TCustomChartSource.CalcExtentXYList() and TCustomChartSource.ExtentCumulative() methods are very similar - some adjustments were made to make them even more similar. In addition, their code has been rearranged, as it will be needed for caching.


4) I renamed the private TListChartSource.ClearCaches() method to InitializeEmptyCaches(), because the name was no longer describing the current functionality properly.




* The attached patch_full.diff fully contains patch_partial.diff, and adds caching to TCustomChartSource and TListChartSource. I think that introduced changes are rather self-explaining.




The attached MultiExtentSpeedTest application gives results like:
- test ExtentCumulative: 1650 ms
- test ExtentList: 850 ms
- test ExtentXYList: 1650 ms

After applying patch_full.diff:
- test ExtentCumulative: 25 ms
- test ExtentList: 15 ms
- test ExtentXYList: 25 ms
TagsNo tags attached.
Fixed in Revision60852, 60843, 60846, 60848
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • MultiExtentSpeedTest.zip (2,616 bytes)
  • patch_partial.diff (10,289 bytes)
    Index: components/tachart/tacustomsource.pas
    ===================================================================
    --- components/tachart/tacustomsource.pas	(revision 60823)
    +++ components/tachart/tacustomsource.pas	(working copy)
    @@ -187,8 +187,8 @@
         procedure SortValuesInRange(
           var AValues: TChartValueTextArray; AStart, AEnd: Integer);
       strict protected
    -    FExtent: TDoubleRect;
    -    FExtentIsValid: Boolean;
    +    FBasicExtent: TDoubleRect;
    +    FBasicExtentIsValid: Boolean;
         FValuesTotal: Double;
         FValuesTotalIsValid: Boolean;
         FXCount: Cardinal;
    @@ -308,6 +308,8 @@
       AItem.Y := 0;
       AItem.Color := clTAColor;
       AItem.Text := '';
    +  for i := 0 to High(AItem.XList) do
    +    AItem.XList[i] := 0;
       for i := 0 to High(AItem.YList) do
         AItem.YList[i] := 0;
     end;
    @@ -784,19 +786,19 @@
       i: Integer;
       vhi, vlo: Double;
     begin
    -  if FExtentIsValid then exit(FExtent);
    -  FExtent := EmptyExtent;
    +  if FBasicExtentIsValid then exit(FBasicExtent);
    +  FBasicExtent := EmptyExtent;
     
       if XCount > 0 then begin
         if HasXErrorBars then
           for i := 0 to Count - 1 do begin
             GetXErrorBarLimits(i, vhi, vlo);
    -        UpdateMinMax(vhi, FExtent.a.X, FExtent.b.X);
    -        UpdateMinMax(vlo, FExtent.a.X, FExtent.b.X);
    +        UpdateMinMax(vhi, FBasicExtent.a.X, FBasicExtent.b.X);
    +        UpdateMinMax(vlo, FBasicExtent.a.X, FBasicExtent.b.X);
           end
         else
           for i:=0 to Count - 1 do
    -        UpdateMinMax(Item[i]^.X, FExtent.a.X, FExtent.b.X);
    +        UpdateMinMax(Item[i]^.X, FBasicExtent.a.X, FBasicExtent.b.X);
       end;
     
       if YCount > 0 then begin
    @@ -803,16 +805,16 @@
         if HasYErrorBars then
           for i := 0 to Count - 1 do begin
             GetYErrorBarLimits(i, vhi, vlo);
    -        UpdateMinMax(vhi, FExtent.a.Y, FExtent.b.Y);
    -        UpdateMinMax(vlo, FExtent.a.Y, FExtent.b.Y);
    +        UpdateMinMax(vhi, FBasicExtent.a.Y, FBasicExtent.b.Y);
    +        UpdateMinMax(vlo, FBasicExtent.a.Y, FBasicExtent.b.Y);
           end
         else
           for i:=0 to Count - 1 do
    -        UpdateMinMax(Item[i]^.Y, FExtent.a.Y, FExtent.b.Y);
    +        UpdateMinMax(Item[i]^.Y, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end;
     
    -  FExtentIsValid := true;
    -  Result := FExtent;
    +  FBasicExtentIsValid := true;
    +  Result := FBasicExtent;
     end;
     
     procedure TCustomChartSource.BeforeDraw;
    @@ -829,25 +831,17 @@
       jyp, jyn: Integer;
     begin
       Result := Extent;
    -  if (YCount < 2) and (XCount < 2) then exit;
     
    -  // Skip the x and y values used for error bars when calculating the list extent.
    -  if UseXList and (XErrorBarData.Kind = ebkChartSource) then begin
    -    jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList is offset by 1
    -    jxn := XErrorBarData.IndexMinus - 1;
    -  end else begin
    -    jxp := -1;
    -    jxn := -1;
    -  end;
    -  if YErrorBarData.Kind = ebkChartSource then begin
    -    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList is offset by 1
    -    jyn := YErrorBarData.IndexMinus - 1;
    -  end else begin
    -    jyp := -1;
    -    jyn := -1;
    -  end;
    +  if UseXList and (XCount > 1) then begin
    +    // Skip the x values used for error bars when calculating the list extent.
    +    if XErrorBarData.Kind = ebkChartSource then begin
    +      jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList index is offset by 1
    +      jxn := XErrorBarData.IndexMinus - 1;
    +    end else begin
    +      jxp := -1;
    +      jxn := -1;
    +    end;
     
    -  if UseXList and (XCount > 1) then
         for i := 0 to Count - 1 do
           with Item[i]^ do begin
             for j := 0 to High(XList) do
    @@ -854,16 +848,27 @@
               if (j <> jxp) and (j <> jxn) then
                 UpdateMinMax(XList[j], Result.a.X, Result.b.X);
           end;
    -  for i := 0 to Count - 1 do
    -    with Item[i]^ do begin
    -      for j := 0 to High(YList) do
    -        if (j <> jyp) and (j <> jyn) then
    -          UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
    -    end
    +  end;
     
    +  if (YCount > 1) then begin
    +    // Skip the y values used for error bars when calculating the list extent.
    +    if YErrorBarData.Kind = ebkChartSource then begin
    +      jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    +      jyn := YErrorBarData.IndexMinus - 1;
    +    end else begin
    +      jyp := -1;
    +      jyn := -1;
    +    end;
    +
    +    for i := 0 to Count - 1 do
    +      with Item[i]^ do begin
    +        for j := 0 to High(YList) do
    +          if (j <> jyp) and (j <> jyn) then
    +            UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
    +      end;
    +  end;
     end;
     
    -
     class procedure TCustomChartSource.CheckFormat(const AFormat: String);
     begin
       Format(AFormat, [0.0, 0.0, '', 0.0, 0.0]);
    @@ -917,28 +922,31 @@
     var
       h: Double;
       i, j: Integer;
    -  jyp: Integer = -1;
    -  jyn: Integer = -1;
    +  jyp, jyn: Integer;
     begin
       Result := Extent;
    -  if YCount < 2 then exit;
     
    -  // Skip the y values used for error bars in calculating the cumulative sum.
    -  if YErrorBarData.Kind = ebkChartSource then begin
    -    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    -    jyn := YErrorBarData.IndexMinus - 1;
    +  if (YCount > 1) then begin
    +    // Skip the y values used for error bars when calculating the cumulative sum.
    +    if YErrorBarData.Kind = ebkChartSource then begin
    +      jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    +      jyn := YErrorBarData.IndexMinus - 1;
    +    end else begin
    +      jyp := -1;
    +      jyn := -1;
    +    end;
    +
    +    for i := 0 to Count - 1 do
    +      with Item[i]^ do begin
    +        h := NumberOr(Y);
    +        for j := 0 to High(YList) do
    +          if (j <> jyp) and (j <> jyn) then begin
    +            h += NumberOr(YList[j]);
    +            // If some of the Y values are negative, h may be non-monotonic.
    +            UpdateMinMax(h, Result.a.Y, Result.b.Y);
    +          end;
    +      end;
       end;
    -
    -  for i := 0 to Count - 1 do
    -    with Item[i]^ do begin
    -      h := NumberOr(Y);
    -      for j := 0 to High(YList) do
    -        if (j <> jyp) and (j <> jyn) then begin
    -          h += NumberOr(YList[j]);
    -          // If some of the Y values are negative, h may be non-monotonic.
    -          UpdateMinMax(h, Result.a.Y, Result.b.Y);
    -        end;
    -    end;
     end;
     
     { Calculates the extent including multiple y values (non-stacked) }
    @@ -1181,7 +1189,7 @@
     
     procedure TCustomChartSource.InvalidateCaches;
     begin
    -  FExtentIsValid := false;
    +  FBasicExtentIsValid := false;
       FValuesTotalIsValid := false;
     end;
     
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60823)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -30,7 +30,7 @@
         FYCountMin: Cardinal;
         procedure AddAt(
           APos: Integer; AX, AY: Double; const ALabel: String; AColor: TChartColor);
    -    procedure ClearCaches;
    +    procedure InitializeEmptyCaches;
         function NewItem: PChartDataItem;
         procedure SetDataPoints(AValue: TStrings);
         procedure SetSorted(AValue: Boolean);
    @@ -547,14 +547,14 @@
       for i := 0 to FData.Count - 1 do
         Dispose(Item[i]);
       FData.Clear;
    -  ClearCaches;
    +  InitializeEmptyCaches;
       Notify;
     end;
     
    -procedure TListChartSource.ClearCaches;
    +procedure TListChartSource.InitializeEmptyCaches;
     begin
    -  FExtent := EmptyExtent;
    -  FExtentIsValid := true;
    +  FBasicExtent := EmptyExtent;
    +  FBasicExtentIsValid := true;
       FValuesTotal := 0;
       FValuesTotalIsValid := true;
     end;
    @@ -590,7 +590,7 @@
       inherited Create(AOwner);
       FData := TFPList.Create;
       FDataPoints := TListChartSourceStrings.Create(Self);
    -  ClearCaches;
    +  InitializeEmptyCaches;
     end;
     
     constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
    @@ -607,9 +607,9 @@
     procedure TListChartSource.Delete(AIndex: Integer);
     begin
       with Item[AIndex]^ do begin
    -    FExtentIsValid := FExtentIsValid and
    -      (((FExtent.a.X < X) and (X < FExtent.b.X)) or (XCount = 0)) and
    -      (((FExtent.a.Y < Y) and (Y < FExtent.b.Y)) or (YCount = 0));
    +    FBasicExtentIsValid := FBasicExtentIsValid and
    +      (((FBasicExtent.a.X < X) and (X < FBasicExtent.b.X)) or (XCount = 0)) and
    +      (((FBasicExtent.a.Y < Y) and (Y < FBasicExtent.b.Y)) or (YCount = 0));
         if FValuesTotalIsValid then
           FValuesTotal -= NumberOr(Y);
       end;
    @@ -720,17 +720,17 @@
     
       procedure UpdateExtent;
       begin
    -    if (not FExtentIsValid) or (XCount = 0) then exit;
    +    if (not FBasicExtentIsValid) or (XCount = 0) then exit;
     
         if not IsNan(AValue) then begin
    -      if AValue < FExtent.a.X then
    -        FExtent.a.X := AValue
    -      else if AValue > FExtent.b.X then
    -        FExtent.b.X := AValue;
    +      if AValue < FBasicExtent.a.X then
    +        FBasicExtent.a.X := AValue
    +      else if AValue > FBasicExtent.b.X then
    +        FBasicExtent.b.X := AValue;
         end;
     
         if not IsNan(oldX) then
    -      FExtentIsValid := (oldX <> FExtent.a.X) and (oldX <> FExtent.b.X);
    +      FBasicExtentIsValid := (oldX <> FBasicExtent.a.X) and (oldX <> FBasicExtent.b.X);
       end;
     
     begin
    @@ -785,17 +785,17 @@
     
       procedure UpdateExtent;
       begin
    -    if (not FExtentIsValid) or (YCount = 0) then exit;
    +    if (not FBasicExtentIsValid) or (YCount = 0) then exit;
     
         if not IsNan(AValue) then begin
    -      if AValue < FExtent.a.Y then
    -        FExtent.a.Y := AValue
    -      else if AValue > FExtent.b.Y then
    -        FExtent.b.Y := AValue;
    +      if AValue < FBasicExtent.a.Y then
    +        FBasicExtent.a.Y := AValue
    +      else if AValue > FBasicExtent.b.Y then
    +        FBasicExtent.b.Y := AValue;
         end;
     
         if not IsNan(oldY) then
    -      FExtentIsValid := (oldY <> FExtent.a.Y) and (oldY <> FExtent.b.Y);
    +      FBasicExtentIsValid := (oldY <> FBasicExtent.a.Y) and (oldY <> FBasicExtent.b.Y);
       end;
     
     begin
    @@ -861,9 +861,9 @@
     
     procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
     begin
    -  if FExtentIsValid then begin
    -    if FXCount > 0 then UpdateMinMax(AX, FExtent.a.X, FExtent.b.X);
    -    if FYCount > 0 then UpdateMinMax(AY, FExtent.a.Y, FExtent.b.Y);
    +  if FBasicExtentIsValid then begin
    +    if FXCount > 0 then UpdateMinMax(AX, FBasicExtent.a.X, FBasicExtent.b.X);
    +    if FYCount > 0 then UpdateMinMax(AY, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end;
       if FValuesTotalIsValid then
         FValuesTotal += NumberOr(AY);
    
    patch_partial.diff (10,289 bytes)
  • patch_full.diff (12,701 bytes)
    Index: components/tachart/tacustomsource.pas
    ===================================================================
    --- components/tachart/tacustomsource.pas	(revision 60823)
    +++ components/tachart/tacustomsource.pas	(working copy)
    @@ -187,8 +187,14 @@
         procedure SortValuesInRange(
           var AValues: TChartValueTextArray; AStart, AEnd: Integer);
       strict protected
    -    FExtent: TDoubleRect;
    -    FExtentIsValid: Boolean;
    +    FBasicExtent: TDoubleRect;
    +    FBasicExtentIsValid: Boolean;
    +    FCumulativeExtent: TDoubleRect;
    +    FCumulativeExtentIsValid: Boolean;
    +    FXListExtent: TDoubleRect;
    +    FXListExtentIsValid: Boolean;
    +    FYListExtent: TDoubleRect;
    +    FYListExtentIsValid: Boolean;
         FValuesTotal: Double;
         FValuesTotalIsValid: Boolean;
         FXCount: Cardinal;
    @@ -308,6 +314,8 @@
       AItem.Y := 0;
       AItem.Color := clTAColor;
       AItem.Text := '';
    +  for i := 0 to High(AItem.XList) do
    +    AItem.XList[i] := 0;
       for i := 0 to High(AItem.YList) do
         AItem.YList[i] := 0;
     end;
    @@ -784,19 +792,19 @@
       i: Integer;
       vhi, vlo: Double;
     begin
    -  if FExtentIsValid then exit(FExtent);
    -  FExtent := EmptyExtent;
    +  if FBasicExtentIsValid then exit(FBasicExtent);
    +  FBasicExtent := EmptyExtent;
     
       if XCount > 0 then begin
         if HasXErrorBars then
           for i := 0 to Count - 1 do begin
             GetXErrorBarLimits(i, vhi, vlo);
    -        UpdateMinMax(vhi, FExtent.a.X, FExtent.b.X);
    -        UpdateMinMax(vlo, FExtent.a.X, FExtent.b.X);
    +        UpdateMinMax(vhi, FBasicExtent.a.X, FBasicExtent.b.X);
    +        UpdateMinMax(vlo, FBasicExtent.a.X, FBasicExtent.b.X);
           end
         else
           for i:=0 to Count - 1 do
    -        UpdateMinMax(Item[i]^.X, FExtent.a.X, FExtent.b.X);
    +        UpdateMinMax(Item[i]^.X, FBasicExtent.a.X, FBasicExtent.b.X);
       end;
     
       if YCount > 0 then begin
    @@ -803,16 +811,16 @@
         if HasYErrorBars then
           for i := 0 to Count - 1 do begin
             GetYErrorBarLimits(i, vhi, vlo);
    -        UpdateMinMax(vhi, FExtent.a.Y, FExtent.b.Y);
    -        UpdateMinMax(vlo, FExtent.a.Y, FExtent.b.Y);
    +        UpdateMinMax(vhi, FBasicExtent.a.Y, FBasicExtent.b.Y);
    +        UpdateMinMax(vlo, FBasicExtent.a.Y, FBasicExtent.b.Y);
           end
         else
           for i:=0 to Count - 1 do
    -        UpdateMinMax(Item[i]^.Y, FExtent.a.Y, FExtent.b.Y);
    +        UpdateMinMax(Item[i]^.Y, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end;
     
    -  FExtentIsValid := true;
    -  Result := FExtent;
    +  FBasicExtentIsValid := true;
    +  Result := FBasicExtent;
     end;
     
     procedure TCustomChartSource.BeforeDraw;
    @@ -829,41 +837,62 @@
       jyp, jyn: Integer;
     begin
       Result := Extent;
    -  if (YCount < 2) and (XCount < 2) then exit;
     
    -  // Skip the x and y values used for error bars when calculating the list extent.
    -  if UseXList and (XErrorBarData.Kind = ebkChartSource) then begin
    -    jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList is offset by 1
    -    jxn := XErrorBarData.IndexMinus - 1;
    -  end else begin
    -    jxp := -1;
    -    jxn := -1;
    +  if UseXList and (XCount > 1) then begin
    +    if not FXListExtentIsValid then begin
    +      FXListExtent := EmptyExtent;
    +
    +      // Skip the x values used for error bars when calculating the list extent.
    +      if XErrorBarData.Kind = ebkChartSource then begin
    +        jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList index is offset by 1
    +        jxn := XErrorBarData.IndexMinus - 1;
    +      end else begin
    +        jxp := -1;
    +        jxn := -1;
    +      end;
    +
    +      for i := 0 to Count - 1 do
    +        with Item[i]^ do begin
    +          for j := 0 to High(XList) do
    +            if (j <> jxp) and (j <> jxn) then
    +              UpdateMinMax(XList[j], FXListExtent.a.X, FXListExtent.b.X);
    +        end;
    +
    +      FXListExtentIsValid := true;
    +    end;
    +
    +    Result.a.X := Min(Result.a.X, FXListExtent.a.X);
    +    Result.b.X := Max(Result.b.X, FXListExtent.b.X);
       end;
    -  if YErrorBarData.Kind = ebkChartSource then begin
    -    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList is offset by 1
    -    jyn := YErrorBarData.IndexMinus - 1;
    -  end else begin
    -    jyp := -1;
    -    jyn := -1;
    -  end;
     
    -  if UseXList and (XCount > 1) then
    -    for i := 0 to Count - 1 do
    -      with Item[i]^ do begin
    -        for j := 0 to High(XList) do
    -          if (j <> jxp) and (j <> jxn) then
    -            UpdateMinMax(XList[j], Result.a.X, Result.b.X);
    +  if (YCount > 1) then begin
    +    if not FYListExtentIsValid then begin
    +      FYListExtent := EmptyExtent;
    +
    +      // Skip the y values used for error bars when calculating the list extent.
    +      if YErrorBarData.Kind = ebkChartSource then begin
    +        jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    +        jyn := YErrorBarData.IndexMinus - 1;
    +      end else begin
    +        jyp := -1;
    +        jyn := -1;
           end;
    -  for i := 0 to Count - 1 do
    -    with Item[i]^ do begin
    -      for j := 0 to High(YList) do
    -        if (j <> jyp) and (j <> jyn) then
    -          UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
    -    end
     
    +      for i := 0 to Count - 1 do
    +        with Item[i]^ do begin
    +          for j := 0 to High(YList) do
    +            if (j <> jyp) and (j <> jyn) then
    +              UpdateMinMax(YList[j], FYListExtent.a.Y, FYListExtent.b.Y);
    +        end;
    +
    +      FYListExtentIsValid := true;
    +    end;
    +
    +    Result.a.Y := Min(Result.a.Y, FYListExtent.a.Y);
    +    Result.b.Y := Max(Result.b.Y, FYListExtent.b.Y);
    +  end;
     end;
     
    -
     class procedure TCustomChartSource.CheckFormat(const AFormat: String);
     begin
       Format(AFormat, [0.0, 0.0, '', 0.0, 0.0]);
    @@ -917,28 +946,40 @@
     var
       h: Double;
       i, j: Integer;
    -  jyp: Integer = -1;
    -  jyn: Integer = -1;
    +  jyp, jyn: Integer;
     begin
       Result := Extent;
    -  if YCount < 2 then exit;
     
    -  // Skip the y values used for error bars in calculating the cumulative sum.
    -  if YErrorBarData.Kind = ebkChartSource then begin
    -    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    -    jyn := YErrorBarData.IndexMinus - 1;
    -  end;
    +  if (YCount > 1) then begin
    +    if not FCumulativeExtentIsValid then begin
    +      FCumulativeExtent := EmptyExtent;
     
    -  for i := 0 to Count - 1 do
    -    with Item[i]^ do begin
    -      h := NumberOr(Y);
    -      for j := 0 to High(YList) do
    -        if (j <> jyp) and (j <> jyn) then begin
    -          h += NumberOr(YList[j]);
    -          // If some of the Y values are negative, h may be non-monotonic.
    -          UpdateMinMax(h, Result.a.Y, Result.b.Y);
    +      // Skip the y values used for error bars when calculating the cumulative sum.
    +      if YErrorBarData.Kind = ebkChartSource then begin
    +        jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
    +        jyn := YErrorBarData.IndexMinus - 1;
    +      end else begin
    +        jyp := -1;
    +        jyn := -1;
    +      end;
    +
    +      for i := 0 to Count - 1 do
    +        with Item[i]^ do begin
    +          h := NumberOr(Y);
    +          for j := 0 to High(YList) do
    +            if (j <> jyp) and (j <> jyn) then begin
    +              h += NumberOr(YList[j]);
    +              // If some of the Y values are negative, h may be non-monotonic.
    +              UpdateMinMax(h, FCumulativeExtent.a.Y, FCumulativeExtent.b.Y);
    +            end;
             end;
    +
    +      FCumulativeExtentIsValid := true;
         end;
    +
    +    Result.a.Y := Min(Result.a.Y, FCumulativeExtent.a.Y);
    +    Result.b.Y := Max(Result.b.Y, FCumulativeExtent.b.Y);
    +  end;
     end;
     
     { Calculates the extent including multiple y values (non-stacked) }
    @@ -1181,7 +1222,10 @@
     
     procedure TCustomChartSource.InvalidateCaches;
     begin
    -  FExtentIsValid := false;
    +  FBasicExtentIsValid := false;
    +  FCumulativeExtentIsValid := false;
    +  FXListExtentIsValid := false;
    +  FYListExtentIsValid := false;
       FValuesTotalIsValid := false;
     end;
     
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60823)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -30,7 +30,7 @@
         FYCountMin: Cardinal;
         procedure AddAt(
           APos: Integer; AX, AY: Double; const ALabel: String; AColor: TChartColor);
    -    procedure ClearCaches;
    +    procedure InitializeEmptyCaches;
         function NewItem: PChartDataItem;
         procedure SetDataPoints(AValue: TStrings);
         procedure SetSorted(AValue: Boolean);
    @@ -547,14 +547,20 @@
       for i := 0 to FData.Count - 1 do
         Dispose(Item[i]);
       FData.Clear;
    -  ClearCaches;
    +  InitializeEmptyCaches;
       Notify;
     end;
     
    -procedure TListChartSource.ClearCaches;
    +procedure TListChartSource.InitializeEmptyCaches;
     begin
    -  FExtent := EmptyExtent;
    -  FExtentIsValid := true;
    +  FBasicExtent := EmptyExtent;
    +  FBasicExtentIsValid := true;
    +  FCumulativeExtent := EmptyExtent;
    +  FCumulativeExtentIsValid := true;
    +  FXListExtent := EmptyExtent;
    +  FXListExtentIsValid := true;
    +  FYListExtent := EmptyExtent;
    +  FYListExtentIsValid := true;
       FValuesTotal := 0;
       FValuesTotalIsValid := true;
     end;
    @@ -590,7 +596,7 @@
       inherited Create(AOwner);
       FData := TFPList.Create;
       FDataPoints := TListChartSourceStrings.Create(Self);
    -  ClearCaches;
    +  InitializeEmptyCaches;
     end;
     
     constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
    @@ -607,12 +613,15 @@
     procedure TListChartSource.Delete(AIndex: Integer);
     begin
       with Item[AIndex]^ do begin
    -    FExtentIsValid := FExtentIsValid and
    -      (((FExtent.a.X < X) and (X < FExtent.b.X)) or (XCount = 0)) and
    -      (((FExtent.a.Y < Y) and (Y < FExtent.b.Y)) or (YCount = 0));
    +    FBasicExtentIsValid := FBasicExtentIsValid and
    +      (((FBasicExtent.a.X < X) and (X < FBasicExtent.b.X)) or (XCount = 0)) and
    +      (((FBasicExtent.a.Y < Y) and (Y < FBasicExtent.b.Y)) or (YCount = 0));
         if FValuesTotalIsValid then
           FValuesTotal -= NumberOr(Y);
       end;
    +  FCumulativeExtentIsValid := false;
    +  FXListExtentIsValid := false;
    +  FYListExtentIsValid := false;
       Dispose(Item[AIndex]);
       FData.Delete(AIndex);
       Notify;
    @@ -711,7 +720,7 @@
       with Item[AIndex]^ do
         for i := 0 to Min(High(AXList), High(XList)) do
           XList[i] := AXList[i];
    -  // wp: Update x extent here ?
    +  FXListExtentIsValid := false;
     end;
     
     function TListChartSource.SetXValue(AIndex: Integer; AValue: Double): Integer;
    @@ -720,17 +729,17 @@
     
       procedure UpdateExtent;
       begin
    -    if (not FExtentIsValid) or (XCount = 0) then exit;
    +    if (not FBasicExtentIsValid) or (XCount = 0) then exit;
     
         if not IsNan(AValue) then begin
    -      if AValue < FExtent.a.X then
    -        FExtent.a.X := AValue
    -      else if AValue > FExtent.b.X then
    -        FExtent.b.X := AValue;
    +      if AValue < FBasicExtent.a.X then
    +        FBasicExtent.a.X := AValue
    +      else if AValue > FBasicExtent.b.X then
    +        FBasicExtent.b.X := AValue;
         end;
     
         if not IsNan(oldX) then
    -      FExtentIsValid := (oldX <> FExtent.a.X) and (oldX <> FExtent.b.X);
    +      FBasicExtentIsValid := (oldX <> FBasicExtent.a.X) and (oldX <> FBasicExtent.b.X);
       end;
     
     begin
    @@ -777,6 +786,8 @@
       with Item[AIndex]^ do
         for i := 0 to Min(High(AYList), High(YList)) do
           YList[i] := AYList[i];
    +  FCumulativeExtentIsValid := false;
    +  FYListExtentIsValid := false;
     end;
     
     procedure TListChartSource.SetYValue(AIndex: Integer; AValue: Double);
    @@ -785,17 +796,17 @@
     
       procedure UpdateExtent;
       begin
    -    if (not FExtentIsValid) or (YCount = 0) then exit;
    +    if (not FBasicExtentIsValid) or (YCount = 0) then exit;
     
         if not IsNan(AValue) then begin
    -      if AValue < FExtent.a.Y then
    -        FExtent.a.Y := AValue
    -      else if AValue > FExtent.b.Y then
    -        FExtent.b.Y := AValue;
    +      if AValue < FBasicExtent.a.Y then
    +        FBasicExtent.a.Y := AValue
    +      else if AValue > FBasicExtent.b.Y then
    +        FBasicExtent.b.Y := AValue;
         end;
     
         if not IsNan(oldY) then
    -      FExtentIsValid := (oldY <> FExtent.a.Y) and (oldY <> FExtent.b.Y);
    +      FBasicExtentIsValid := (oldY <> FBasicExtent.a.Y) and (oldY <> FBasicExtent.b.Y);
       end;
     
     begin
    @@ -861,12 +872,15 @@
     
     procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
     begin
    -  if FExtentIsValid then begin
    -    if FXCount > 0 then UpdateMinMax(AX, FExtent.a.X, FExtent.b.X);
    -    if FYCount > 0 then UpdateMinMax(AY, FExtent.a.Y, FExtent.b.Y);
    +  if FBasicExtentIsValid then begin
    +    if FXCount > 0 then UpdateMinMax(AX, FBasicExtent.a.X, FBasicExtent.b.X);
    +    if FYCount > 0 then UpdateMinMax(AY, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end;
       if FValuesTotalIsValid then
         FValuesTotal += NumberOr(AY);
    +  FCumulativeExtentIsValid := false;
    +  FXListExtentIsValid := false;
    +  FYListExtentIsValid := false;
       Notify;
     end;
     
    
    patch_full.diff (12,701 bytes)
  • Test.zip (2,375 bytes)

Relationships

related to 0035268 closedwp TAChart: TCubicSplineSeries may omit some points when drawing 

Activities

Marcin Wiazowski

2019-04-03 19:22

reporter  

MultiExtentSpeedTest.zip (2,616 bytes)

Marcin Wiazowski

2019-04-03 19:23

reporter  

patch_partial.diff (10,289 bytes)
Index: components/tachart/tacustomsource.pas
===================================================================
--- components/tachart/tacustomsource.pas	(revision 60823)
+++ components/tachart/tacustomsource.pas	(working copy)
@@ -187,8 +187,8 @@
     procedure SortValuesInRange(
       var AValues: TChartValueTextArray; AStart, AEnd: Integer);
   strict protected
-    FExtent: TDoubleRect;
-    FExtentIsValid: Boolean;
+    FBasicExtent: TDoubleRect;
+    FBasicExtentIsValid: Boolean;
     FValuesTotal: Double;
     FValuesTotalIsValid: Boolean;
     FXCount: Cardinal;
@@ -308,6 +308,8 @@
   AItem.Y := 0;
   AItem.Color := clTAColor;
   AItem.Text := '';
+  for i := 0 to High(AItem.XList) do
+    AItem.XList[i] := 0;
   for i := 0 to High(AItem.YList) do
     AItem.YList[i] := 0;
 end;
@@ -784,19 +786,19 @@
   i: Integer;
   vhi, vlo: Double;
 begin
-  if FExtentIsValid then exit(FExtent);
-  FExtent := EmptyExtent;
+  if FBasicExtentIsValid then exit(FBasicExtent);
+  FBasicExtent := EmptyExtent;
 
   if XCount > 0 then begin
     if HasXErrorBars then
       for i := 0 to Count - 1 do begin
         GetXErrorBarLimits(i, vhi, vlo);
-        UpdateMinMax(vhi, FExtent.a.X, FExtent.b.X);
-        UpdateMinMax(vlo, FExtent.a.X, FExtent.b.X);
+        UpdateMinMax(vhi, FBasicExtent.a.X, FBasicExtent.b.X);
+        UpdateMinMax(vlo, FBasicExtent.a.X, FBasicExtent.b.X);
       end
     else
       for i:=0 to Count - 1 do
-        UpdateMinMax(Item[i]^.X, FExtent.a.X, FExtent.b.X);
+        UpdateMinMax(Item[i]^.X, FBasicExtent.a.X, FBasicExtent.b.X);
   end;
 
   if YCount > 0 then begin
@@ -803,16 +805,16 @@
     if HasYErrorBars then
       for i := 0 to Count - 1 do begin
         GetYErrorBarLimits(i, vhi, vlo);
-        UpdateMinMax(vhi, FExtent.a.Y, FExtent.b.Y);
-        UpdateMinMax(vlo, FExtent.a.Y, FExtent.b.Y);
+        UpdateMinMax(vhi, FBasicExtent.a.Y, FBasicExtent.b.Y);
+        UpdateMinMax(vlo, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end
     else
       for i:=0 to Count - 1 do
-        UpdateMinMax(Item[i]^.Y, FExtent.a.Y, FExtent.b.Y);
+        UpdateMinMax(Item[i]^.Y, FBasicExtent.a.Y, FBasicExtent.b.Y);
   end;
 
-  FExtentIsValid := true;
-  Result := FExtent;
+  FBasicExtentIsValid := true;
+  Result := FBasicExtent;
 end;
 
 procedure TCustomChartSource.BeforeDraw;
@@ -829,25 +831,17 @@
   jyp, jyn: Integer;
 begin
   Result := Extent;
-  if (YCount < 2) and (XCount < 2) then exit;
 
-  // Skip the x and y values used for error bars when calculating the list extent.
-  if UseXList and (XErrorBarData.Kind = ebkChartSource) then begin
-    jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList is offset by 1
-    jxn := XErrorBarData.IndexMinus - 1;
-  end else begin
-    jxp := -1;
-    jxn := -1;
-  end;
-  if YErrorBarData.Kind = ebkChartSource then begin
-    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList is offset by 1
-    jyn := YErrorBarData.IndexMinus - 1;
-  end else begin
-    jyp := -1;
-    jyn := -1;
-  end;
+  if UseXList and (XCount > 1) then begin
+    // Skip the x values used for error bars when calculating the list extent.
+    if XErrorBarData.Kind = ebkChartSource then begin
+      jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList index is offset by 1
+      jxn := XErrorBarData.IndexMinus - 1;
+    end else begin
+      jxp := -1;
+      jxn := -1;
+    end;
 
-  if UseXList and (XCount > 1) then
     for i := 0 to Count - 1 do
       with Item[i]^ do begin
         for j := 0 to High(XList) do
@@ -854,16 +848,27 @@
           if (j <> jxp) and (j <> jxn) then
             UpdateMinMax(XList[j], Result.a.X, Result.b.X);
       end;
-  for i := 0 to Count - 1 do
-    with Item[i]^ do begin
-      for j := 0 to High(YList) do
-        if (j <> jyp) and (j <> jyn) then
-          UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
-    end
+  end;
 
+  if (YCount > 1) then begin
+    // Skip the y values used for error bars when calculating the list extent.
+    if YErrorBarData.Kind = ebkChartSource then begin
+      jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
+      jyn := YErrorBarData.IndexMinus - 1;
+    end else begin
+      jyp := -1;
+      jyn := -1;
+    end;
+
+    for i := 0 to Count - 1 do
+      with Item[i]^ do begin
+        for j := 0 to High(YList) do
+          if (j <> jyp) and (j <> jyn) then
+            UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
+      end;
+  end;
 end;
 
-
 class procedure TCustomChartSource.CheckFormat(const AFormat: String);
 begin
   Format(AFormat, [0.0, 0.0, '', 0.0, 0.0]);
@@ -917,28 +922,31 @@
 var
   h: Double;
   i, j: Integer;
-  jyp: Integer = -1;
-  jyn: Integer = -1;
+  jyp, jyn: Integer;
 begin
   Result := Extent;
-  if YCount < 2 then exit;
 
-  // Skip the y values used for error bars in calculating the cumulative sum.
-  if YErrorBarData.Kind = ebkChartSource then begin
-    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
-    jyn := YErrorBarData.IndexMinus - 1;
+  if (YCount > 1) then begin
+    // Skip the y values used for error bars when calculating the cumulative sum.
+    if YErrorBarData.Kind = ebkChartSource then begin
+      jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
+      jyn := YErrorBarData.IndexMinus - 1;
+    end else begin
+      jyp := -1;
+      jyn := -1;
+    end;
+
+    for i := 0 to Count - 1 do
+      with Item[i]^ do begin
+        h := NumberOr(Y);
+        for j := 0 to High(YList) do
+          if (j <> jyp) and (j <> jyn) then begin
+            h += NumberOr(YList[j]);
+            // If some of the Y values are negative, h may be non-monotonic.
+            UpdateMinMax(h, Result.a.Y, Result.b.Y);
+          end;
+      end;
   end;
-
-  for i := 0 to Count - 1 do
-    with Item[i]^ do begin
-      h := NumberOr(Y);
-      for j := 0 to High(YList) do
-        if (j <> jyp) and (j <> jyn) then begin
-          h += NumberOr(YList[j]);
-          // If some of the Y values are negative, h may be non-monotonic.
-          UpdateMinMax(h, Result.a.Y, Result.b.Y);
-        end;
-    end;
 end;
 
 { Calculates the extent including multiple y values (non-stacked) }
@@ -1181,7 +1189,7 @@
 
 procedure TCustomChartSource.InvalidateCaches;
 begin
-  FExtentIsValid := false;
+  FBasicExtentIsValid := false;
   FValuesTotalIsValid := false;
 end;
 
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60823)
+++ components/tachart/tasources.pas	(working copy)
@@ -30,7 +30,7 @@
     FYCountMin: Cardinal;
     procedure AddAt(
       APos: Integer; AX, AY: Double; const ALabel: String; AColor: TChartColor);
-    procedure ClearCaches;
+    procedure InitializeEmptyCaches;
     function NewItem: PChartDataItem;
     procedure SetDataPoints(AValue: TStrings);
     procedure SetSorted(AValue: Boolean);
@@ -547,14 +547,14 @@
   for i := 0 to FData.Count - 1 do
     Dispose(Item[i]);
   FData.Clear;
-  ClearCaches;
+  InitializeEmptyCaches;
   Notify;
 end;
 
-procedure TListChartSource.ClearCaches;
+procedure TListChartSource.InitializeEmptyCaches;
 begin
-  FExtent := EmptyExtent;
-  FExtentIsValid := true;
+  FBasicExtent := EmptyExtent;
+  FBasicExtentIsValid := true;
   FValuesTotal := 0;
   FValuesTotalIsValid := true;
 end;
@@ -590,7 +590,7 @@
   inherited Create(AOwner);
   FData := TFPList.Create;
   FDataPoints := TListChartSourceStrings.Create(Self);
-  ClearCaches;
+  InitializeEmptyCaches;
 end;
 
 constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
@@ -607,9 +607,9 @@
 procedure TListChartSource.Delete(AIndex: Integer);
 begin
   with Item[AIndex]^ do begin
-    FExtentIsValid := FExtentIsValid and
-      (((FExtent.a.X < X) and (X < FExtent.b.X)) or (XCount = 0)) and
-      (((FExtent.a.Y < Y) and (Y < FExtent.b.Y)) or (YCount = 0));
+    FBasicExtentIsValid := FBasicExtentIsValid and
+      (((FBasicExtent.a.X < X) and (X < FBasicExtent.b.X)) or (XCount = 0)) and
+      (((FBasicExtent.a.Y < Y) and (Y < FBasicExtent.b.Y)) or (YCount = 0));
     if FValuesTotalIsValid then
       FValuesTotal -= NumberOr(Y);
   end;
@@ -720,17 +720,17 @@
 
   procedure UpdateExtent;
   begin
-    if (not FExtentIsValid) or (XCount = 0) then exit;
+    if (not FBasicExtentIsValid) or (XCount = 0) then exit;
 
     if not IsNan(AValue) then begin
-      if AValue < FExtent.a.X then
-        FExtent.a.X := AValue
-      else if AValue > FExtent.b.X then
-        FExtent.b.X := AValue;
+      if AValue < FBasicExtent.a.X then
+        FBasicExtent.a.X := AValue
+      else if AValue > FBasicExtent.b.X then
+        FBasicExtent.b.X := AValue;
     end;
 
     if not IsNan(oldX) then
-      FExtentIsValid := (oldX <> FExtent.a.X) and (oldX <> FExtent.b.X);
+      FBasicExtentIsValid := (oldX <> FBasicExtent.a.X) and (oldX <> FBasicExtent.b.X);
   end;
 
 begin
@@ -785,17 +785,17 @@
 
   procedure UpdateExtent;
   begin
-    if (not FExtentIsValid) or (YCount = 0) then exit;
+    if (not FBasicExtentIsValid) or (YCount = 0) then exit;
 
     if not IsNan(AValue) then begin
-      if AValue < FExtent.a.Y then
-        FExtent.a.Y := AValue
-      else if AValue > FExtent.b.Y then
-        FExtent.b.Y := AValue;
+      if AValue < FBasicExtent.a.Y then
+        FBasicExtent.a.Y := AValue
+      else if AValue > FBasicExtent.b.Y then
+        FBasicExtent.b.Y := AValue;
     end;
 
     if not IsNan(oldY) then
-      FExtentIsValid := (oldY <> FExtent.a.Y) and (oldY <> FExtent.b.Y);
+      FBasicExtentIsValid := (oldY <> FBasicExtent.a.Y) and (oldY <> FBasicExtent.b.Y);
   end;
 
 begin
@@ -861,9 +861,9 @@
 
 procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
 begin
-  if FExtentIsValid then begin
-    if FXCount > 0 then UpdateMinMax(AX, FExtent.a.X, FExtent.b.X);
-    if FYCount > 0 then UpdateMinMax(AY, FExtent.a.Y, FExtent.b.Y);
+  if FBasicExtentIsValid then begin
+    if FXCount > 0 then UpdateMinMax(AX, FBasicExtent.a.X, FBasicExtent.b.X);
+    if FYCount > 0 then UpdateMinMax(AY, FBasicExtent.a.Y, FBasicExtent.b.Y);
   end;
   if FValuesTotalIsValid then
     FValuesTotal += NumberOr(AY);
patch_partial.diff (10,289 bytes)

Marcin Wiazowski

2019-04-03 19:23

reporter  

patch_full.diff (12,701 bytes)
Index: components/tachart/tacustomsource.pas
===================================================================
--- components/tachart/tacustomsource.pas	(revision 60823)
+++ components/tachart/tacustomsource.pas	(working copy)
@@ -187,8 +187,14 @@
     procedure SortValuesInRange(
       var AValues: TChartValueTextArray; AStart, AEnd: Integer);
   strict protected
-    FExtent: TDoubleRect;
-    FExtentIsValid: Boolean;
+    FBasicExtent: TDoubleRect;
+    FBasicExtentIsValid: Boolean;
+    FCumulativeExtent: TDoubleRect;
+    FCumulativeExtentIsValid: Boolean;
+    FXListExtent: TDoubleRect;
+    FXListExtentIsValid: Boolean;
+    FYListExtent: TDoubleRect;
+    FYListExtentIsValid: Boolean;
     FValuesTotal: Double;
     FValuesTotalIsValid: Boolean;
     FXCount: Cardinal;
@@ -308,6 +314,8 @@
   AItem.Y := 0;
   AItem.Color := clTAColor;
   AItem.Text := '';
+  for i := 0 to High(AItem.XList) do
+    AItem.XList[i] := 0;
   for i := 0 to High(AItem.YList) do
     AItem.YList[i] := 0;
 end;
@@ -784,19 +792,19 @@
   i: Integer;
   vhi, vlo: Double;
 begin
-  if FExtentIsValid then exit(FExtent);
-  FExtent := EmptyExtent;
+  if FBasicExtentIsValid then exit(FBasicExtent);
+  FBasicExtent := EmptyExtent;
 
   if XCount > 0 then begin
     if HasXErrorBars then
       for i := 0 to Count - 1 do begin
         GetXErrorBarLimits(i, vhi, vlo);
-        UpdateMinMax(vhi, FExtent.a.X, FExtent.b.X);
-        UpdateMinMax(vlo, FExtent.a.X, FExtent.b.X);
+        UpdateMinMax(vhi, FBasicExtent.a.X, FBasicExtent.b.X);
+        UpdateMinMax(vlo, FBasicExtent.a.X, FBasicExtent.b.X);
       end
     else
       for i:=0 to Count - 1 do
-        UpdateMinMax(Item[i]^.X, FExtent.a.X, FExtent.b.X);
+        UpdateMinMax(Item[i]^.X, FBasicExtent.a.X, FBasicExtent.b.X);
   end;
 
   if YCount > 0 then begin
@@ -803,16 +811,16 @@
     if HasYErrorBars then
       for i := 0 to Count - 1 do begin
         GetYErrorBarLimits(i, vhi, vlo);
-        UpdateMinMax(vhi, FExtent.a.Y, FExtent.b.Y);
-        UpdateMinMax(vlo, FExtent.a.Y, FExtent.b.Y);
+        UpdateMinMax(vhi, FBasicExtent.a.Y, FBasicExtent.b.Y);
+        UpdateMinMax(vlo, FBasicExtent.a.Y, FBasicExtent.b.Y);
       end
     else
       for i:=0 to Count - 1 do
-        UpdateMinMax(Item[i]^.Y, FExtent.a.Y, FExtent.b.Y);
+        UpdateMinMax(Item[i]^.Y, FBasicExtent.a.Y, FBasicExtent.b.Y);
   end;
 
-  FExtentIsValid := true;
-  Result := FExtent;
+  FBasicExtentIsValid := true;
+  Result := FBasicExtent;
 end;
 
 procedure TCustomChartSource.BeforeDraw;
@@ -829,41 +837,62 @@
   jyp, jyn: Integer;
 begin
   Result := Extent;
-  if (YCount < 2) and (XCount < 2) then exit;
 
-  // Skip the x and y values used for error bars when calculating the list extent.
-  if UseXList and (XErrorBarData.Kind = ebkChartSource) then begin
-    jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList is offset by 1
-    jxn := XErrorBarData.IndexMinus - 1;
-  end else begin
-    jxp := -1;
-    jxn := -1;
+  if UseXList and (XCount > 1) then begin
+    if not FXListExtentIsValid then begin
+      FXListExtent := EmptyExtent;
+
+      // Skip the x values used for error bars when calculating the list extent.
+      if XErrorBarData.Kind = ebkChartSource then begin
+        jxp := XErrorBarData.IndexPlus - 1;  // -1 because XList index is offset by 1
+        jxn := XErrorBarData.IndexMinus - 1;
+      end else begin
+        jxp := -1;
+        jxn := -1;
+      end;
+
+      for i := 0 to Count - 1 do
+        with Item[i]^ do begin
+          for j := 0 to High(XList) do
+            if (j <> jxp) and (j <> jxn) then
+              UpdateMinMax(XList[j], FXListExtent.a.X, FXListExtent.b.X);
+        end;
+
+      FXListExtentIsValid := true;
+    end;
+
+    Result.a.X := Min(Result.a.X, FXListExtent.a.X);
+    Result.b.X := Max(Result.b.X, FXListExtent.b.X);
   end;
-  if YErrorBarData.Kind = ebkChartSource then begin
-    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList is offset by 1
-    jyn := YErrorBarData.IndexMinus - 1;
-  end else begin
-    jyp := -1;
-    jyn := -1;
-  end;
 
-  if UseXList and (XCount > 1) then
-    for i := 0 to Count - 1 do
-      with Item[i]^ do begin
-        for j := 0 to High(XList) do
-          if (j <> jxp) and (j <> jxn) then
-            UpdateMinMax(XList[j], Result.a.X, Result.b.X);
+  if (YCount > 1) then begin
+    if not FYListExtentIsValid then begin
+      FYListExtent := EmptyExtent;
+
+      // Skip the y values used for error bars when calculating the list extent.
+      if YErrorBarData.Kind = ebkChartSource then begin
+        jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
+        jyn := YErrorBarData.IndexMinus - 1;
+      end else begin
+        jyp := -1;
+        jyn := -1;
       end;
-  for i := 0 to Count - 1 do
-    with Item[i]^ do begin
-      for j := 0 to High(YList) do
-        if (j <> jyp) and (j <> jyn) then
-          UpdateMinMax(YList[j], Result.a.Y, Result.b.Y);
-    end
 
+      for i := 0 to Count - 1 do
+        with Item[i]^ do begin
+          for j := 0 to High(YList) do
+            if (j <> jyp) and (j <> jyn) then
+              UpdateMinMax(YList[j], FYListExtent.a.Y, FYListExtent.b.Y);
+        end;
+
+      FYListExtentIsValid := true;
+    end;
+
+    Result.a.Y := Min(Result.a.Y, FYListExtent.a.Y);
+    Result.b.Y := Max(Result.b.Y, FYListExtent.b.Y);
+  end;
 end;
 
-
 class procedure TCustomChartSource.CheckFormat(const AFormat: String);
 begin
   Format(AFormat, [0.0, 0.0, '', 0.0, 0.0]);
@@ -917,28 +946,40 @@
 var
   h: Double;
   i, j: Integer;
-  jyp: Integer = -1;
-  jyn: Integer = -1;
+  jyp, jyn: Integer;
 begin
   Result := Extent;
-  if YCount < 2 then exit;
 
-  // Skip the y values used for error bars in calculating the cumulative sum.
-  if YErrorBarData.Kind = ebkChartSource then begin
-    jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
-    jyn := YErrorBarData.IndexMinus - 1;
-  end;
+  if (YCount > 1) then begin
+    if not FCumulativeExtentIsValid then begin
+      FCumulativeExtent := EmptyExtent;
 
-  for i := 0 to Count - 1 do
-    with Item[i]^ do begin
-      h := NumberOr(Y);
-      for j := 0 to High(YList) do
-        if (j <> jyp) and (j <> jyn) then begin
-          h += NumberOr(YList[j]);
-          // If some of the Y values are negative, h may be non-monotonic.
-          UpdateMinMax(h, Result.a.Y, Result.b.Y);
+      // Skip the y values used for error bars when calculating the cumulative sum.
+      if YErrorBarData.Kind = ebkChartSource then begin
+        jyp := YErrorBarData.IndexPlus - 1;  // -1 because YList index is offset by 1
+        jyn := YErrorBarData.IndexMinus - 1;
+      end else begin
+        jyp := -1;
+        jyn := -1;
+      end;
+
+      for i := 0 to Count - 1 do
+        with Item[i]^ do begin
+          h := NumberOr(Y);
+          for j := 0 to High(YList) do
+            if (j <> jyp) and (j <> jyn) then begin
+              h += NumberOr(YList[j]);
+              // If some of the Y values are negative, h may be non-monotonic.
+              UpdateMinMax(h, FCumulativeExtent.a.Y, FCumulativeExtent.b.Y);
+            end;
         end;
+
+      FCumulativeExtentIsValid := true;
     end;
+
+    Result.a.Y := Min(Result.a.Y, FCumulativeExtent.a.Y);
+    Result.b.Y := Max(Result.b.Y, FCumulativeExtent.b.Y);
+  end;
 end;
 
 { Calculates the extent including multiple y values (non-stacked) }
@@ -1181,7 +1222,10 @@
 
 procedure TCustomChartSource.InvalidateCaches;
 begin
-  FExtentIsValid := false;
+  FBasicExtentIsValid := false;
+  FCumulativeExtentIsValid := false;
+  FXListExtentIsValid := false;
+  FYListExtentIsValid := false;
   FValuesTotalIsValid := false;
 end;
 
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60823)
+++ components/tachart/tasources.pas	(working copy)
@@ -30,7 +30,7 @@
     FYCountMin: Cardinal;
     procedure AddAt(
       APos: Integer; AX, AY: Double; const ALabel: String; AColor: TChartColor);
-    procedure ClearCaches;
+    procedure InitializeEmptyCaches;
     function NewItem: PChartDataItem;
     procedure SetDataPoints(AValue: TStrings);
     procedure SetSorted(AValue: Boolean);
@@ -547,14 +547,20 @@
   for i := 0 to FData.Count - 1 do
     Dispose(Item[i]);
   FData.Clear;
-  ClearCaches;
+  InitializeEmptyCaches;
   Notify;
 end;
 
-procedure TListChartSource.ClearCaches;
+procedure TListChartSource.InitializeEmptyCaches;
 begin
-  FExtent := EmptyExtent;
-  FExtentIsValid := true;
+  FBasicExtent := EmptyExtent;
+  FBasicExtentIsValid := true;
+  FCumulativeExtent := EmptyExtent;
+  FCumulativeExtentIsValid := true;
+  FXListExtent := EmptyExtent;
+  FXListExtentIsValid := true;
+  FYListExtent := EmptyExtent;
+  FYListExtentIsValid := true;
   FValuesTotal := 0;
   FValuesTotalIsValid := true;
 end;
@@ -590,7 +596,7 @@
   inherited Create(AOwner);
   FData := TFPList.Create;
   FDataPoints := TListChartSourceStrings.Create(Self);
-  ClearCaches;
+  InitializeEmptyCaches;
 end;
 
 constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
@@ -607,12 +613,15 @@
 procedure TListChartSource.Delete(AIndex: Integer);
 begin
   with Item[AIndex]^ do begin
-    FExtentIsValid := FExtentIsValid and
-      (((FExtent.a.X < X) and (X < FExtent.b.X)) or (XCount = 0)) and
-      (((FExtent.a.Y < Y) and (Y < FExtent.b.Y)) or (YCount = 0));
+    FBasicExtentIsValid := FBasicExtentIsValid and
+      (((FBasicExtent.a.X < X) and (X < FBasicExtent.b.X)) or (XCount = 0)) and
+      (((FBasicExtent.a.Y < Y) and (Y < FBasicExtent.b.Y)) or (YCount = 0));
     if FValuesTotalIsValid then
       FValuesTotal -= NumberOr(Y);
   end;
+  FCumulativeExtentIsValid := false;
+  FXListExtentIsValid := false;
+  FYListExtentIsValid := false;
   Dispose(Item[AIndex]);
   FData.Delete(AIndex);
   Notify;
@@ -711,7 +720,7 @@
   with Item[AIndex]^ do
     for i := 0 to Min(High(AXList), High(XList)) do
       XList[i] := AXList[i];
-  // wp: Update x extent here ?
+  FXListExtentIsValid := false;
 end;
 
 function TListChartSource.SetXValue(AIndex: Integer; AValue: Double): Integer;
@@ -720,17 +729,17 @@
 
   procedure UpdateExtent;
   begin
-    if (not FExtentIsValid) or (XCount = 0) then exit;
+    if (not FBasicExtentIsValid) or (XCount = 0) then exit;
 
     if not IsNan(AValue) then begin
-      if AValue < FExtent.a.X then
-        FExtent.a.X := AValue
-      else if AValue > FExtent.b.X then
-        FExtent.b.X := AValue;
+      if AValue < FBasicExtent.a.X then
+        FBasicExtent.a.X := AValue
+      else if AValue > FBasicExtent.b.X then
+        FBasicExtent.b.X := AValue;
     end;
 
     if not IsNan(oldX) then
-      FExtentIsValid := (oldX <> FExtent.a.X) and (oldX <> FExtent.b.X);
+      FBasicExtentIsValid := (oldX <> FBasicExtent.a.X) and (oldX <> FBasicExtent.b.X);
   end;
 
 begin
@@ -777,6 +786,8 @@
   with Item[AIndex]^ do
     for i := 0 to Min(High(AYList), High(YList)) do
       YList[i] := AYList[i];
+  FCumulativeExtentIsValid := false;
+  FYListExtentIsValid := false;
 end;
 
 procedure TListChartSource.SetYValue(AIndex: Integer; AValue: Double);
@@ -785,17 +796,17 @@
 
   procedure UpdateExtent;
   begin
-    if (not FExtentIsValid) or (YCount = 0) then exit;
+    if (not FBasicExtentIsValid) or (YCount = 0) then exit;
 
     if not IsNan(AValue) then begin
-      if AValue < FExtent.a.Y then
-        FExtent.a.Y := AValue
-      else if AValue > FExtent.b.Y then
-        FExtent.b.Y := AValue;
+      if AValue < FBasicExtent.a.Y then
+        FBasicExtent.a.Y := AValue
+      else if AValue > FBasicExtent.b.Y then
+        FBasicExtent.b.Y := AValue;
     end;
 
     if not IsNan(oldY) then
-      FExtentIsValid := (oldY <> FExtent.a.Y) and (oldY <> FExtent.b.Y);
+      FBasicExtentIsValid := (oldY <> FBasicExtent.a.Y) and (oldY <> FBasicExtent.b.Y);
   end;
 
 begin
@@ -861,12 +872,15 @@
 
 procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
 begin
-  if FExtentIsValid then begin
-    if FXCount > 0 then UpdateMinMax(AX, FExtent.a.X, FExtent.b.X);
-    if FYCount > 0 then UpdateMinMax(AY, FExtent.a.Y, FExtent.b.Y);
+  if FBasicExtentIsValid then begin
+    if FXCount > 0 then UpdateMinMax(AX, FBasicExtent.a.X, FBasicExtent.b.X);
+    if FYCount > 0 then UpdateMinMax(AY, FBasicExtent.a.Y, FBasicExtent.b.Y);
   end;
   if FValuesTotalIsValid then
     FValuesTotal += NumberOr(AY);
+  FCumulativeExtentIsValid := false;
+  FXListExtentIsValid := false;
+  FYListExtentIsValid := false;
   Notify;
 end;
 
patch_full.diff (12,701 bytes)

wp

2019-04-05 14:14

developer   ~0115242

Applied with minor modifications. Thank you.

Marcin Wiazowski

2019-04-05 20:55

reporter  

Test.zip (2,375 bytes)

Marcin Wiazowski

2019-04-05 20:55

reporter   ~0115252

I attached an additional test application.


Results when compiled with r60839 (i.e. without extent caching added):

  Test1 - extent is:
    X = [1 .. 2]
    Y = [1 .. 5002]

  Test2 - extent is:
    X = [1 .. 2]
    Y = [1 .. 2]


Results currently (r60846):

  Test1 - extent is:
    X = [1 .. 2]
    Y = [1 .. 1002]

  Test2 - extent is:
    X = [1 .. 2]
    Y = [1 .. 1002]


So I'm afraid that we need to clear FCumulativeExtentIsValid, FXListExtentIsValid and FYListExtentIsValid in TCustomChartSource.InvalidateCaches(), as it was in the attached patch...

wp

2019-04-06 00:29

developer   ~0115256

Sorry for forgetting this piece. You could help me in reviewing your patches if you would pack less topics into the same patch so that I could ideally apply the patches as a whole. Currently I have to split them manually, and this is where such things happen.

Marcin Wiazowski

2019-04-06 00:40

reporter   ~0115257

Fix confirmed.

I'll do my best to prepare patches in a most convenient way, but I'm not sure if I understand you correctly - could you please tell me what is this splitting needed for?

wp

2019-04-06 01:37

developer   ~0115258

I want to keep things separate. Here, for example, you renamed FExtent and FExtentIsValid (which I wanted to do anyway), and there was the missing initialization in SetDataItemDefaults - they do not have so much in common with the caching of the extents. Therefore, I picked out these two topics and applied them first.

Ideally a patch should handle only a single item. If later a regression due to the new caching would be found it would be much easier to work with revisions containing "pure" fixes not "contaminated" by other topics.

I know creating "atomic" patches is more difficult for the reporter because he cannot submit all his ideas in a single patch and must wait until the individual parts are applied. I don't know a good solution, and I must confess that in may own work I often get carried away and have too many items in a single commit.

But maybe you could try to focus on the main topic and submit at first a patch for it only; less critical things like renaming could be combined in a follow-up patch of the same report or another report (if not related).

Marcin Wiazowski

2019-04-06 03:23

reporter   ~0115259

Thank you for your explanations. I'll try to follow your advice.

Issue History

Date Modified Username Field Change
2019-04-03 19:22 Marcin Wiazowski New Issue
2019-04-03 19:22 Marcin Wiazowski File Added: MultiExtentSpeedTest.zip
2019-04-03 19:23 Marcin Wiazowski File Added: patch_partial.diff
2019-04-03 19:23 Marcin Wiazowski File Added: patch_full.diff
2019-04-05 11:36 wp Assigned To => wp
2019-04-05 11:36 wp Status new => assigned
2019-04-05 11:38 wp Relationship added related to 0035268
2019-04-05 14:14 wp Fixed in Revision => 60852, 60843, 60846
2019-04-05 14:14 wp LazTarget => -
2019-04-05 14:14 wp Note Added: 0115242
2019-04-05 14:14 wp Status assigned => resolved
2019-04-05 14:14 wp Resolution open => fixed
2019-04-05 14:14 wp Target Version => 2.2
2019-04-05 20:55 Marcin Wiazowski File Added: Test.zip
2019-04-05 20:55 Marcin Wiazowski Note Added: 0115252
2019-04-05 20:55 Marcin Wiazowski Status resolved => assigned
2019-04-05 20:55 Marcin Wiazowski Resolution fixed => reopened
2019-04-06 00:29 wp Fixed in Revision 60852, 60843, 60846 => 60852, 60843, 60846, 60848
2019-04-06 00:29 wp Note Added: 0115256
2019-04-06 00:29 wp Status assigned => resolved
2019-04-06 00:29 wp Resolution reopened => fixed
2019-04-06 00:40 Marcin Wiazowski Note Added: 0115257
2019-04-06 00:40 Marcin Wiazowski Status resolved => assigned
2019-04-06 00:40 Marcin Wiazowski Resolution fixed => reopened
2019-04-06 01:37 wp Note Added: 0115258
2019-04-06 01:37 wp Status assigned => resolved
2019-04-06 01:37 wp Resolution reopened => fixed
2019-04-06 03:23 Marcin Wiazowski Note Added: 0115259
2019-04-06 03:23 Marcin Wiazowski Status resolved => closed