View Issue Details

IDProjectCategoryView StatusLast Update
0035089LazarusTAChartpublic2019-06-12 14:24
ReporterMarcin WiazowskiAssigned Towp 
PrioritynormalSeverityminorReproducibilityrandom
Status closedResolutionfixed 
Product Version2.0Product Build60424 
Target Version2.2Fixed in Version 
Summary0035089: TAChart: crash in IDE
DescriptionSteps to reproduce:
- load the attached Reproduce application in IDE,
- in Object Inspector, select Chart1FieldSeries, go to the Source property, and try to change its value from Chart1FieldSeries.Builtin to ListChartSource.

IDE (2.0.0) randomly crashes or not.
TagsNo tags attached.
Fixed in Revision60425, 60427, 60464, 60471, 60491
LazTarget2.2
WidgetsetWin32/Win64
Attached Files
  • Reproduce.zip (3,082 bytes)
  • test.diff (14,061 bytes)
    Index: components/tachart/tachartstrconsts.pas
    ===================================================================
    --- components/tachart/tachartstrconsts.pas	(revision 60443)
    +++ components/tachart/tachartstrconsts.pas	(working copy)
    @@ -69,7 +69,8 @@
       rsDistanceMeasurement = 'Distance measurement';
     
       // Chart sources
    -  rsSourceCountError = '%0:s requires a chart source with at least %1:d %2:s value(s)';
    +  rsSourceCountError = '%0:s requires a chart source with at least %1:d "%2:s" value(s) for each data point';
    +  rsSourceCountError2 = '"%0:s" source must have at least %1:d "%2:s" value(s) for each data point';
     
       // Transformations
       tasAxisTransformsEditorTitle = 'Edit axis transformations';
    Index: components/tachart/tacustomseries.pas
    ===================================================================
    --- components/tachart/tacustomseries.pas	(revision 60443)
    +++ components/tachart/tacustomseries.pas	(working copy)
    @@ -165,7 +165,7 @@
         procedure SourceChanged(ASender: TObject); virtual;
         procedure VisitSources(
           AVisitor: TChartOnSourceVisitor; AAxis: TChartAxis; var AData); override;
    -    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); virtual;
    +    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); virtual;
       strict protected
         function LegendTextPoint(AIndex: Integer): String; inline;
       protected
    @@ -787,15 +787,15 @@
     
     procedure TChartSeries.CheckSource(ASource: TCustomChartSource);
     var
    -  nx, ny: Integer;
    +  nx, ny: Cardinal;
     begin
       if ASource = nil then
         exit;
       GetXYCountNeeded(nx, ny);
       if ASource.XCount < nx then
    -    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'x'])
    -  else if ASource.YCount < ny then
    -    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'y']);
    +    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'X']);
    +  if ASource.YCount < ny then
    +    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'Y']);
     end;
     
     procedure TChartSeries.Clear;
    @@ -816,11 +816,14 @@
     constructor TChartSeries.Create(AOwner: TComponent);
     const
       BUILTIN_SOURCE_NAME = 'Builtin';
    +var
    +  nx, ny: Cardinal;
     begin
       inherited Create(AOwner);
     
       FListener := TListener.Create(@FSource,  @SourceChanged);
    -  FBuiltinSource := TListChartSource.Create(Self);
    +  GetXYCountNeeded(nx, ny);
    +  FBuiltinSource := TListChartSource.Create(Self, nx, ny);
       FBuiltinSource.Name := BUILTIN_SOURCE_NAME;
       FBuiltinSource.Broadcaster.Subscribe(FListener);
       FMarks := TChartMarks.Create(FChart);
    @@ -962,7 +965,7 @@
         Result := 0;
     end;
     
    -class procedure TChartSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
    +class procedure TChartSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
     begin
       AXCount := 1;
       AYCount := 1;
    @@ -1124,6 +1127,12 @@
     
     procedure TChartSeries.SourceChanged(ASender: TObject);
     begin
    +  try
    +    CheckSource(Source);
    +  except
    +    Source := nil; // revert to built-in source
    +    raise;
    +  end;
       StyleChanged(ASender);
     end;
     
    Index: components/tachart/tacustomsource.pas
    ===================================================================
    --- components/tachart/tacustomsource.pas	(revision 60443)
    +++ components/tachart/tacustomsource.pas	(working copy)
    @@ -825,7 +825,7 @@
       inherited Create(AOwner);
       FXCount := 1;
       FYCount := 1;
    -  for i:=0 to 1 do begin
    +  for i:=Low(FErrorBarData) to High(FErrorBarData) do begin
         FErrorBarData[i] := TChartErrorBarData.Create;
         FErrorBarData[i].OnChange := @ChangeErrorBars;
       end;
    Index: components/tachart/tamultiseries.pas
    ===================================================================
    --- components/tachart/tamultiseries.pas	(revision 60443)
    +++ components/tachart/tamultiseries.pas	(working copy)
    @@ -55,7 +55,7 @@
         function GetLabelDataPoint(AIndex, AYIndex: Integer): TDoublePoint; override;
         procedure GetLegendItems(AItems: TChartLegendItems); override;
         function GetSeriesColor: TColor; override;
    -    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
    +    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
         function ToolTargetDistance(const AParams: TNearestPointParams;
           AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
         procedure UpdateLabelDirectionReferenceLevel(AIndex, AYIndex: Integer;
    @@ -111,7 +111,7 @@
       protected
         procedure GetLegendItems(AItems: TChartLegendItems); override;
         function GetSeriesColor: TColor; override;
    -    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
    +    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
         function ToolTargetDistance(const AParams: TNearestPointParams;
           AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
       public
    @@ -179,7 +179,7 @@
       protected
         procedure GetLegendItems(AItems: TChartLegendItems); override;
         function GetSeriesColor: TColor; override;
    -    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
    +    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
         function ToolTargetDistance(const AParams: TNearestPointParams;
           AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
       public
    @@ -233,7 +233,7 @@
         function GetColor(AIndex: Integer): TColor; inline;
         function GetVectorPoints(AIndex: Integer;
           out AStartPt, AEndPt: TDoublePoint): Boolean; inline;
    -    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
    +    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
       public
         procedure Assign(ASource: TPersistent); override;
         constructor Create(AOwner: TComponent); override;
    @@ -465,7 +465,6 @@
     function TBubbleSeries.AddXY(AX, AY, ARadius: Double; AXLabel: String;
       AColor: TColor): Integer;
     begin
    -  if ListSource.YCount < 2 then ListSource.YCount := 2;
       Result := AddXY(AX, AY, [ARadius], AXLabel, AColor);
     end;
     
    @@ -506,8 +505,6 @@
       dummyR: TRect = (Left:0; Top:0; Right:0; Bottom:0);
       ext: TDoubleRect;
     begin
    -  if Source.YCount < 2 then exit;
    -
       ADrawer.Pen := BubblePen;
       ADrawer.Brush := BubbleBrush;
     
    @@ -544,9 +541,6 @@
       item: PChartDataItem;
     begin
       Result := EmptyExtent;
    -  if Source.YCount < 2 then
    -    exit;
    -
       for i := 0 to Count - 1 do begin
         item := Source[i];
         sp := item^.Point;
    @@ -708,7 +702,7 @@
       Result := FBubbleBrush.Color;
     end;
     
    -class procedure TBubbleSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
    +class procedure TBubbleSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
     begin
       AXCount := 1;
       AYCount := 2;
    @@ -896,7 +890,6 @@
       AX, AYLoWhisker, AYLoBox, AY, AYHiBox, AYHiWhisker: Double; AXLabel: String;
       AColor: TColor): Integer;
     begin
    -  if ListSource.YCount < 5 then ListSource.YCount := 5;
       Result := AddXY(
         AX, AYLoWhisker, [AYLoBox, AY, AYHiBox, AYHiWhisker], AXLabel, AColor);
     end;
    @@ -971,7 +964,7 @@
       x, ymin, yqmin, ymed, yqmax, ymax, wb, ww, w: Double;
       i: Integer;
     begin
    -  if IsEmpty or (Source.YCount < 5) then
    +  if IsEmpty then
         exit;
       if FWidthStyle = bwsPercentMin then
         UpdateMinXRange;
    @@ -1028,7 +1021,6 @@
       end;
     
     begin
    -  if Source.YCount < 5 then exit(EmptyExtent);
       Result := Source.ExtentList;
       // Show first and last boxes fully.
       x := GetGraphPointX(0);
    @@ -1122,7 +1114,7 @@
       Result := BoxBrush.Color;
     end;
     
    -class procedure TBoxAndWhiskerSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
    +class procedure TBoxAndWhiskerSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
     begin
       AXCount := 1;
       AYCount := 5;
    @@ -1240,8 +1232,6 @@
     var
       y: Double;
     begin
    -  if ListSource.YCount < 4 then ListSource.YCount := 4;
    -
       if YIndexOpen = 0 then
         y := AOpen
       else if YIndexHigh = 0 then
    @@ -1425,7 +1415,6 @@
       x: Double;
       tw: Double;
     begin
    -  if Source.YCount < 4 then exit(EmptyExtent);
       Result := Source.ExtentList;                            // axis units
       // Show first and last open/close ticks and candle boxes fully.
       x := GetGraphPointX(0);                                 // graph units
    @@ -1521,7 +1510,7 @@
       Result := LinePen.Color;
     end;
     
    -class procedure TOpenHighLowCloseSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
    +class procedure TOpenHighLowCloseSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
     begin
       AXCount := 1;
       AYCount := 4;
    @@ -1671,8 +1660,6 @@
     begin
       inherited Create(AOwner);
       ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
    -  ListSource.XCount := 2;
    -  ListSource.YCount := 2;
       FArrow := TChartArrow.Create(ParentChart);
       FArrow.Length := 20;
       FArrow.Width := 10;
    @@ -1900,7 +1887,7 @@
       end;
     end;
     
    -class procedure TFieldSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
    +class procedure TFieldSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
     begin
       AXCount := 2;
       AYCount := 2;
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60443)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -23,6 +23,8 @@
     
       TListChartSource = class(TCustomChartSource)
       private
    +    FXCountMin: Cardinal;
    +    FYCountMin: Cardinal;
         FData: TFPList;
         FDataPoints: TStrings;
         FSorted: Boolean;
    @@ -44,7 +46,8 @@
           EXListEmptyError = class(EChartError);
           EYListEmptyError = class(EChartError);
       public
    -    constructor Create(AOwner: TComponent); override;
    +    constructor Create(AOwner: TComponent); override; overload;
    +    constructor Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal); overload;
         destructor Destroy; override;
       public
         function Add(
    @@ -73,8 +76,8 @@
         property Sorted: Boolean read FSorted write SetSorted default false;
         property XCount;
         property XErrorBarData;
    +    property YCount;
         property YErrorBarData;
    -    property YCount;
       end;
     
       { TMWCRandomGenerator }
    @@ -247,7 +250,7 @@
     implementation
     
     uses
    -  Math, StrUtils, SysUtils, TAMath;
    +  Math, StrUtils, SysUtils, TAMath, TAChartStrConsts;
     
     type
     
    @@ -304,7 +307,8 @@
       fs := DefaultFormatSettings;
       fs.DecimalSeparator := '.';
       with FSource[Index]^ do begin
    -    Result := Format('%g', [X], fs);
    +    if FSource.XCount > 0 then
    +      Result := Format('%g', [X], fs);
         for i := 0 to High(XList) do
           Result += Format('|%g', [XList[i]], fs);
         if FSource.YCount > 0 then
    @@ -350,13 +354,14 @@
     begin
       parts := Split(AString);
       try
    -    if (FSource.XCount = 1) and (FSource.YCount + 3 < Cardinal(parts.Count)) then
    +    if (FSource.XCount = 1) and (FSource.YCount + 3 < Cardinal(parts.Count)) then // ????? what about XCount <> 1 ?
           FSource.YCount := parts.Count - 3;
         with ADataItem^ do begin
    -      X := StrToFloatOrDateTimeDef(NextPart);
    -      if FSource.XCount > 1 then
    +      if FSource.XCount > 0 then begin
    +        X := StrToFloatOrDateTimeDef(NextPart);
             for i := 0 to High(XList) do
               XList[i] := StrToFloatOrDateTimeDef(NextPart);
    +      end;
           if FSource.YCount > 0 then begin
             Y := StrToFloatOrDateTimeDef(NextPart);
             for i := 0 to High(YList) do
    @@ -467,14 +472,25 @@
     var
       i: Integer;
     begin
    +  if ASource.XCount < FXCountMin then
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [Name, FXCountMin, 'X']);
    +  if ASource.YCount < FYCountMin then
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [Name, FYCountMin, 'Y']);
    +
       BeginUpdate;
       try
         Clear;
    +    if ASource is TListChartSource then
    +    begin
    +      FXCountMin := Max(FXCountMin, TListChartSource(ASource).FXCountMin);
    +      FYCountMin := Max(FYCountMin, TListChartSource(ASource).FYCountMin);
    +    end;
    +    XCount := ASource.XCount;
         YCount := ASource.YCount;
         for i := 0 to ASource.Count - 1 do
           with ASource[i]^ do begin
    -        AddAt(FData.Count, X, Y, Text, Color);
    -        SetYList(FData.Count - 1, YList);
    +        AddAt(FData.Count, X, Y, Text, Color); // ????? what about XCount = 0 and/or YCount = 0 ?
    +        SetYList(FData.Count - 1, YList); // ????? what about XCount?
           end;
         if Sorted and not ASource.IsSorted then Sort;
       finally
    @@ -487,10 +503,20 @@
       inherited Create(AOwner);
       FData := TFPList.Create;
       FDataPoints := TListChartSourceStrings.Create(Self);
    -  FYCount := 1;
       ClearCaches;
     end;
     
    +constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
    +begin
    +  Create(AOwner);
    +  FXCountMin := AXCountMin;
    +  FYCountMin := AYCountMin;
    +  if FXCount < FXCountMin then
    +    FXCount := FXCountMin;
    +  if FYCount < FYCountMin then
    +    FYCount := FYCountMin;
    +end;
    +
     procedure TListChartSource.Delete(AIndex: Integer);
     begin
       with Item[AIndex]^ do begin
    @@ -579,10 +605,13 @@
     var
       i: Integer;
     begin
    +  if AValue < FXCountMin then
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [Name, FXCountMin, 'X']);
       if AValue = FXCount then exit;
       FXCount := AValue;
       for i := 0 to Count - 1 do
         SetLength(Item[i]^.XList, Max(FXCount - 1, 0));
    +  Notify;
     end;
     
     procedure TListChartSource.SetXList(
    @@ -653,10 +682,13 @@
     var
       i: Integer;
     begin
    +  if AValue < FYCountMin then
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [Name, FYCountMin, 'Y']);
       if AValue = FYCount then exit;
       FYCount := AValue;
       for i := 0 to Count - 1 do
         SetLength(Item[i]^.YList, Max(FYCount - 1, 0));
    +  Notify;
     end;
     
     procedure TListChartSource.SetYList(
    @@ -807,7 +839,6 @@
       inherited Create(AOwner);
       FCurItem.Color := clTAColor;
       FRNG := TMWCRandomGenerator.Create;
    -  FYCount := 1;
       RandSeed := Trunc(Frac(Now) * MaxInt);
     end;
     
    @@ -1218,7 +1249,7 @@
     begin
       if
         (FOrigin <> nil) and (ASender = FOrigin) and
    -    (FOrigin.YCount <> FOriginYCount)
    +    (FOrigin.YCount <> FOriginYCount) // ????? what about XCount?
       then begin
         UpdateYOrder;
         exit;
    
    test.diff (14,061 bytes)
  • Reproduce2a.png (4,299 bytes)
    Reproduce2a.png (4,299 bytes)
  • Reproduce2b.png (14,056 bytes)
    Reproduce2b.png (14,056 bytes)
  • Reproduce2.zip (3,128 bytes)
  • patch2.diff (4,789 bytes)
    Index: components/tachart/tachartstrconsts.pas
    ===================================================================
    --- components/tachart/tachartstrconsts.pas	(revision 60463)
    +++ components/tachart/tachartstrconsts.pas	(working copy)
    @@ -69,7 +69,8 @@
       rsDistanceMeasurement = 'Distance measurement';
     
       // Chart sources
    -  rsSourceCountError = '%0:s requires a chart source with at least %1:d %2:s value(s) per data point.';
    +  rsSourceCountError = '%0:s requires a chart source with at least %1:d "%2:s" value(s) per data point.';
    +  rsSourceCountError2 = 'This %0:s instance must have at least %1:d "%2:s" value(s) per data point.';
     
       // Transformations
       tasAxisTransformsEditorTitle = 'Edit axis transformations';
    Index: components/tachart/tacustomseries.pas
    ===================================================================
    --- components/tachart/tacustomseries.pas	(revision 60463)
    +++ components/tachart/tacustomseries.pas	(working copy)
    @@ -788,9 +788,9 @@
         exit;
       GetXYCountNeeded(nx, ny);
       if ASource.XCount < nx then
    -    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'x'])
    -  else if ASource.YCount < ny then
    -    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'y']);
    +    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'X']);
    +  if ASource.YCount < ny then
    +    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'Y']);
     end;
     
     procedure TChartSeries.Clear;
    @@ -817,11 +817,9 @@
       inherited Create(AOwner);
     
       FListener := TListener.Create(@FSource,  @SourceChanged);
    -  FBuiltinSource := TListChartSource.Create(Self);
    +  GetXYCountNeeded(nx, ny);
    +  FBuiltinSource := TListChartSource.Create(Self, nx, ny);
       FBuiltinSource.Name := BUILTIN_SOURCE_NAME;
    -  GetXYCountNeeded(nx, ny);
    -  FBuiltinSource.XCount := nx;
    -  FBuiltinSource.YCount := ny;
       FBuiltinSource.Broadcaster.Subscribe(FListener);
       FMarks := TChartMarks.Create(FChart);
       FStylesListener := TListener.Create(@FStyles,  @StyleChanged);
    @@ -1125,7 +1123,12 @@
     procedure TChartSeries.SourceChanged(ASender: TObject);
     begin
       if ASender is TCustomChartSource then
    +  try
         CheckSource(TCustomChartSource(ASender));
    +  except
    +    Source := nil; // revert to built-in source
    +    raise;
    +  end;
       StyleChanged(ASender);
     end;
     
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60463)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -23,6 +23,8 @@
     
       TListChartSource = class(TCustomChartSource)
       private
    +    FXCountMin: Cardinal;
    +    FYCountMin: Cardinal;
         FData: TFPList;
         FDataPoints: TStrings;
         FSorted: Boolean;
    @@ -44,7 +46,8 @@
           EXListEmptyError = class(EChartError);
           EYListEmptyError = class(EChartError);
       public
    -    constructor Create(AOwner: TComponent); override;
    +    constructor Create(AOwner: TComponent); override; overload;
    +    constructor Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal); overload;
         destructor Destroy; override;
       public
         function Add(
    @@ -247,7 +250,7 @@
     implementation
     
     uses
    -  Math, StrUtils, SysUtils, TAMath;
    +  Math, StrUtils, SysUtils, TAMath, TAChartStrConsts;
     
     type
     
    @@ -467,6 +470,11 @@
     var
       i: Integer;
     begin
    +  if ASource.XCount < FXCountMin then
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
    +  if ASource.YCount < FYCountMin then
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
    +
       BeginUpdate;
       try
         Clear;
    @@ -492,6 +500,17 @@
       ClearCaches;
     end;
     
    +constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
    +begin
    +  Create(AOwner);
    +  FXCountMin := AXCountMin;
    +  FYCountMin := AYCountMin;
    +  if FXCount < FXCountMin then
    +    FXCount := FXCountMin;
    +  if FYCount < FYCountMin then
    +    FYCount := FYCountMin;
    +end;
    +
     procedure TListChartSource.Delete(AIndex: Integer);
     begin
       with Item[AIndex]^ do begin
    @@ -580,10 +599,13 @@
     var
       i: Integer;
     begin
    +  if AValue < FXCountMin then
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
       if AValue = FXCount then exit;
       FXCount := AValue;
       for i := 0 to Count - 1 do
         SetLength(Item[i]^.XList, Max(FXCount - 1, 0));
    +  Notify;
     end;
     
     procedure TListChartSource.SetXList(
    @@ -654,10 +676,13 @@
     var
       i: Integer;
     begin
    +  if AValue < FYCountMin then
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
       if AValue = FYCount then exit;
       FYCount := AValue;
       for i := 0 to Count - 1 do
         SetLength(Item[i]^.YList, Max(FYCount - 1, 0));
    +  Notify;
     end;
     
     procedure TListChartSource.SetYList(
    
    patch2.diff (4,789 bytes)
  • patch3.diff (3,956 bytes)
    Index: components/tachart/tamultiseries.pas
    ===================================================================
    --- components/tachart/tamultiseries.pas	(revision 60470)
    +++ components/tachart/tamultiseries.pas	(working copy)
    @@ -514,11 +514,7 @@
       irect: TRect;
       dummyR: TRect = (Left:0; Top:0; Right:0; Bottom:0);
       ext: TDoubleRect;
    -  nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if Source.YCount < ny then exit;
    -
       ADrawer.Pen := BubblePen;
       ADrawer.Brush := BubbleBrush;
     
    @@ -557,9 +553,6 @@
       item: PChartDataItem;
     begin
       Result := EmptyExtent;
    -  if Source.YCount < 2 then
    -    exit;
    -
       for i := 0 to Count - 1 do begin
         item := Source[i];
         sp := item^.Point;
    @@ -982,8 +975,7 @@
       i: Integer;
       nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if IsEmpty or (Source.YCount < ny) then
    +  if IsEmpty then
         exit;
       if FWidthStyle = bwsPercentMin then
         UpdateMinXRange;
    @@ -1029,8 +1021,9 @@
         DoLine(x - wb, ymed, x + wb, ymed);
       end;
     
    +  GetXYCountNeeded(nx, ny);
       if Source.YCount > ny then
    -    for i := 0 to ny-1 do DrawLabels(ADrawer, ny)
    +    for i := 0 to ny-1 do DrawLabels(ADrawer, i)
       else
         DrawLabels(ADrawer);
     end;
    @@ -1046,7 +1039,6 @@
       end;
     
     begin
    -  if Source.YCount < 5 then exit(EmptyExtent);
       Result := Source.ExtentList;
       // Show first and last boxes fully.
       j := -1;
    @@ -1431,9 +1423,6 @@
       p: TPen;
       nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
    -
       my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
       if IsEmpty or (my >= Source.YCount) then exit;
     
    @@ -1474,6 +1463,7 @@
         end;
       end;
     
    +  GetXYCountNeeded(nx, ny);
       if Source.YCount > ny then
         for i := 0 to ny-1 do DrawLabels(ADrawer, i)
       else
    @@ -1486,7 +1476,6 @@
       tw: Double;
       j: Integer;
     begin
    -  if Source.YCount < 4 then exit(EmptyExtent);
       Result := Source.ExtentList;                            // axis units
       // Show first and last open/close ticks and candle boxes fully.
       j := -1;
    @@ -1764,8 +1753,6 @@
     begin
       inherited Create(AOwner);
       ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
    -  ListSource.XCount := 2;
    -  ListSource.YCount := 2;
       FArrow := TChartArrow.Create(ParentChart);
       FArrow.Length := 20;
       FArrow.Width := 10;
    @@ -1810,11 +1797,7 @@
       i: Integer;
       p1, p2: TDoublePoint;
       lPen: TPen;
    -  nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
    -
       with Extent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60470)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -75,8 +75,8 @@
         property Sorted: Boolean read FSorted write SetSorted default false;
         property XCount;
         property XErrorBarData;
    +    property YCount;
         property YErrorBarData;
    -    property YCount;
       end;
     
       { TMWCRandomGenerator }
    @@ -599,7 +599,7 @@
       i: Integer;
     begin
       if AValue < FXCountMin then
    -    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'x']);
       if AValue = FXCount then exit;
       FXCount := AValue;
       for i := 0 to Count - 1 do
    @@ -676,7 +676,7 @@
       i: Integer;
     begin
       if AValue < FYCountMin then
    -    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'y']);
       if AValue = FYCount then exit;
       FYCount := AValue;
       for i := 0 to Count - 1 do
    @@ -832,7 +832,6 @@
       inherited Create(AOwner);
       FCurItem.Color := clTAColor;
       FRNG := TMWCRandomGenerator.Create;
    -  FYCount := 1;
       RandSeed := Trunc(Frac(Now) * MaxInt);
     end;
     
    
    patch3.diff (3,956 bytes)
  • patch3_updated.diff (4,175 bytes)
    Index: components/tachart/tamultiseries.pas
    ===================================================================
    --- components/tachart/tamultiseries.pas	(revision 60470)
    +++ components/tachart/tamultiseries.pas	(working copy)
    @@ -516,9 +516,6 @@
       ext: TDoubleRect;
       nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if Source.YCount < ny then exit;
    -
       ADrawer.Pen := BubblePen;
       ADrawer.Brush := BubbleBrush;
     
    @@ -540,6 +537,7 @@
           ADrawer.SetBrushColor(ColorDef(item^.Color, BubbleBrush.Color));
         ADrawer.Ellipse(irect.Left, irect.Top, irect.Right, irect.Bottom);
       end;
    +  GetXYCountNeeded(nx, ny);
       if Source.YCount > ny then
         for i := 0 to ny - 1 do DrawLabels(ADrawer, i)
       else
    @@ -557,9 +555,6 @@
       item: PChartDataItem;
     begin
       Result := EmptyExtent;
    -  if Source.YCount < 2 then
    -    exit;
    -
       for i := 0 to Count - 1 do begin
         item := Source[i];
         sp := item^.Point;
    @@ -982,8 +977,7 @@
       i: Integer;
       nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if IsEmpty or (Source.YCount < ny) then
    +  if IsEmpty then
         exit;
       if FWidthStyle = bwsPercentMin then
         UpdateMinXRange;
    @@ -1029,8 +1023,9 @@
         DoLine(x - wb, ymed, x + wb, ymed);
       end;
     
    +  GetXYCountNeeded(nx, ny);
       if Source.YCount > ny then
    -    for i := 0 to ny-1 do DrawLabels(ADrawer, ny)
    +    for i := 0 to ny-1 do DrawLabels(ADrawer, i)
       else
         DrawLabels(ADrawer);
     end;
    @@ -1046,7 +1041,6 @@
       end;
     
     begin
    -  if Source.YCount < 5 then exit(EmptyExtent);
       Result := Source.ExtentList;
       // Show first and last boxes fully.
       j := -1;
    @@ -1431,9 +1425,6 @@
       p: TPen;
       nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
    -
       my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
       if IsEmpty or (my >= Source.YCount) then exit;
     
    @@ -1474,6 +1465,7 @@
         end;
       end;
     
    +  GetXYCountNeeded(nx, ny);
       if Source.YCount > ny then
         for i := 0 to ny-1 do DrawLabels(ADrawer, i)
       else
    @@ -1486,7 +1478,6 @@
       tw: Double;
       j: Integer;
     begin
    -  if Source.YCount < 4 then exit(EmptyExtent);
       Result := Source.ExtentList;                            // axis units
       // Show first and last open/close ticks and candle boxes fully.
       j := -1;
    @@ -1764,8 +1755,6 @@
     begin
       inherited Create(AOwner);
       ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
    -  ListSource.XCount := 2;
    -  ListSource.YCount := 2;
       FArrow := TChartArrow.Create(ParentChart);
       FArrow.Length := 20;
       FArrow.Width := 10;
    @@ -1810,11 +1799,7 @@
       i: Integer;
       p1, p2: TDoublePoint;
       lPen: TPen;
    -  nx, ny: Cardinal;
     begin
    -  GetXYCountNeeded(nx, ny);
    -  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
    -
       with Extent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60470)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -75,8 +75,8 @@
         property Sorted: Boolean read FSorted write SetSorted default false;
         property XCount;
         property XErrorBarData;
    +    property YCount;
         property YErrorBarData;
    -    property YCount;
       end;
     
       { TMWCRandomGenerator }
    @@ -599,7 +599,7 @@
       i: Integer;
     begin
       if AValue < FXCountMin then
    -    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
    +    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'x']);
       if AValue = FXCount then exit;
       FXCount := AValue;
       for i := 0 to Count - 1 do
    @@ -676,7 +676,7 @@
       i: Integer;
     begin
       if AValue < FYCountMin then
    -    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
    +    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'y']);
       if AValue = FYCount then exit;
       FYCount := AValue;
       for i := 0 to Count - 1 do
    @@ -832,7 +832,6 @@
       inherited Create(AOwner);
       FCurItem.Color := clTAColor;
       FRNG := TMWCRandomGenerator.Create;
    -  FYCount := 1;
       RandSeed := Trunc(Frac(Now) * MaxInt);
     end;
     
    
    patch3_updated.diff (4,175 bytes)
  • patch4.diff (958 bytes)
    Index: components/tachart/tacustomseries.pas
    ===================================================================
    --- components/tachart/tacustomseries.pas	(revision 60471)
    +++ components/tachart/tacustomseries.pas	(working copy)
    @@ -1069,10 +1069,12 @@
     
     procedure TChartSeries.SetSource(AValue: TCustomChartSource);
     begin
    +  if AValue = FBuiltinSource then
    +    AValue := nil;
       if FSource = AValue then exit;
    +  CheckSource(AValue);
       if FListener.IsListening then
         Source.Broadcaster.Unsubscribe(FListener);
    -  CheckSource(AValue);
       FSource := AValue;
       Source.Broadcaster.Subscribe(FListener);
       SourceChanged(Self);
    @@ -1122,7 +1124,7 @@
     
     procedure TChartSeries.SourceChanged(ASender: TObject);
     begin
    -  if ASender is TCustomChartSource then
    +  if (ASender <> FBuiltinSource) and (ASender is TCustomChartSource) then // built-in source is guaranteed to be valid
         try
           CheckSource(TCustomChartSource(ASender));
         except
    
    patch4.diff (958 bytes)

Relationships

related to 0035665 closedwp TAChart: deriving from TListChartSource may lead to infinite loop 

Activities

Marcin Wiazowski

2019-02-15 02:32

reporter  

Reproduce.zip (3,082 bytes)

Marcin Wiazowski

2019-02-15 14:39

reporter   ~0114147

TFieldSeries requires data source having XCount >= 2 and Y >= 2.

The crash is probably because TFieldSeries blindly assumes that this is always true.

wp

2019-02-15 16:22

developer   ~0114153

Fixed. Thanks for reporting.

Marcin Wiazowski

2019-02-18 01:28

reporter   ~0114234

The GetXYCountNeeded() procedure, that you introduced, is great!

Now it's much better, but crash is still possible: it's enough to assign a source having XCount = YCount = 2 to TFieldSeries, but decrease XCount and/or YCount *later*. I'm attaching a patch, that solves the problem.



General ideas:

- A series' source can be: a built-in source or some external, assigned source.

- In TListChartSource.SetXCount() and SetYCount(), calls to "Notify()" have been added (apparently they have been lacking) - so source's user can verify that new XCount and YCount values are valid; if not valid, a built-in source is assigned to the series back.

- But we must also prevent the built-in source from setting invalid XCount and/or YCount values. To achieve this, I added an additional constructor to TListChartSource:

    constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
    begin
      Create(AOwner); <==== DEFAULT CONSTRUCTOR IS CALLED
      FXCountMin := AXCountMin;
      FYCountMin := AYCountMin;
      if FXCount < FXCountMin then
        FXCount := FXCountMin;
      if FYCount < FYCountMin then
        FYCount := FYCountMin;
    end;

  The newly introduced FXCountMin / FYCountMin values are used in SetXCount() / SetYCount() for verification - an exception is raised when trying to set too low values.


  The new constructor is now used in TChartSeries.Create:

    GetXYCountNeeded(nx, ny);
    FBuiltinSource := TListChartSource.Create(Self, nx, ny);

  so built-in source cannot be damaged anymore; a nice side effect is that all the initialization / verification code for XCount / YCount, in all TChartSeries descendants, can be just removed.



Some other changes:

A) XCount and YCount are Cardinals, so the GetXYCountNeeded() procedure has been modified to return also Cardinals (to avoid comparing signed Integers with unsigned Cardinals).

B) Some cosmetic changes were made.

C) In TListChartSource implementation, in some places, XCount is still assumed to be always equal 1. This is not true - XCount and YCount can have any values, including zero. I corrected some of these places, in some other I just added "?????" comments (I was not sure if and how the code should be corrected). Please verify the changes that I made for XCount and take a look at these "?????" comments.

D) In TListChartSource.Create() andTRandomChartSource.Create, redundant assignments "FYCount := 1" have been removed.

E) A message like:

  'TFieldSeries requires a chart source with at least 2 x value(s)'

  was very confusing to me: I always read "2 x" as "2x" = "2 * of something" = "2 times". So I modified the message to:

  'TFieldSeries requires a chart source with at least 2 "X" value(s) for each data point'

Marcin Wiazowski

2019-02-18 01:28

reporter  

test.diff (14,061 bytes)
Index: components/tachart/tachartstrconsts.pas
===================================================================
--- components/tachart/tachartstrconsts.pas	(revision 60443)
+++ components/tachart/tachartstrconsts.pas	(working copy)
@@ -69,7 +69,8 @@
   rsDistanceMeasurement = 'Distance measurement';
 
   // Chart sources
-  rsSourceCountError = '%0:s requires a chart source with at least %1:d %2:s value(s)';
+  rsSourceCountError = '%0:s requires a chart source with at least %1:d "%2:s" value(s) for each data point';
+  rsSourceCountError2 = '"%0:s" source must have at least %1:d "%2:s" value(s) for each data point';
 
   // Transformations
   tasAxisTransformsEditorTitle = 'Edit axis transformations';
Index: components/tachart/tacustomseries.pas
===================================================================
--- components/tachart/tacustomseries.pas	(revision 60443)
+++ components/tachart/tacustomseries.pas	(working copy)
@@ -165,7 +165,7 @@
     procedure SourceChanged(ASender: TObject); virtual;
     procedure VisitSources(
       AVisitor: TChartOnSourceVisitor; AAxis: TChartAxis; var AData); override;
-    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); virtual;
+    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); virtual;
   strict protected
     function LegendTextPoint(AIndex: Integer): String; inline;
   protected
@@ -787,15 +787,15 @@
 
 procedure TChartSeries.CheckSource(ASource: TCustomChartSource);
 var
-  nx, ny: Integer;
+  nx, ny: Cardinal;
 begin
   if ASource = nil then
     exit;
   GetXYCountNeeded(nx, ny);
   if ASource.XCount < nx then
-    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'x'])
-  else if ASource.YCount < ny then
-    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'y']);
+    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'X']);
+  if ASource.YCount < ny then
+    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'Y']);
 end;
 
 procedure TChartSeries.Clear;
@@ -816,11 +816,14 @@
 constructor TChartSeries.Create(AOwner: TComponent);
 const
   BUILTIN_SOURCE_NAME = 'Builtin';
+var
+  nx, ny: Cardinal;
 begin
   inherited Create(AOwner);
 
   FListener := TListener.Create(@FSource,  @SourceChanged);
-  FBuiltinSource := TListChartSource.Create(Self);
+  GetXYCountNeeded(nx, ny);
+  FBuiltinSource := TListChartSource.Create(Self, nx, ny);
   FBuiltinSource.Name := BUILTIN_SOURCE_NAME;
   FBuiltinSource.Broadcaster.Subscribe(FListener);
   FMarks := TChartMarks.Create(FChart);
@@ -962,7 +965,7 @@
     Result := 0;
 end;
 
-class procedure TChartSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
+class procedure TChartSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
 begin
   AXCount := 1;
   AYCount := 1;
@@ -1124,6 +1127,12 @@
 
 procedure TChartSeries.SourceChanged(ASender: TObject);
 begin
+  try
+    CheckSource(Source);
+  except
+    Source := nil; // revert to built-in source
+    raise;
+  end;
   StyleChanged(ASender);
 end;
 
Index: components/tachart/tacustomsource.pas
===================================================================
--- components/tachart/tacustomsource.pas	(revision 60443)
+++ components/tachart/tacustomsource.pas	(working copy)
@@ -825,7 +825,7 @@
   inherited Create(AOwner);
   FXCount := 1;
   FYCount := 1;
-  for i:=0 to 1 do begin
+  for i:=Low(FErrorBarData) to High(FErrorBarData) do begin
     FErrorBarData[i] := TChartErrorBarData.Create;
     FErrorBarData[i].OnChange := @ChangeErrorBars;
   end;
Index: components/tachart/tamultiseries.pas
===================================================================
--- components/tachart/tamultiseries.pas	(revision 60443)
+++ components/tachart/tamultiseries.pas	(working copy)
@@ -55,7 +55,7 @@
     function GetLabelDataPoint(AIndex, AYIndex: Integer): TDoublePoint; override;
     procedure GetLegendItems(AItems: TChartLegendItems); override;
     function GetSeriesColor: TColor; override;
-    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
+    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
     function ToolTargetDistance(const AParams: TNearestPointParams;
       AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
     procedure UpdateLabelDirectionReferenceLevel(AIndex, AYIndex: Integer;
@@ -111,7 +111,7 @@
   protected
     procedure GetLegendItems(AItems: TChartLegendItems); override;
     function GetSeriesColor: TColor; override;
-    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
+    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
     function ToolTargetDistance(const AParams: TNearestPointParams;
       AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
   public
@@ -179,7 +179,7 @@
   protected
     procedure GetLegendItems(AItems: TChartLegendItems); override;
     function GetSeriesColor: TColor; override;
-    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
+    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
     function ToolTargetDistance(const AParams: TNearestPointParams;
       AGraphPt: TDoublePoint; APointIdx, AXIdx, AYIdx: Integer): Integer; override;
   public
@@ -233,7 +233,7 @@
     function GetColor(AIndex: Integer): TColor; inline;
     function GetVectorPoints(AIndex: Integer;
       out AStartPt, AEndPt: TDoublePoint): Boolean; inline;
-    class procedure GetXYCountNeeded(out AXCount, AYCount: Integer); override;
+    class procedure GetXYCountNeeded(out AXCount, AYCount: Cardinal); override;
   public
     procedure Assign(ASource: TPersistent); override;
     constructor Create(AOwner: TComponent); override;
@@ -465,7 +465,6 @@
 function TBubbleSeries.AddXY(AX, AY, ARadius: Double; AXLabel: String;
   AColor: TColor): Integer;
 begin
-  if ListSource.YCount < 2 then ListSource.YCount := 2;
   Result := AddXY(AX, AY, [ARadius], AXLabel, AColor);
 end;
 
@@ -506,8 +505,6 @@
   dummyR: TRect = (Left:0; Top:0; Right:0; Bottom:0);
   ext: TDoubleRect;
 begin
-  if Source.YCount < 2 then exit;
-
   ADrawer.Pen := BubblePen;
   ADrawer.Brush := BubbleBrush;
 
@@ -544,9 +541,6 @@
   item: PChartDataItem;
 begin
   Result := EmptyExtent;
-  if Source.YCount < 2 then
-    exit;
-
   for i := 0 to Count - 1 do begin
     item := Source[i];
     sp := item^.Point;
@@ -708,7 +702,7 @@
   Result := FBubbleBrush.Color;
 end;
 
-class procedure TBubbleSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
+class procedure TBubbleSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
 begin
   AXCount := 1;
   AYCount := 2;
@@ -896,7 +890,6 @@
   AX, AYLoWhisker, AYLoBox, AY, AYHiBox, AYHiWhisker: Double; AXLabel: String;
   AColor: TColor): Integer;
 begin
-  if ListSource.YCount < 5 then ListSource.YCount := 5;
   Result := AddXY(
     AX, AYLoWhisker, [AYLoBox, AY, AYHiBox, AYHiWhisker], AXLabel, AColor);
 end;
@@ -971,7 +964,7 @@
   x, ymin, yqmin, ymed, yqmax, ymax, wb, ww, w: Double;
   i: Integer;
 begin
-  if IsEmpty or (Source.YCount < 5) then
+  if IsEmpty then
     exit;
   if FWidthStyle = bwsPercentMin then
     UpdateMinXRange;
@@ -1028,7 +1021,6 @@
   end;
 
 begin
-  if Source.YCount < 5 then exit(EmptyExtent);
   Result := Source.ExtentList;
   // Show first and last boxes fully.
   x := GetGraphPointX(0);
@@ -1122,7 +1114,7 @@
   Result := BoxBrush.Color;
 end;
 
-class procedure TBoxAndWhiskerSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
+class procedure TBoxAndWhiskerSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
 begin
   AXCount := 1;
   AYCount := 5;
@@ -1240,8 +1232,6 @@
 var
   y: Double;
 begin
-  if ListSource.YCount < 4 then ListSource.YCount := 4;
-
   if YIndexOpen = 0 then
     y := AOpen
   else if YIndexHigh = 0 then
@@ -1425,7 +1415,6 @@
   x: Double;
   tw: Double;
 begin
-  if Source.YCount < 4 then exit(EmptyExtent);
   Result := Source.ExtentList;                            // axis units
   // Show first and last open/close ticks and candle boxes fully.
   x := GetGraphPointX(0);                                 // graph units
@@ -1521,7 +1510,7 @@
   Result := LinePen.Color;
 end;
 
-class procedure TOpenHighLowCloseSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
+class procedure TOpenHighLowCloseSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
 begin
   AXCount := 1;
   AYCount := 4;
@@ -1671,8 +1660,6 @@
 begin
   inherited Create(AOwner);
   ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
-  ListSource.XCount := 2;
-  ListSource.YCount := 2;
   FArrow := TChartArrow.Create(ParentChart);
   FArrow.Length := 20;
   FArrow.Width := 10;
@@ -1900,7 +1887,7 @@
   end;
 end;
 
-class procedure TFieldSeries.GetXYCountNeeded(out AXCount, AYCount: Integer);
+class procedure TFieldSeries.GetXYCountNeeded(out AXCount, AYCount: Cardinal);
 begin
   AXCount := 2;
   AYCount := 2;
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60443)
+++ components/tachart/tasources.pas	(working copy)
@@ -23,6 +23,8 @@
 
   TListChartSource = class(TCustomChartSource)
   private
+    FXCountMin: Cardinal;
+    FYCountMin: Cardinal;
     FData: TFPList;
     FDataPoints: TStrings;
     FSorted: Boolean;
@@ -44,7 +46,8 @@
       EXListEmptyError = class(EChartError);
       EYListEmptyError = class(EChartError);
   public
-    constructor Create(AOwner: TComponent); override;
+    constructor Create(AOwner: TComponent); override; overload;
+    constructor Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal); overload;
     destructor Destroy; override;
   public
     function Add(
@@ -73,8 +76,8 @@
     property Sorted: Boolean read FSorted write SetSorted default false;
     property XCount;
     property XErrorBarData;
+    property YCount;
     property YErrorBarData;
-    property YCount;
   end;
 
   { TMWCRandomGenerator }
@@ -247,7 +250,7 @@
 implementation
 
 uses
-  Math, StrUtils, SysUtils, TAMath;
+  Math, StrUtils, SysUtils, TAMath, TAChartStrConsts;
 
 type
 
@@ -304,7 +307,8 @@
   fs := DefaultFormatSettings;
   fs.DecimalSeparator := '.';
   with FSource[Index]^ do begin
-    Result := Format('%g', [X], fs);
+    if FSource.XCount > 0 then
+      Result := Format('%g', [X], fs);
     for i := 0 to High(XList) do
       Result += Format('|%g', [XList[i]], fs);
     if FSource.YCount > 0 then
@@ -350,13 +354,14 @@
 begin
   parts := Split(AString);
   try
-    if (FSource.XCount = 1) and (FSource.YCount + 3 < Cardinal(parts.Count)) then
+    if (FSource.XCount = 1) and (FSource.YCount + 3 < Cardinal(parts.Count)) then // ????? what about XCount <> 1 ?
       FSource.YCount := parts.Count - 3;
     with ADataItem^ do begin
-      X := StrToFloatOrDateTimeDef(NextPart);
-      if FSource.XCount > 1 then
+      if FSource.XCount > 0 then begin
+        X := StrToFloatOrDateTimeDef(NextPart);
         for i := 0 to High(XList) do
           XList[i] := StrToFloatOrDateTimeDef(NextPart);
+      end;
       if FSource.YCount > 0 then begin
         Y := StrToFloatOrDateTimeDef(NextPart);
         for i := 0 to High(YList) do
@@ -467,14 +472,25 @@
 var
   i: Integer;
 begin
+  if ASource.XCount < FXCountMin then
+    raise EXCountError.CreateFmt(rsSourceCountError2, [Name, FXCountMin, 'X']);
+  if ASource.YCount < FYCountMin then
+    raise EYCountError.CreateFmt(rsSourceCountError2, [Name, FYCountMin, 'Y']);
+
   BeginUpdate;
   try
     Clear;
+    if ASource is TListChartSource then
+    begin
+      FXCountMin := Max(FXCountMin, TListChartSource(ASource).FXCountMin);
+      FYCountMin := Max(FYCountMin, TListChartSource(ASource).FYCountMin);
+    end;
+    XCount := ASource.XCount;
     YCount := ASource.YCount;
     for i := 0 to ASource.Count - 1 do
       with ASource[i]^ do begin
-        AddAt(FData.Count, X, Y, Text, Color);
-        SetYList(FData.Count - 1, YList);
+        AddAt(FData.Count, X, Y, Text, Color); // ????? what about XCount = 0 and/or YCount = 0 ?
+        SetYList(FData.Count - 1, YList); // ????? what about XCount?
       end;
     if Sorted and not ASource.IsSorted then Sort;
   finally
@@ -487,10 +503,20 @@
   inherited Create(AOwner);
   FData := TFPList.Create;
   FDataPoints := TListChartSourceStrings.Create(Self);
-  FYCount := 1;
   ClearCaches;
 end;
 
+constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
+begin
+  Create(AOwner);
+  FXCountMin := AXCountMin;
+  FYCountMin := AYCountMin;
+  if FXCount < FXCountMin then
+    FXCount := FXCountMin;
+  if FYCount < FYCountMin then
+    FYCount := FYCountMin;
+end;
+
 procedure TListChartSource.Delete(AIndex: Integer);
 begin
   with Item[AIndex]^ do begin
@@ -579,10 +605,13 @@
 var
   i: Integer;
 begin
+  if AValue < FXCountMin then
+    raise EXCountError.CreateFmt(rsSourceCountError2, [Name, FXCountMin, 'X']);
   if AValue = FXCount then exit;
   FXCount := AValue;
   for i := 0 to Count - 1 do
     SetLength(Item[i]^.XList, Max(FXCount - 1, 0));
+  Notify;
 end;
 
 procedure TListChartSource.SetXList(
@@ -653,10 +682,13 @@
 var
   i: Integer;
 begin
+  if AValue < FYCountMin then
+    raise EYCountError.CreateFmt(rsSourceCountError2, [Name, FYCountMin, 'Y']);
   if AValue = FYCount then exit;
   FYCount := AValue;
   for i := 0 to Count - 1 do
     SetLength(Item[i]^.YList, Max(FYCount - 1, 0));
+  Notify;
 end;
 
 procedure TListChartSource.SetYList(
@@ -807,7 +839,6 @@
   inherited Create(AOwner);
   FCurItem.Color := clTAColor;
   FRNG := TMWCRandomGenerator.Create;
-  FYCount := 1;
   RandSeed := Trunc(Frac(Now) * MaxInt);
 end;
 
@@ -1218,7 +1249,7 @@
 begin
   if
     (FOrigin <> nil) and (ASender = FOrigin) and
-    (FOrigin.YCount <> FOriginYCount)
+    (FOrigin.YCount <> FOriginYCount) // ????? what about XCount?
   then begin
     UpdateYOrder;
     exit;
test.diff (14,061 bytes)

wp

2019-02-19 00:54

developer   ~0114254

There's a problem: Chart sources and series are independent, and my gut feeling tells me that a series should not be allowed to tailor a source to its needs. This is because a chart source can be attached to several series. What is good for one series type may be bad for another series. Example: Suppose there is a chart source with financial data intended for an OHLC plot ("open"/"high"/"low"/"close" prices of some share value). When your code forces its minimum y count to 4 it will no longer be possible to assign this source to a line series to plot only the "open" prices - there will always be 4 curves now.

The only occasion where I would accept this behavior is in relation to the built-in listsource which is owned by the series, and there is no (well, little) risk to interfere with another series.

I fixed it like you by hooking into the notification which the series receives when the source is changed. However, I did not reactivate the ListSource; I did not see a necessity for it so far, and I guess that in weird cases the ListSource can have issues either, so, this is probably not a safe drawback.

From your code I also took the idea to set the XCount and YCount of the build-in list source in the constructor of the series.

-----------------

As for the other changes:

(A) I don't like Cardinals: Although a "count" can never be negative physically, I always trap into cases where I want to set it to -1, for example to indicate that that the user did not yet enter any value for it). Changed all XCount and YCount occurances to Integer - and noticed that it breaks compilation of the visual fpspreadsheet components. Maybe there are other third-party chart sources out there. Breaking them for no real good reason is not nice, so I reverted my changes and applied your modification.

-------------------

(C) XCount was introduced for something like a Gantt series (still unfinished sources lying around somewhere) which turned out to become quite complicated; therefore, I had decided to test it with the FieldSeries which so far is the only series with more than one x value. For providing multiple x values I adapted the chart sources which I could easily test. Lacking a good test series, I did not touch the TCalculatedChartSource because I cannot imagine that there is a use case with several x values, so far. I'd prefer to keep it this way until necessity arises.

---------------

XCount and YCount cannot be zero. The "usual" x and y values of a data point are hard-coded elements of the TChartDataItem record which is provided by the chart source. XCount and YCount are defined such that these values are included (http://wiki.lazarus.freepascal.org/TAChart_documentation#Multi-valued_sources). And having no x and y data does not make any sense for plotting. There are, however, also series which do not have a chart source - they get their data from code or events (TFuncSeries).

------------------

TListChartSourceStrings:

The ListChartSourceStrings are the strings in the grid populated by the DataPoints editor of the ListSource. For multi-valued series you first define XCount and YCount in the object inspector, then open the Datapoints editor to fill the data.

I doubt whether the line

  if (FSource.XCount = 1) and (FSource.YCount + 3 < Cardinal(parts.Count)) then // ????? what about XCount <> 1 ?
    FSource.YCount := parts.Count - 3;

really is needed. It will be removed when there is some hint that it causes misinterpretation of the DataPoints strings. (I refrain from doing unnecessary changes).

---------------

Notify after SetXCount (and SetYCount)?

I tend to keep it as it is because this happens usually before any data are available - so, nothing to update. Of course, somebody could play some risky games here like changing XCount/YCount on the fly...

Marcin Wiazowski

2019-02-19 01:59

reporter   ~0114256

Thanks for your last, hard work.



> Chart sources and series are independent, and my gut feeling tells me that a series should not be allowed to tailor a source to its needs. [...]

Same standpoint here.



> The only occasion where I would accept this behavior is in relation to the built-in listsource which is owned by the series, and there is no (well, little) risk to interfere with another series.

Neither the current code, nor the test code that I attached, modifies series source's XCount or YCount - of course with the exception of built-in sources, that must be initialized to be usable.

In the test code, FXCountMin and FYCountMin are used ONLY to guarantee, that THE BUILT-IN source will not be destroyed by decreasing its XCount or YCount. External source is NEVER modified - when external source's XCount or YCount become too low, series reverts from the external source to its built-in source (which, in this case, is guaranteed to be valid).



> XCount and YCount cannot be zero

Please take a look at the TDbChartSource.Create():

  constructor TDbChartSource.Create(AOwner: TComponent);
  begin
    inherited Create(AOwner);
    ...
    FYCount := 0; // Set to 1 by inherited. <==== YCount is set to 0 here
  end;



> And having no x and y data does not make any sense for plotting.

Not quite sure - even for XCount = YCount = 0 there are still colors and labels - this may be usable (in other words - there is no need to artificially introduce a limitation that XCount and/or YCont must not be 0).

Marcin Wiazowski

2019-02-19 02:15

reporter   ~0114257

constructor TRandomChartSource.Create(AOwner: TComponent);
begin
  inherited Create(AOwner);
  ...
  FYCount := 1; <==== REDUNDANT - CAN BE REMOVED
  ...
end;

Marcin Wiazowski

2019-02-19 03:45

reporter   ~0114258

Ok, I made some mistake in the attached patch - the following lines should be removed:

  if ASource is TListChartSource then
  begin
    FXCountMin := Max(FXCountMin, TListChartSource(ASource).FXCountMin);
    FYCountMin := Max(FYCountMin, TListChartSource(ASource).FYCountMin);
  end;


After removing these lines, external data source will never be modified, in any way. FXCountMin and FYCountMin will be - and remain - set only for built-in sources - for all other sources they will be always set to zero, thus inactive.

wp

2019-02-19 23:27

developer   ~0114274

> In the test code, FXCountMin and FYCountMin are used ONLY to guarantee, that
> THE BUILT-IN source will not be destroyed by decreasing its XCount or YCount.

Why should the built-in source be destroyed? It is only destroyed when its owning series is destroyed (unless the user does it intentionally...)

Please provide a project in which the *current* version of TAChart still crashes the IDE due to incorrect usage of XCount/YCount.

----------

> Please take a look at the TDbChartSource.Create():
>
> constructor TDbChartSource.Create(AOwner: TComponent);
> begin
> inherited Create(AOwner);
> ...
> FYCount := 0; // Set to 1 by inherited. <==== YCount is set to 0 here
> end;

This source is a bit different because it has a stringlist FFieldYList with the names of the y fields. YCount is set to the length of this list in SetFieldY later. Only when the user does not specify y field name(s) in the property FieldY the YCount remains 0 as defined here in the constructor. But this state does not leave the component because the ChartDataItem available in the Items[] property is set up in the GetDataItem method where the case FYCount=0 is handled by assigning default data. Therefore, the series will always get a data record from the db source even if FYCount = 0.

BTW, TDbChartSource has not yet been updated to support multiple x values. Maybe I should do this now.

Marcin Wiazowski

2019-02-20 03:15

reporter   ~0114280

> Why should the built-in source be destroyed? It is only destroyed when its owning series is destroyed (unless the user does it intentionally...)

Apparently, I wasn't clear enough. By using a "destroyed" term, I was not thinking about freeing the source (by calling its destructor), but about damaging it by decreasing XCount and/or YCount in such way, that the source's client (i.e. series) starts to use out-of-bounds accesses.




> Please provide a project in which the *current* version of TAChart still crashes the IDE due to incorrect usage of XCount/YCount.

Here you are: I rebuilt IDE by using TAChart from r60463. Load Reproduce2 project and:

1) Side note first: there is a bug in loading ListChartSource - in Unit1.lfm there is:

  object ListChartSource: TListChartSource
    DataPoints.Strings = (
      '1|1|2|2|?|'
      '3|3|4|4|?|'
    )
    XCount = 2
    YCount = 2
  end

  but data points are loaded as:

    '1|0|1|2|?|'
    '3|0|3|4|?|'

  (ignore this problem now and just go to the next step)

2) ListChartSource has XCount = 2 and YCount = 2, and it is assigned to Chart1FieldSeries.Source initially. Now select ListChartSource in Object Inspector and set its XCount to 1 and YCount to 1.

3) Minimize the edited form and restore it again - so form (and thus the chart) will be repainted. IDE will most probably not crash immediately, but the whole form will become black - which is a first symptom of invalid memory accesses (see Reproduce2a.png).

4) Save the project and close Lazarus IDE. An exception will probably occur when exiting: "TFieldSeries requires a chart source with at least 2 x value(s) per data point." - close the message.

5) Load Lazarus IDE again and load the project. Form will not be loaded properly, and Unit1.lfm will be opened in the editor - probably three times. Try to close these Unit1.lfm's in the editor (Ctrl+F4) - when closing second of them, "List index (-1) out of bounds." exception will most probably be generated (see Reproduce2b.png). Then the IDE will most probably hang. This leads us to a conclusion, that - only by using IDE - we damaged the project.




The cause is: currently, we CANNOT assign ListChartSource having XCount < 2 or YCount < 2 to TFieldSeries - but we CAN first assign ListChartSource having XCount = 2 and YCount = 2 and *LATER* decrease ListChartSource's XCount and/or YCount in the Object Inspector.

Solution: apply patch2.diff. It will:

A) add Notify() calls to TListChartSource.SetXCount() and SetYCount() - so series using the source will be notified about the change and will have a possibility of recovering in case of too low new source's XCount and/or YCount - i.e. in this case series will revert to its built-in source (that is guaranteed to be valid).

B) guarantee, that the built-in source is always valid, i.e. cannot be "damaged" by setting its XCount and/or YCount to values smaller than allowed - this is internally achieved by using TListChartSource.FXCountMin and TListChartSource.FYCountMin. Although nobody will intentionally change built-in source's XCount and/or YCount, this may be done unintentionally, by calling SomeSeriesInstance.Source.XCount := ..., when the user thinks that SomeSeriesInstance.Source points to external source, but it in fact points to the built-in source. Or even more indirectly, by calling SomeSeriesInstance.ListSource.CopyFrom(SomeListChartSource).

C) cosmetic change: exception message has been changed from '2 x value(s)' to '2 "X" value(s)', to make it (once again) more clear.




> There's a problem: Chart sources and series are independent, and my gut feeling tells me that a series should not be allowed to tailor a source to its needs.

This is an additional bonus of patch2.diff: built-in source will be *GUARANTEED* to have sufficient XCount and YCount values - so no checks/modifications will be needed. External source will either have its XCount and YCount values sufficient, or a change in the external source's XCount and YCount values will be immediately detected and - in case of too low values - the series will revert to its built-in source - which is, as already said, *GUARANTEED* to have sufficient XCount and YCount values. So, once again, no checks for/modifications of source's XCount and YCount will be needed.

Some verification code has been removed recently:

  - in function TBubbleSeries.AddXY: "if ListSource.YCount < 2 then ListSource.YCount := 2;"

  - in function TBoxAndWhiskerSeries.AddXY: "if ListSource.YCount < 5 then ListSource.YCount := 5;"

  - in function TOpenHighLowCloseSeries.AddXOHLC: "if ListSource.YCount < 4 then ListSource.YCount := 4;"

Removing this code prevented modifying the external data source, but also opened a possibility of a runtime crash; applying the patch will make this (already nonexistent) code needless, but will also prevent from crashes.




I just applied patch2.diff and rebuilt Lazarus IDE - everything works properly now. In step 2) above, when changing ListChartSource's XCount or YCount to 1, IDE just informs that this source can no longer be used by TFieldSeries, and Chart1FieldSeries' Source is reverted to Chart1FieldSeries.Builtin. In step 3), the form does NOT become black. In step 4), project is NOT damaged. Loading project in step 5) does NOT cause any problems.

Marcin Wiazowski

2019-02-20 03:16

reporter  

Reproduce2a.png (4,299 bytes)
Reproduce2a.png (4,299 bytes)

Marcin Wiazowski

2019-02-20 03:16

reporter  

Reproduce2b.png (14,056 bytes)
Reproduce2b.png (14,056 bytes)

Marcin Wiazowski

2019-02-20 03:16

reporter  

Reproduce2.zip (3,128 bytes)

Marcin Wiazowski

2019-02-20 03:17

reporter  

patch2.diff (4,789 bytes)
Index: components/tachart/tachartstrconsts.pas
===================================================================
--- components/tachart/tachartstrconsts.pas	(revision 60463)
+++ components/tachart/tachartstrconsts.pas	(working copy)
@@ -69,7 +69,8 @@
   rsDistanceMeasurement = 'Distance measurement';
 
   // Chart sources
-  rsSourceCountError = '%0:s requires a chart source with at least %1:d %2:s value(s) per data point.';
+  rsSourceCountError = '%0:s requires a chart source with at least %1:d "%2:s" value(s) per data point.';
+  rsSourceCountError2 = 'This %0:s instance must have at least %1:d "%2:s" value(s) per data point.';
 
   // Transformations
   tasAxisTransformsEditorTitle = 'Edit axis transformations';
Index: components/tachart/tacustomseries.pas
===================================================================
--- components/tachart/tacustomseries.pas	(revision 60463)
+++ components/tachart/tacustomseries.pas	(working copy)
@@ -788,9 +788,9 @@
     exit;
   GetXYCountNeeded(nx, ny);
   if ASource.XCount < nx then
-    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'x'])
-  else if ASource.YCount < ny then
-    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'y']);
+    raise EXCountError.CreateFmt(rsSourceCountError, [ClassName, nx, 'X']);
+  if ASource.YCount < ny then
+    raise EYCountError.CreateFmt(rsSourceCountError, [ClassName, ny, 'Y']);
 end;
 
 procedure TChartSeries.Clear;
@@ -817,11 +817,9 @@
   inherited Create(AOwner);
 
   FListener := TListener.Create(@FSource,  @SourceChanged);
-  FBuiltinSource := TListChartSource.Create(Self);
+  GetXYCountNeeded(nx, ny);
+  FBuiltinSource := TListChartSource.Create(Self, nx, ny);
   FBuiltinSource.Name := BUILTIN_SOURCE_NAME;
-  GetXYCountNeeded(nx, ny);
-  FBuiltinSource.XCount := nx;
-  FBuiltinSource.YCount := ny;
   FBuiltinSource.Broadcaster.Subscribe(FListener);
   FMarks := TChartMarks.Create(FChart);
   FStylesListener := TListener.Create(@FStyles,  @StyleChanged);
@@ -1125,7 +1123,12 @@
 procedure TChartSeries.SourceChanged(ASender: TObject);
 begin
   if ASender is TCustomChartSource then
+  try
     CheckSource(TCustomChartSource(ASender));
+  except
+    Source := nil; // revert to built-in source
+    raise;
+  end;
   StyleChanged(ASender);
 end;
 
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60463)
+++ components/tachart/tasources.pas	(working copy)
@@ -23,6 +23,8 @@
 
   TListChartSource = class(TCustomChartSource)
   private
+    FXCountMin: Cardinal;
+    FYCountMin: Cardinal;
     FData: TFPList;
     FDataPoints: TStrings;
     FSorted: Boolean;
@@ -44,7 +46,8 @@
       EXListEmptyError = class(EChartError);
       EYListEmptyError = class(EChartError);
   public
-    constructor Create(AOwner: TComponent); override;
+    constructor Create(AOwner: TComponent); override; overload;
+    constructor Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal); overload;
     destructor Destroy; override;
   public
     function Add(
@@ -247,7 +250,7 @@
 implementation
 
 uses
-  Math, StrUtils, SysUtils, TAMath;
+  Math, StrUtils, SysUtils, TAMath, TAChartStrConsts;
 
 type
 
@@ -467,6 +470,11 @@
 var
   i: Integer;
 begin
+  if ASource.XCount < FXCountMin then
+    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
+  if ASource.YCount < FYCountMin then
+    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
+
   BeginUpdate;
   try
     Clear;
@@ -492,6 +500,17 @@
   ClearCaches;
 end;
 
+constructor TListChartSource.Create(AOwner: TComponent; AXCountMin, AYCountMin: Cardinal);
+begin
+  Create(AOwner);
+  FXCountMin := AXCountMin;
+  FYCountMin := AYCountMin;
+  if FXCount < FXCountMin then
+    FXCount := FXCountMin;
+  if FYCount < FYCountMin then
+    FYCount := FYCountMin;
+end;
+
 procedure TListChartSource.Delete(AIndex: Integer);
 begin
   with Item[AIndex]^ do begin
@@ -580,10 +599,13 @@
 var
   i: Integer;
 begin
+  if AValue < FXCountMin then
+    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
   if AValue = FXCount then exit;
   FXCount := AValue;
   for i := 0 to Count - 1 do
     SetLength(Item[i]^.XList, Max(FXCount - 1, 0));
+  Notify;
 end;
 
 procedure TListChartSource.SetXList(
@@ -654,10 +676,13 @@
 var
   i: Integer;
 begin
+  if AValue < FYCountMin then
+    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
   if AValue = FYCount then exit;
   FYCount := AValue;
   for i := 0 to Count - 1 do
     SetLength(Item[i]^.YList, Max(FYCount - 1, 0));
+  Notify;
 end;
 
 procedure TListChartSource.SetYList(
patch2.diff (4,789 bytes)

wp

2019-02-20 10:04

developer   ~0114288

Last edited: 2019-02-20 10:19

View 3 revisions

Thanks. Due to clear steps to reproduce I finally understood your point and your patch. Applied in r60464.

There is a problem, though. When I tried to reproduce the issue before the patch I somehow ended with a project which still crashes now. I cannot reproduce the steps, but the effect is the same as when I edit the lfm file of your demo project "reproduce2.zip" and set the XCount and YCount to 1 (instead of 2). Obviously the streaming mechanism may lead to a situation where your patch is ignored. Of course, editing the lfm file is something for the tough guys only, but it may be indicative of a remaining issue somewhere.

wp

2019-02-20 19:58

developer   ~0114309

> Side note first: there is a bug in loading ListChartSource - in Unit1.lfm there is:
>
> object ListChartSource: TListChartSource
> DataPoints.Strings = (
> '1|1|2|2|?|'
> '3|3|4|4|?|'
> )
> XCount = 2
> YCount = 2
> end
>
> but data points are loaded as:
>
> '1|0|1|2|?|'
> '3|0|3|4|?|'

Looks like that, when the DataPoint.Strings are read from the LFM, XCount and YCount are still 1. The line that you mentioned above

  if (* (FSource.XCount = 1) and *) (FSource.YCount + 3 < Cardinal(parts.Count)) then
    FSource.YCount := parts.Count - 3;

(the first part, FSource.XCount = 1, here commented, is an addition by myself when I added multi-X support) just takes care of this: it calculates YCount from the items in the DataPointStrings lines - but when XCount > 1 this is no longer possible... No idea, yet.

Marcin Wiazowski

2019-02-20 20:44

reporter   ~0114310

> When I tried to reproduce the issue before the patch I somehow ended with a project which still crashes now.

Before applying the patch, it was possible to generate a crashing form - just by performing the steps 1) .. 4) above.



> I cannot reproduce the steps

This is because now - i.e. after applying the patch - such a crashing form cannot be created - by using IDE - anymore.



> but the effect is the same as when I edit the lfm file of your demo project "reproduce2.zip" and set the XCount and YCount to 1 (instead of 2).

Such a crashing form can still be created manually, by editing the LFM file - this cannot be avoided.



> Obviously the streaming mechanism may lead to a situation where your patch is ignored.

Before the patch, the streaming machanism just loaded the form with an invalid series' source, so many problems occured later (exceptions, hangs).

Currently, the patch is not ignored:
I) you cannot create a crashing form by using IDE anymore
II) you can still create a crashing form manually, by editing the LFM file
III) you can still load a project with crashing form, that has been created BEFORE applying the patch - this is in fact same as II)

but, currently, a crashing form from II) and III) will no longer cause crashes/hangs in IDE - so it shouldn't be called "crashing" anymore - maybe an invalid form. Please note, that - after applying the patch - loading an invalid form in IDE no longer causes crashes / hangs / "List index (-1) out of bounds." exceptions - just a single message "TFieldSeries requires a chart source with at least 2 x value(s) per data point" is shown, and IDE is then still stable and working properly.



So I don't think that there is anything more that should be done - please note, that you can damage the LFM in many ways, for example:

  Pen.Color = clBlue
=>
  Pen.Color = aaa

  Marks.LabelBrush.Style = bsClear
=>
  Marks.LabelBrush.AAA = bsClear

  Source = ListChartSource
=>
  Source = Chart1

In all these cases, IDE just displays an error message and remains stable - and this is same as it is now for an LFM file with invalid series' source.



So, as for me, the problem is fixed.

Marcin Wiazowski

2019-02-20 20:45

reporter   ~0114311

Since series' source is now guaranteed to be valid, so I'm attaching patch3.diff, that removes the obsolete checking/initialization code.



It also patches a bug in TBoxAndWhiskerSeries.Draw():

there is:
  for i := 0 to ny-1 do DrawLabels(ADrawer, i)

there should be:
  for i := 0 to ny-1 do DrawLabels(ADrawer, ny)



It also changes uppercase to lowercase in the exception messages in TListChartSource.SetXCount and SetXCount, as you prefer.

Marcin Wiazowski

2019-02-20 20:45

reporter  

patch3.diff (3,956 bytes)
Index: components/tachart/tamultiseries.pas
===================================================================
--- components/tachart/tamultiseries.pas	(revision 60470)
+++ components/tachart/tamultiseries.pas	(working copy)
@@ -514,11 +514,7 @@
   irect: TRect;
   dummyR: TRect = (Left:0; Top:0; Right:0; Bottom:0);
   ext: TDoubleRect;
-  nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if Source.YCount < ny then exit;
-
   ADrawer.Pen := BubblePen;
   ADrawer.Brush := BubbleBrush;
 
@@ -557,9 +553,6 @@
   item: PChartDataItem;
 begin
   Result := EmptyExtent;
-  if Source.YCount < 2 then
-    exit;
-
   for i := 0 to Count - 1 do begin
     item := Source[i];
     sp := item^.Point;
@@ -982,8 +975,7 @@
   i: Integer;
   nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if IsEmpty or (Source.YCount < ny) then
+  if IsEmpty then
     exit;
   if FWidthStyle = bwsPercentMin then
     UpdateMinXRange;
@@ -1029,8 +1021,9 @@
     DoLine(x - wb, ymed, x + wb, ymed);
   end;
 
+  GetXYCountNeeded(nx, ny);
   if Source.YCount > ny then
-    for i := 0 to ny-1 do DrawLabels(ADrawer, ny)
+    for i := 0 to ny-1 do DrawLabels(ADrawer, i)
   else
     DrawLabels(ADrawer);
 end;
@@ -1046,7 +1039,6 @@
   end;
 
 begin
-  if Source.YCount < 5 then exit(EmptyExtent);
   Result := Source.ExtentList;
   // Show first and last boxes fully.
   j := -1;
@@ -1431,9 +1423,6 @@
   p: TPen;
   nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
-
   my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
   if IsEmpty or (my >= Source.YCount) then exit;
 
@@ -1474,6 +1463,7 @@
     end;
   end;
 
+  GetXYCountNeeded(nx, ny);
   if Source.YCount > ny then
     for i := 0 to ny-1 do DrawLabels(ADrawer, i)
   else
@@ -1486,7 +1476,6 @@
   tw: Double;
   j: Integer;
 begin
-  if Source.YCount < 4 then exit(EmptyExtent);
   Result := Source.ExtentList;                            // axis units
   // Show first and last open/close ticks and candle boxes fully.
   j := -1;
@@ -1764,8 +1753,6 @@
 begin
   inherited Create(AOwner);
   ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
-  ListSource.XCount := 2;
-  ListSource.YCount := 2;
   FArrow := TChartArrow.Create(ParentChart);
   FArrow.Length := 20;
   FArrow.Width := 10;
@@ -1810,11 +1797,7 @@
   i: Integer;
   p1, p2: TDoublePoint;
   lPen: TPen;
-  nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
-
   with Extent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60470)
+++ components/tachart/tasources.pas	(working copy)
@@ -75,8 +75,8 @@
     property Sorted: Boolean read FSorted write SetSorted default false;
     property XCount;
     property XErrorBarData;
+    property YCount;
     property YErrorBarData;
-    property YCount;
   end;
 
   { TMWCRandomGenerator }
@@ -599,7 +599,7 @@
   i: Integer;
 begin
   if AValue < FXCountMin then
-    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
+    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'x']);
   if AValue = FXCount then exit;
   FXCount := AValue;
   for i := 0 to Count - 1 do
@@ -676,7 +676,7 @@
   i: Integer;
 begin
   if AValue < FYCountMin then
-    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
+    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'y']);
   if AValue = FYCount then exit;
   FYCount := AValue;
   for i := 0 to Count - 1 do
@@ -832,7 +832,6 @@
   inherited Create(AOwner);
   FCurItem.Color := clTAColor;
   FRNG := TMWCRandomGenerator.Create;
-  FYCount := 1;
   RandSeed := Trunc(Frac(Now) * MaxInt);
 end;
 
patch3.diff (3,956 bytes)

Marcin Wiazowski

2019-02-20 20:54

reporter  

patch3_updated.diff (4,175 bytes)
Index: components/tachart/tamultiseries.pas
===================================================================
--- components/tachart/tamultiseries.pas	(revision 60470)
+++ components/tachart/tamultiseries.pas	(working copy)
@@ -516,9 +516,6 @@
   ext: TDoubleRect;
   nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if Source.YCount < ny then exit;
-
   ADrawer.Pen := BubblePen;
   ADrawer.Brush := BubbleBrush;
 
@@ -540,6 +537,7 @@
       ADrawer.SetBrushColor(ColorDef(item^.Color, BubbleBrush.Color));
     ADrawer.Ellipse(irect.Left, irect.Top, irect.Right, irect.Bottom);
   end;
+  GetXYCountNeeded(nx, ny);
   if Source.YCount > ny then
     for i := 0 to ny - 1 do DrawLabels(ADrawer, i)
   else
@@ -557,9 +555,6 @@
   item: PChartDataItem;
 begin
   Result := EmptyExtent;
-  if Source.YCount < 2 then
-    exit;
-
   for i := 0 to Count - 1 do begin
     item := Source[i];
     sp := item^.Point;
@@ -982,8 +977,7 @@
   i: Integer;
   nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if IsEmpty or (Source.YCount < ny) then
+  if IsEmpty then
     exit;
   if FWidthStyle = bwsPercentMin then
     UpdateMinXRange;
@@ -1029,8 +1023,9 @@
     DoLine(x - wb, ymed, x + wb, ymed);
   end;
 
+  GetXYCountNeeded(nx, ny);
   if Source.YCount > ny then
-    for i := 0 to ny-1 do DrawLabels(ADrawer, ny)
+    for i := 0 to ny-1 do DrawLabels(ADrawer, i)
   else
     DrawLabels(ADrawer);
 end;
@@ -1046,7 +1041,6 @@
   end;
 
 begin
-  if Source.YCount < 5 then exit(EmptyExtent);
   Result := Source.ExtentList;
   // Show first and last boxes fully.
   j := -1;
@@ -1431,9 +1425,6 @@
   p: TPen;
   nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
-
   my := MaxIntValue([YIndexOpen, YIndexHigh, YIndexLow, YIndexClose]);
   if IsEmpty or (my >= Source.YCount) then exit;
 
@@ -1474,6 +1465,7 @@
     end;
   end;
 
+  GetXYCountNeeded(nx, ny);
   if Source.YCount > ny then
     for i := 0 to ny-1 do DrawLabels(ADrawer, i)
   else
@@ -1486,7 +1478,6 @@
   tw: Double;
   j: Integer;
 begin
-  if Source.YCount < 4 then exit(EmptyExtent);
   Result := Source.ExtentList;                            // axis units
   // Show first and last open/close ticks and candle boxes fully.
   j := -1;
@@ -1764,8 +1755,6 @@
 begin
   inherited Create(AOwner);
   ToolTargets := [nptPoint, nptXList, nptYList, nptCustom];
-  ListSource.XCount := 2;
-  ListSource.YCount := 2;
   FArrow := TChartArrow.Create(ParentChart);
   FArrow.Length := 20;
   FArrow.Width := 10;
@@ -1810,11 +1799,7 @@
   i: Integer;
   p1, p2: TDoublePoint;
   lPen: TPen;
-  nx, ny: Cardinal;
 begin
-  GetXYCountNeeded(nx, ny);
-  if (Source.XCount < nx) or (Source.YCount < ny) then exit;
-
   with Extent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60470)
+++ components/tachart/tasources.pas	(working copy)
@@ -75,8 +75,8 @@
     property Sorted: Boolean read FSorted write SetSorted default false;
     property XCount;
     property XErrorBarData;
+    property YCount;
     property YErrorBarData;
-    property YCount;
   end;
 
   { TMWCRandomGenerator }
@@ -599,7 +599,7 @@
   i: Integer;
 begin
   if AValue < FXCountMin then
-    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'X']);
+    raise EXCountError.CreateFmt(rsSourceCountError2, [ClassName, FXCountMin, 'x']);
   if AValue = FXCount then exit;
   FXCount := AValue;
   for i := 0 to Count - 1 do
@@ -676,7 +676,7 @@
   i: Integer;
 begin
   if AValue < FYCountMin then
-    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'Y']);
+    raise EYCountError.CreateFmt(rsSourceCountError2, [ClassName, FYCountMin, 'y']);
   if AValue = FYCount then exit;
   FYCount := AValue;
   for i := 0 to Count - 1 do
@@ -832,7 +832,6 @@
   inherited Create(AOwner);
   FCurItem.Color := clTAColor;
   FRNG := TMWCRandomGenerator.Create;
-  FYCount := 1;
   RandSeed := Trunc(Frac(Now) * MaxInt);
 end;
 
patch3_updated.diff (4,175 bytes)

Marcin Wiazowski

2019-02-20 20:55

reporter   ~0114312

I made some mistake in patch3.diff.

Please ignore it and use patch3_updated.diff instead.

Marcin Wiazowski

2019-02-20 21:42

reporter   ~0114313

I know a solution for the problem with loading TListChartSource from LFM.

I'll post it here.

wp

2019-02-20 23:03

developer   ~0114314

> So, as for me, the problem is fixed.

OK, then I mark the report as resolved.

As for the DataPoints issue of the ListSource, I open a separate report. If you happen to find a solution attach it there.

Marcin Wiazowski

2019-02-21 02:37

reporter   ~0114317

I can confirm, that all the problems described above are fixed now - with an exception of the problem with loading TListChartSource from LFM, which has been moved to 0035125.

Marcin Wiazowski

2019-02-21 13:01

reporter   ~0114328

I found some issues in the last modified code:



1) There is:

procedure TChartSeries.SetSource(AValue: TCustomChartSource);
begin
  if FSource = AValue then exit;
  if FListener.IsListening then
    Source.Broadcaster.Unsubscribe(FListener); <==== old source unsubscribed
  CheckSource(AValue); <==== but old source will remain in use in case of exception here!
  FSource := AValue;
  Source.Broadcaster.Subscribe(FListener);
  SourceChanged(Self);
end;

New source should be checked BEFORE making any changes to the old source, so the code should be changed to:

procedure TChartSeries.SetSource(AValue: TCustomChartSource);
begin
  if FSource = AValue then exit;
  CheckSource(AValue); <==== MOVED HERE
  if FListener.IsListening then
    Source.Broadcaster.Unsubscribe(FListener);
  FSource := AValue;
  ...



2) Let's imagine a piece of code:

procedure Test;
var
  OrigSource: TCustomChartSource;
begin
  OrigSource := SomeSeries.Source;
  try
    SomeSeries.Source := ListChartSource1;

    ... some operations here ...
  finally
    SomeSeries.Source := OrigSource;
  end;
end;

OrigSource is initialized to SomeSeries.FBuiltinSource. When we finally want to return to previous state, we in fact set an external source to FBuiltinSource. This confuses LFM streaming machinery - TChartSeries.IsSourceStored starts to return an invalid result.

Solution is trivial:

procedure TChartSeries.SetSource(AValue: TCustomChartSource);
begin
  if AValue = FBuiltinSource then <==== ADDED
    AValue := nil;
  if FSource = AValue then exit;
  ...



3) Optimization: built-in source is guaranteed to be valid. So the code:

procedure TChartSeries.SourceChanged(ASender: TObject);
begin
  if ASender is TCustomChartSource then
    try
      CheckSource(TCustomChartSource(ASender));
    ...

can be changed to:

procedure TChartSeries.SourceChanged(ASender: TObject);
begin
  if (ASender <> FBuiltinSource) and (ASender is TCustomChartSource) then
    try
      CheckSource(TCustomChartSource(ASender));
    ...

This avoids executing both the "is" operator (it's time-consuming) and also the "CheckSource" call.




The attached patch4.diff solves these problems.

Marcin Wiazowski

2019-02-21 13:01

reporter  

patch4.diff (958 bytes)
Index: components/tachart/tacustomseries.pas
===================================================================
--- components/tachart/tacustomseries.pas	(revision 60471)
+++ components/tachart/tacustomseries.pas	(working copy)
@@ -1069,10 +1069,12 @@
 
 procedure TChartSeries.SetSource(AValue: TCustomChartSource);
 begin
+  if AValue = FBuiltinSource then
+    AValue := nil;
   if FSource = AValue then exit;
+  CheckSource(AValue);
   if FListener.IsListening then
     Source.Broadcaster.Unsubscribe(FListener);
-  CheckSource(AValue);
   FSource := AValue;
   Source.Broadcaster.Subscribe(FListener);
   SourceChanged(Self);
@@ -1122,7 +1124,7 @@
 
 procedure TChartSeries.SourceChanged(ASender: TObject);
 begin
-  if ASender is TCustomChartSource then
+  if (ASender <> FBuiltinSource) and (ASender is TCustomChartSource) then // built-in source is guaranteed to be valid
     try
       CheckSource(TCustomChartSource(ASender));
     except
patch4.diff (958 bytes)

Marcin Wiazowski

2019-02-24 23:53

reporter   ~0114391

Fix confirmed, thanks!

Issue History

Date Modified Username Field Change
2019-02-15 02:32 Marcin Wiazowski New Issue
2019-02-15 02:32 Marcin Wiazowski File Added: Reproduce.zip
2019-02-15 09:26 wp Assigned To => wp
2019-02-15 09:26 wp Status new => assigned
2019-02-15 14:39 Marcin Wiazowski Note Added: 0114147
2019-02-15 16:22 wp Fixed in Revision => 60425, 60427
2019-02-15 16:22 wp LazTarget => 2.2
2019-02-15 16:22 wp Note Added: 0114153
2019-02-15 16:22 wp Status assigned => resolved
2019-02-15 16:22 wp Resolution open => fixed
2019-02-15 16:22 wp Target Version => 2.2
2019-02-18 01:28 Marcin Wiazowski Note Added: 0114234
2019-02-18 01:28 Marcin Wiazowski Status resolved => assigned
2019-02-18 01:28 Marcin Wiazowski Resolution fixed => reopened
2019-02-18 01:28 Marcin Wiazowski File Added: test.diff
2019-02-19 00:54 wp Note Added: 0114254
2019-02-19 00:54 wp Status assigned => resolved
2019-02-19 00:54 wp Resolution reopened => fixed
2019-02-19 01:59 Marcin Wiazowski Note Added: 0114256
2019-02-19 01:59 Marcin Wiazowski Status resolved => assigned
2019-02-19 01:59 Marcin Wiazowski Resolution fixed => reopened
2019-02-19 02:15 Marcin Wiazowski Note Added: 0114257
2019-02-19 03:45 Marcin Wiazowski Note Added: 0114258
2019-02-19 23:27 wp Note Added: 0114274
2019-02-20 03:15 Marcin Wiazowski Note Added: 0114280
2019-02-20 03:16 Marcin Wiazowski File Added: Reproduce2a.png
2019-02-20 03:16 Marcin Wiazowski File Added: Reproduce2b.png
2019-02-20 03:16 Marcin Wiazowski File Added: Reproduce2.zip
2019-02-20 03:17 Marcin Wiazowski File Added: patch2.diff
2019-02-20 10:04 wp Note Added: 0114288
2019-02-20 10:04 wp Fixed in Revision 60425, 60427 => 60425, 60427, 60464
2019-02-20 10:04 wp Status assigned => resolved
2019-02-20 10:04 wp Resolution reopened => fixed
2019-02-20 10:05 wp Status resolved => feedback
2019-02-20 10:17 wp Note Edited: 0114288 View Revisions
2019-02-20 10:19 wp Note Edited: 0114288 View Revisions
2019-02-20 19:58 wp Note Added: 0114309
2019-02-20 20:44 Marcin Wiazowski Note Added: 0114310
2019-02-20 20:44 Marcin Wiazowski Status feedback => assigned
2019-02-20 20:45 Marcin Wiazowski Note Added: 0114311
2019-02-20 20:45 Marcin Wiazowski File Added: patch3.diff
2019-02-20 20:54 Marcin Wiazowski File Added: patch3_updated.diff
2019-02-20 20:55 Marcin Wiazowski Note Added: 0114312
2019-02-20 21:42 Marcin Wiazowski Note Added: 0114313
2019-02-20 23:03 wp Fixed in Revision 60425, 60427, 60464 => 60425, 60427, 60464, 60471
2019-02-20 23:03 wp Note Added: 0114314
2019-02-20 23:03 wp Status assigned => resolved
2019-02-21 02:37 Marcin Wiazowski Note Added: 0114317
2019-02-21 03:53 Marcin Wiazowski Status resolved => closed
2019-02-21 13:01 Marcin Wiazowski Note Added: 0114328
2019-02-21 13:01 Marcin Wiazowski Status closed => assigned
2019-02-21 13:01 Marcin Wiazowski Resolution fixed => reopened
2019-02-21 13:01 Marcin Wiazowski File Added: patch4.diff
2019-02-24 23:12 wp Fixed in Revision 60425, 60427, 60464, 60471 => 60425, 60427, 60464, 60471, 60491
2019-02-24 23:12 wp Status assigned => resolved
2019-02-24 23:12 wp Resolution reopened => fixed
2019-02-24 23:53 Marcin Wiazowski Note Added: 0114391
2019-02-24 23:53 Marcin Wiazowski Status resolved => closed
2019-06-02 11:55 wp Relationship added related to 0035665
2019-06-12 14:23 wp Relationship added related to 0035705
2019-06-12 14:24 wp Relationship deleted related to 0035705