View Issue Details

IDProjectCategoryView StatusLast Update
0035183LazarusTAChartpublic2019-03-06 17:58
ReporterMarcin WiazowskiAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN)Product Build60570 
Target Version2.2Fixed in Version 
Summary0035183: TAChart: extent in TFitSeries is not calculated properly
DescriptionThe extent returned for TFitSeries is:

  function TFitSeries.Extent: TDoubleRect;
  begin
    Result := Source.BasicExtent;
  end;

so it's only based on data points - it doesn't take into account a shape of the curve.



I made some experiments, so I'm attaching a test application and experiment.diff.

Important: after applying the patch and launching the test application, curves are still not drawn fully properly - it is needed to minimize and restore the application, so charts are repainted.

I don't know if the attached code works properly with the DrawFitRangeOnly / FitRange functionality.
TagsNo tags attached.
Fixed in Revision60586, 60597, 60598
LazTarget2.2
WidgetsetWin32/Win64
Attached Files
  • Reproduce.zip (2,632 bytes)
  • Reproduce.png (22,822 bytes)
    Reproduce.png (22,822 bytes)
  • experiment.diff (1,536 bytes)
    Index: components/tachart/tafuncseries.pas
    ===================================================================
    --- components/tachart/tafuncseries.pas	(revision 60570)
    +++ components/tachart/tafuncseries.pas	(working copy)
    @@ -305,6 +305,7 @@
         FErrCode: TFitErrCode;
         FFitStatistics: TFitStatistics;
         FConfidenceLevel: Double;
    +    FExtentNestedCount: Integer;
         function GetParam(AIndex: Integer): Double;
         function GetParamCount: Integer;
         function GetParamError(AIndex: Integer): Double;
    @@ -1793,8 +1794,38 @@
     end;
     
     function TFitSeries.Extent: TDoubleRect;
    +var
    +  de : TIntervalList;
    +  ymin, ymax: Double;
     begin
       Result := Source.BasicExtent;
    +
    +  if IsEmpty or (not Active) then exit;
    +  if (Result.a.X = EmptyExtent.a.X) or (Result.b.X = EmptyExtent.b.X) then exit;
    +  if FExtentNestedCount <> 0 then exit;
    +
    +  if (FState = fpsValid) and (FErrCode = fitOK) then begin
    +    Inc(FExtentNestedCount);
    +    try
    +      de := PrepareIntervals;
    +      try
    +        ymin := SafeInfinity;
    +        ymax := NegInfinity;
    +        with TDrawFuncHelper.Create(Self, de, @Calculate, Step) do
    +          try
    +            CalcAxisExtentY(Result.a.X, Result.b.X, ymin, ymax);
    +            UpdateMinMax(ymin, Result.a.Y, Result.b.Y);
    +            UpdateMinMax(ymax, Result.a.Y, Result.b.Y);
    +          finally
    +            Free;
    +          end;
    +      finally
    +        de.Free;
    +      end;
    +    finally
    +      Dec(FExtentNestedCount);
    +    end;
    +  end;
     end;
     
     function TFitSeries.FitParams: TDoubleDynArray;
    
    experiment.diff (1,536 bytes)
  • test.diff (4,910 bytes)
    Index: components/tachart/tafuncseries.pas
    ===================================================================
    --- components/tachart/tafuncseries.pas	(revision 60574)
    +++ components/tachart/tafuncseries.pas	(working copy)
    @@ -1601,7 +1601,7 @@
     var
       ext: TDoubleRect;
     begin
    -  with Extent do begin
    +  with Source.BasicExtent do begin
         ext.a := AxisToGraph(a);
         ext.b := AxisToGraph(b);
       end;
    @@ -1793,8 +1793,31 @@
     end;
     
     function TFitSeries.Extent: TDoubleRect;
    +var
    +  de : TIntervalList;
     begin
       Result := Source.BasicExtent;
    +  if IsEmpty or (not Active) then exit;
    +
    +  if ParentChart = nil then exit;
    +  ParentChart.ScaleNeedsSecondPass := True;
    +  if not ParentChart.ScaleValid then exit;
    +
    +  if FAutoFit then ExecFit;
    +
    +  if (FState = fpsValid) and (FErrCode = fitOK) then begin
    +    de := PrepareIntervals;
    +    try
    +      with TDrawFuncHelper.Create(Self, de, @Calculate, Step) do
    +        try
    +          CalcAxisExtentY(Result.a.X, Result.b.X, Result.a.Y, Result.b.Y);
    +        finally
    +          Free;
    +        end;
    +    finally
    +      de.Free;
    +    end;
    +  end;
     end;
     
     function TFitSeries.FitParams: TDoubleDynArray;
    Index: components/tachart/tagraph.pas
    ===================================================================
    --- components/tachart/tagraph.pas	(revision 60574)
    +++ components/tachart/tagraph.pas	(working copy)
    @@ -1,4 +1,4 @@
    -{
    +{
      /***************************************************************************
                                    TAGraph.pas
                                    -----------
    @@ -233,6 +233,8 @@
         FOnExtentChanging: TChartEvent;
         FPrevLogicalExtent: TDoubleRect;
         FScale: TDoublePoint;    // Coordinates transformation
    +    FScaleValid: Boolean;
    +    FScaleNeedsSecondPass: Boolean;
         FSavedClipRect: TRect;
         FClipRectLock: Integer;
     
    @@ -359,6 +361,8 @@
         function XImageToGraph(AX: Integer): Double; inline;
         function YGraphToImage(AY: Double): Integer; inline;
         function YImageToGraph(AY: Integer): Double; inline;
    +    property ScaleValid: Boolean read FScaleValid;
    +    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
     
       public
         procedure LockClipRect;
    @@ -591,6 +595,7 @@
       FOffset.Y := rY.CalcOffset(FScale.Y);
       rX.UpdateMinMax(@XImageToGraph);
       rY.UpdateMinMax(@YImageToGraph);
    +  FScaleValid := True;
     end;
     
     procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
    @@ -686,6 +691,8 @@
       FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
     
       FScale := DoublePoint(1, -1);
    +  FScaleValid := False;
    +  FScaleNeedsSecondPass := False;
     
       Width := DEFAULT_CHART_WIDTH;
       Height := DEFAULT_CHART_HEIGHT;
    @@ -878,32 +885,49 @@
     procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
     var
       ldd: TChartLegendDrawingData;
    +  tmpExtent: TDoubleRect;
    +  tries: Integer;
       s: TBasicChartSeries;
       ts: TBasicChartToolset;
     begin
    -  Prepare;
    +  ldd.FItems := nil;
    +  try
    +    Prepare;
     
    -  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
    +    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
     
    -  FClipRect := ARect;
    -  with MarginsExternal do begin
    -    FClipRect.Left += Left;
    -    FClipRect.Top += Top;
    -    FClipRect.Right -= Right;
    -    FClipRect.Bottom -= Bottom;
    -  end;
    +    for tries := 1 to 2 do
    +    begin
    +      FClipRect := ARect;
    +      with MarginsExternal do begin
    +        FClipRect.Left += Left;
    +        FClipRect.Top += Top;
    +        FClipRect.Right -= Right;
    +        FClipRect.Bottom -= Bottom;
    +      end;
     
    -  with ClipRect do begin
    -    FTitle.Measure(ADrawer, 1, Left, Right, Top);
    -    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    -  end;
    +      with ClipRect do begin
    +        FTitle.Measure(ADrawer, 1, Left, Right, Top);
    +        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    +      end;
     
    -  ldd.FItems := nil;
    -  if Legend.Visible then
    -    ldd := PrepareLegend(ADrawer, FClipRect);
    +      if Legend.Visible then
    +        ldd := PrepareLegend(ADrawer, FClipRect);
     
    -  try
    -    PrepareAxis(ADrawer);
    +      PrepareAxis(ADrawer);
    +
    +      if not FScaleNeedsSecondPass then break;
    +
    +      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
    +                               // in the Prepare() call above
    +      tmpExtent := GetFullExtent; // perform second pass
    +      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
    +
    +      // as in the Prepare() call
    +      FLogicalExtent := tmpExtent;
    +      FCurrentExtent := FLogicalExtent;
    +    end;
    +
         if Legend.Visible and not Legend.UseSidebar then
           Legend.Prepare(ldd, FClipRect);
     
    @@ -1470,6 +1494,9 @@
       a: TChartAxis;
       s: TBasicChartSeries;
     begin
    +  FScaleValid := False;
    +  FScaleNeedsSecondPass := False;
    +
       for a in AxisList do
         if a.Transformations <> nil then
           a.Transformations.SetChart(Self);
    
    test.diff (4,910 bytes)
  • Reproduce2.zip (2,322 bytes)
  • update.diff (5,301 bytes)
    Index: components/tachart/tafuncseries.pas
    ===================================================================
    --- components/tachart/tafuncseries.pas	(revision 60589)
    +++ components/tachart/tafuncseries.pas	(working copy)
    @@ -1799,9 +1799,10 @@
       Result := Source.BasicExtent;
       if IsEmpty or (not Active) then exit;
     
    +  // TDrawFuncHelper needs a valid image-to-graph conversion
       if ParentChart = nil then exit;
    -  ParentChart.ScaleNeedsSecondPass := True;
    -  if not ParentChart.ScaleValid then exit;
    +  ParentChart.ExtentCalculationNeedsSecondPass;
    +  if not ParentChart.ImageGraphConvInitialized then exit;
     
       if FAutoFit then ExecFit;
     
    Index: components/tachart/tagraph.pas
    ===================================================================
    --- components/tachart/tagraph.pas	(revision 60589)
    +++ components/tachart/tagraph.pas	(working copy)
    @@ -233,8 +233,8 @@
         FOnExtentChanging: TChartEvent;
         FPrevLogicalExtent: TDoubleRect;
         FScale: TDoublePoint;    // Coordinates transformation
    -    FScaleValid: Boolean;
    -    FScaleNeedsSecondPass: Boolean;
    +    FScaleAndOffsetValid: Boolean;
    +    FScaleAndOffsetNeedSecondPass: Boolean;
         FSavedClipRect: TRect;
         FClipRectLock: Integer;
     
    @@ -361,8 +361,8 @@
         function XImageToGraph(AX: Integer): Double; inline;
         function YGraphToImage(AY: Double): Integer; inline;
         function YImageToGraph(AY: Integer): Double; inline;
    -    property ScaleValid: Boolean read FScaleValid;
    -    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
    +    procedure ExtentCalculationNeedsSecondPass; inline;
    +    property ImageGraphConvInitialized: Boolean read FScaleAndOffsetValid;
     
       public
         procedure LockClipRect;
    @@ -595,7 +595,7 @@
       FOffset.Y := rY.CalcOffset(FScale.Y);
       rX.UpdateMinMax(@XImageToGraph);
       rY.UpdateMinMax(@YImageToGraph);
    -  FScaleValid := True;
    +  FScaleAndOffsetValid := True;
     end;
     
     procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
    @@ -691,8 +691,8 @@
       FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
     
       FScale := DoublePoint(1, -1);
    -  FScaleValid := False;
    -  FScaleNeedsSecondPass := False;
    +  FScaleAndOffsetValid := False;
    +  FScaleAndOffsetNeedSecondPass := False;
     
       Width := DEFAULT_CHART_WIDTH;
       Height := DEFAULT_CHART_HEIGHT;
    @@ -884,6 +884,7 @@
     
     procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
     var
    +  NewClipRect: TRect;
       ldd: TChartLegendDrawingData;
       tmpExtent: TDoubleRect;
       tries: Integer;
    @@ -890,40 +891,40 @@
       s: TBasicChartSeries;
       ts: TBasicChartToolset;
     begin
    +  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
    +
    +  NewClipRect := ARect;
    +  with NewClipRect do begin
    +    Left += MarginsExternal.Left;
    +    Top += MarginsExternal.Top;
    +    Right -= MarginsExternal.Right;
    +    Bottom -= MarginsExternal.Bottom;
    +    FTitle.Measure(ADrawer, 1, Left, Right, Top);
    +    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    +  end;
    +
       ldd.FItems := nil;
       try
         Prepare;
     
    -    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
    +    if Legend.Visible then
    +      ldd := PrepareLegend(ADrawer, NewClipRect);
     
    -    for tries := 1 to 2 do
    +    for tries := 3 downto 0 do
         begin
    -      FClipRect := ARect;
    -      with MarginsExternal do begin
    -        FClipRect.Left += Left;
    -        FClipRect.Top += Top;
    -        FClipRect.Right -= Right;
    -        FClipRect.Bottom -= Bottom;
    -      end;
    +      FClipRect := NewClipRect;
     
    -      with ClipRect do begin
    -        FTitle.Measure(ADrawer, 1, Left, Right, Top);
    -        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    -      end;
    -
    -      if Legend.Visible then
    -        ldd := PrepareLegend(ADrawer, FClipRect);
    -
           PrepareAxis(ADrawer);
     
    -      if not FScaleNeedsSecondPass then break;
    +      if (tries = 0) or (not FScaleAndOffsetNeedSecondPass) then break;
     
    -      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
    -                               // in the Prepare() call above
    -      tmpExtent := GetFullExtent; // perform second pass
    -      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
    +      if FIsZoomed then break; // In this case, GetFullExtent() has not
    +                               // been called in the Prepare() call above
    +      tmpExtent := GetFullExtent; // Perform a second pass of extent calculation
    +      if tmpExtent = FLogicalExtent then break; // Converged successfully - second
    +                                                // pass hasn't changed the extent
     
    -      // as in the Prepare() call
    +      // As in the Prepare() call above
           FLogicalExtent := tmpExtent;
           FCurrentExtent := FLogicalExtent;
         end;
    @@ -1494,8 +1495,8 @@
       a: TChartAxis;
       s: TBasicChartSeries;
     begin
    -  FScaleValid := False;
    -  FScaleNeedsSecondPass := False;
    +  FScaleAndOffsetValid := False;
    +  FScaleAndOffsetNeedSecondPass := False;
     
       for a in AxisList do
         if a.Transformations <> nil then
    @@ -1881,6 +1882,11 @@
       Result := (AY - FOffset.Y) / FScale.Y;
     end;
     
    +procedure TChart.ExtentCalculationNeedsSecondPass;
    +begin
    +  FScaleAndOffsetNeedSecondPass := True;
    +end;
    +
     procedure TChart.ZoomFull(AImmediateRecalc: Boolean);
     begin
       if AImmediateRecalc then
    
    update.diff (5,301 bytes)
  • Exclusions.zip (2,327 bytes)
  • update2.diff (8,873 bytes)
    Index: components/tachart/tafuncseries.pas
    ===================================================================
    --- components/tachart/tafuncseries.pas	(revision 60594)
    +++ components/tachart/tafuncseries.pas	(working copy)
    @@ -293,6 +293,8 @@
       strict private
         FAutoFit: Boolean;
         FDrawFitRangeOnly: Boolean;
    +    FDomainExclusions: TIntervalList;
    +    FExtentAutoY: Boolean;
         FFitEquation: TFitEquation;
         FFitParams: TFitParamArray; // raw values, not transformed!
         FFitRange: TChartRange;
    @@ -313,6 +315,7 @@
         function GetParam_tValue(AIndex: Integer): Double;
         function IsFixedParamsStored: Boolean;
         procedure SetDrawFitRangeOnly(AValue: Boolean);
    +    procedure SetExtentAutoY(AValue: Boolean);
         procedure SetFitEquation(AValue: TFitEquation);
         procedure SetFitRange(AValue: TChartRange);
         procedure SetFixedParams(AValue: String);
    @@ -336,6 +339,7 @@
         function PrepareIntervals: TIntervalList;
         procedure SourceChanged(ASender: TObject); override;
       public
    +    procedure Assign(ASource: TPersistent); override;
         constructor Create(AOwner: TComponent); override;
         destructor Destroy; override;
       public
    @@ -365,6 +369,7 @@
         property Param_pValue[AIndex: Integer]: Double read GetParam_pValue;
         {$IFEND}
         property Param_tValue[AIndex: Integer]: Double read GetParam_tValue;
    +    property DomainExclusions: TIntervalList read FDomainExclusions;
         property FitStatistics: TFitStatistics read FFitStatistics;
         property ConfidenceLevel: Double read FConfidenceLevel write FConfidenceLevel;
         property ErrCode: TFitErrCode read FErrCode;
    @@ -375,6 +380,8 @@
         property AxisIndexY;
         property DrawFitRangeOnly: Boolean
           read FDrawFitRangeOnly write SetDrawFitRangeOnly default true;
    +    property ExtentAutoY: Boolean
    +      read FExtentAutoY write SetExtentAutoY default true;
         property FitEquation: TFitEquation
           read FFitEquation write SetFitEquation default fePolynomial;
         property FitRange: TChartRange read FFitRange write SetFitRange;
    @@ -599,7 +606,7 @@
     procedure TCustomFuncSeries.Assign(ASource: TPersistent);
     begin
       if ASource is TCustomFuncSeries then
    -    with TFuncSeries(ASource) do begin
    +    with TCustomFuncSeries(ASource) do begin
           Self.FDomainExclusions.Assign(FDomainExclusions);
           Self.FExtentAutoY := FExtentAutoY;
           Self.Pen := FPen;
    @@ -1617,11 +1624,26 @@
       FFitRange.Intersect(AXMin, AXMax);
     end;
     
    +procedure TFitSeries.Assign(ASource: TPersistent);
    +begin
    +  if ASource is TFitSeries then
    +    with TFitSeries(ASource) do begin
    +      Self.FDomainExclusions.Assign(FDomainExclusions);
    +      Self.FExtentAutoY := FExtentAutoY;
    +      Self.Pen := FPen;
    +      Self.FStep := FStep;
    +    end;
    +  inherited Assign(ASource);
    +end;
    +
     constructor TFitSeries.Create(AOwner: TComponent);
     begin
       inherited Create(AOwner);
       ToolTargets := [nptPoint, nptCustom];
       FAutoFit := true;
    +  FDomainExclusions := TIntervalList.Create;
    +  FDomainExclusions.OnChange := @StyleChanged;
    +  FExtentAutoY := true;
       FFitEquation := fePolynomial;
       FFitRange := TFitSeriesRange.Create(Self);
       FDrawFitRangeOnly := true;
    @@ -1636,6 +1658,7 @@
     
     destructor TFitSeries.Destroy;
     begin
    +  FreeAndNil(FDomainExclusions);
       FreeAndNil(FPen);
       FreeAndNil(FFitRange);
       FreeAndNil(FFitStatistics);
    @@ -1799,9 +1822,11 @@
       Result := Source.BasicExtent;
       if IsEmpty or (not Active) then exit;
     
    +  // TDrawFuncHelper needs a valid image-to-graph conversion
    +  if not FExtentAutoY then exit;
       if ParentChart = nil then exit;
    -  ParentChart.ScaleNeedsSecondPass := True;
    -  if not ParentChart.ScaleValid then exit;
    +  ParentChart.MultiPassScalingNeeded;
    +  if not ParentChart.ScalingValid then exit;
     
       if FAutoFit then ExecFit;
     
    @@ -2131,6 +2156,7 @@
     begin
       Result := TIntervalList.Create;
       try
    +    Result.Assign(DomainExclusions);
         CalcXRange(xmin, xmax);
         if DrawFitRangeOnly then begin
           Result.AddRange(NegInfinity, xmin);
    @@ -2149,6 +2175,13 @@
       UpdateParentChart;
     end;
     
    +procedure TFitSeries.SetExtentAutoY(AValue: Boolean);
    +begin
    +  if FExtentAutoY = AValue then exit;
    +  FExtentAutoY := AValue;
    +  UpdateParentChart;
    +end;
    +
     procedure TFitSeries.SetFitEquation(AValue: TFitEquation);
     begin
       if FFitEquation = AValue then exit;
    Index: components/tachart/tagraph.pas
    ===================================================================
    --- components/tachart/tagraph.pas	(revision 60594)
    +++ components/tachart/tagraph.pas	(working copy)
    @@ -233,8 +233,8 @@
         FOnExtentChanging: TChartEvent;
         FPrevLogicalExtent: TDoubleRect;
         FScale: TDoublePoint;    // Coordinates transformation
    -    FScaleValid: Boolean;
    -    FScaleNeedsSecondPass: Boolean;
    +    FScalingValid: Boolean;
    +    FMultiPassScalingNeeded: Boolean;
         FSavedClipRect: TRect;
         FClipRectLock: Integer;
     
    @@ -361,8 +361,8 @@
         function XImageToGraph(AX: Integer): Double; inline;
         function YGraphToImage(AY: Double): Integer; inline;
         function YImageToGraph(AY: Integer): Double; inline;
    -    property ScaleValid: Boolean read FScaleValid;
    -    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
    +    procedure MultiPassScalingNeeded; inline;
    +    property ScalingValid: Boolean read FScalingValid;
     
       public
         procedure LockClipRect;
    @@ -595,7 +595,7 @@
       FOffset.Y := rY.CalcOffset(FScale.Y);
       rX.UpdateMinMax(@XImageToGraph);
       rY.UpdateMinMax(@YImageToGraph);
    -  FScaleValid := True;
    +  FScalingValid := True;
     end;
     
     procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
    @@ -691,8 +691,8 @@
       FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
     
       FScale := DoublePoint(1, -1);
    -  FScaleValid := False;
    -  FScaleNeedsSecondPass := False;
    +  FScalingValid := False;
    +  FMultiPassScalingNeeded := False;
     
       Width := DEFAULT_CHART_WIDTH;
       Height := DEFAULT_CHART_HEIGHT;
    @@ -884,6 +884,7 @@
     
     procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
     var
    +  NewClipRect: TRect;
       ldd: TChartLegendDrawingData;
       tmpExtent: TDoubleRect;
       tries: Integer;
    @@ -890,40 +891,40 @@
       s: TBasicChartSeries;
       ts: TBasicChartToolset;
     begin
    +  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
    +
    +  NewClipRect := ARect;
    +  with NewClipRect do begin
    +    Left += MarginsExternal.Left;
    +    Top += MarginsExternal.Top;
    +    Right -= MarginsExternal.Right;
    +    Bottom -= MarginsExternal.Bottom;
    +    FTitle.Measure(ADrawer, 1, Left, Right, Top);
    +    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    +  end;
    +
       ldd.FItems := nil;
       try
         Prepare;
     
    -    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
    +    if Legend.Visible then
    +      ldd := PrepareLegend(ADrawer, NewClipRect);
     
    -    for tries := 1 to 2 do
    +    for tries := 3 downto 0 do
         begin
    -      FClipRect := ARect;
    -      with MarginsExternal do begin
    -        FClipRect.Left += Left;
    -        FClipRect.Top += Top;
    -        FClipRect.Right -= Right;
    -        FClipRect.Bottom -= Bottom;
    -      end;
    +      FClipRect := NewClipRect;
     
    -      with ClipRect do begin
    -        FTitle.Measure(ADrawer, 1, Left, Right, Top);
    -        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
    -      end;
    -
    -      if Legend.Visible then
    -        ldd := PrepareLegend(ADrawer, FClipRect);
    -
           PrepareAxis(ADrawer);
     
    -      if not FScaleNeedsSecondPass then break;
    +      if (tries = 0) or (not FMultiPassScalingNeeded) then break;
     
    -      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
    -                               // in the Prepare() call above
    -      tmpExtent := GetFullExtent; // perform second pass
    -      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
    +      if FIsZoomed then break; // In this case, GetFullExtent() has not
    +                               // been called in the Prepare() call above
    +      tmpExtent := GetFullExtent; // Perform a next pass of extent calculation
    +      if tmpExtent = FLogicalExtent then break; // Converged successfully - next
    +                                                // pass hasn't changed the extent
     
    -      // as in the Prepare() call
    +      // As in the Prepare() call above
           FLogicalExtent := tmpExtent;
           FCurrentExtent := FLogicalExtent;
         end;
    @@ -1494,8 +1495,8 @@
       a: TChartAxis;
       s: TBasicChartSeries;
     begin
    -  FScaleValid := False;
    -  FScaleNeedsSecondPass := False;
    +  FScalingValid := False;
    +  FMultiPassScalingNeeded := False;
     
       for a in AxisList do
         if a.Transformations <> nil then
    @@ -1881,6 +1882,11 @@
       Result := (AY - FOffset.Y) / FScale.Y;
     end;
     
    +procedure TChart.MultiPassScalingNeeded;
    +begin
    +  FMultiPassScalingNeeded := True;
    +end;
    +
     procedure TChart.ZoomFull(AImmediateRecalc: Boolean);
     begin
       if AImmediateRecalc then
    
    update2.diff (8,873 bytes)
  • comments.diff (917 bytes)
    Index: tagraph.pas
    ===================================================================
    --- tagraph.pas	(revision 60597)
    +++ tagraph.pas	(working copy)
    @@ -918,13 +918,13 @@
     
           if (tries = 0) or (not FMultiPassScalingNeeded) then break;
     
    -      // FIsZoom=true: GetFullExtent has not been called in Prepare() above
    +      // FIsZoomed=true: GetFullExtent() has not been called in Prepare() above
           if FIsZoomed then break;
     
           // Perform a next pass of extent calculation
           tmpExtent := GetFullExtent;
    -      // Converged successfully - next pass has not changed the extent --> break
    -      if tmpExtent = FLogicalExtent then break;
    +      // Converged successfully if next pass has not changed the extent --> break
    +      if tmpExtent = FLogicalExtent then break;                                               
     
           // As in the Prepare() call above
           FLogicalExtent := tmpExtent;
    
    comments.diff (917 bytes)

Activities

Marcin Wiazowski

2019-03-03 21:19

reporter  

Reproduce.zip (2,632 bytes)

Marcin Wiazowski

2019-03-03 21:19

reporter  

Reproduce.png (22,822 bytes)
Reproduce.png (22,822 bytes)

Marcin Wiazowski

2019-03-03 21:20

reporter  

experiment.diff (1,536 bytes)
Index: components/tachart/tafuncseries.pas
===================================================================
--- components/tachart/tafuncseries.pas	(revision 60570)
+++ components/tachart/tafuncseries.pas	(working copy)
@@ -305,6 +305,7 @@
     FErrCode: TFitErrCode;
     FFitStatistics: TFitStatistics;
     FConfidenceLevel: Double;
+    FExtentNestedCount: Integer;
     function GetParam(AIndex: Integer): Double;
     function GetParamCount: Integer;
     function GetParamError(AIndex: Integer): Double;
@@ -1793,8 +1794,38 @@
 end;
 
 function TFitSeries.Extent: TDoubleRect;
+var
+  de : TIntervalList;
+  ymin, ymax: Double;
 begin
   Result := Source.BasicExtent;
+
+  if IsEmpty or (not Active) then exit;
+  if (Result.a.X = EmptyExtent.a.X) or (Result.b.X = EmptyExtent.b.X) then exit;
+  if FExtentNestedCount <> 0 then exit;
+
+  if (FState = fpsValid) and (FErrCode = fitOK) then begin
+    Inc(FExtentNestedCount);
+    try
+      de := PrepareIntervals;
+      try
+        ymin := SafeInfinity;
+        ymax := NegInfinity;
+        with TDrawFuncHelper.Create(Self, de, @Calculate, Step) do
+          try
+            CalcAxisExtentY(Result.a.X, Result.b.X, ymin, ymax);
+            UpdateMinMax(ymin, Result.a.Y, Result.b.Y);
+            UpdateMinMax(ymax, Result.a.Y, Result.b.Y);
+          finally
+            Free;
+          end;
+      finally
+        de.Free;
+      end;
+    finally
+      Dec(FExtentNestedCount);
+    end;
+  end;
 end;
 
 function TFitSeries.FitParams: TDoubleDynArray;
experiment.diff (1,536 bytes)

wp

2019-03-03 23:39

developer   ~0114608

With your patch applied, run the fitdemo of TAChart and select the exp test function and the power fitting function - since the fitted exponent parameter is < 0 here the fitting function diverges at x=0, and the incorrectly fitted curve dominates the chart pushing all data points to the bottom.

Another issue is that the fit must be calculated inside the Extent method if valid fit results are not yet available (this is missing in your patch). The earliest call to ExecFit is in the Loaded method where scaling parameters are still unknown, i.e. Chart.FScale.X = 1. This means that the step size with which the extent is sampled (which is in image units) is now understood to be in graph or axis units. Your 3rd plot with the polygon fit thus is sampled only at x=1, x=2 and x=3 which still leaves the fitted curve truncated. Or conversely, imagine an x axis ranging between 0 and 1E9, then 1E9 function calls will be made for extent calculation, just because the chart is not yet scaled correctly.

I'd prefer to keep the current Extent calculation based on the data point values, but provide a public method FitExtent (or similar) which the user can call at runtime to see the entire fitted curve. Since this happens only after the fit has been calculated and drawn the extent calculation should be correct then.

Marcin Wiazowski

2019-03-04 02:25

reporter  

test.diff (4,910 bytes)
Index: components/tachart/tafuncseries.pas
===================================================================
--- components/tachart/tafuncseries.pas	(revision 60574)
+++ components/tachart/tafuncseries.pas	(working copy)
@@ -1601,7 +1601,7 @@
 var
   ext: TDoubleRect;
 begin
-  with Extent do begin
+  with Source.BasicExtent do begin
     ext.a := AxisToGraph(a);
     ext.b := AxisToGraph(b);
   end;
@@ -1793,8 +1793,31 @@
 end;
 
 function TFitSeries.Extent: TDoubleRect;
+var
+  de : TIntervalList;
 begin
   Result := Source.BasicExtent;
+  if IsEmpty or (not Active) then exit;
+
+  if ParentChart = nil then exit;
+  ParentChart.ScaleNeedsSecondPass := True;
+  if not ParentChart.ScaleValid then exit;
+
+  if FAutoFit then ExecFit;
+
+  if (FState = fpsValid) and (FErrCode = fitOK) then begin
+    de := PrepareIntervals;
+    try
+      with TDrawFuncHelper.Create(Self, de, @Calculate, Step) do
+        try
+          CalcAxisExtentY(Result.a.X, Result.b.X, Result.a.Y, Result.b.Y);
+        finally
+          Free;
+        end;
+    finally
+      de.Free;
+    end;
+  end;
 end;
 
 function TFitSeries.FitParams: TDoubleDynArray;
Index: components/tachart/tagraph.pas
===================================================================
--- components/tachart/tagraph.pas	(revision 60574)
+++ components/tachart/tagraph.pas	(working copy)
@@ -1,4 +1,4 @@
-{
+{
  /***************************************************************************
                                TAGraph.pas
                                -----------
@@ -233,6 +233,8 @@
     FOnExtentChanging: TChartEvent;
     FPrevLogicalExtent: TDoubleRect;
     FScale: TDoublePoint;    // Coordinates transformation
+    FScaleValid: Boolean;
+    FScaleNeedsSecondPass: Boolean;
     FSavedClipRect: TRect;
     FClipRectLock: Integer;
 
@@ -359,6 +361,8 @@
     function XImageToGraph(AX: Integer): Double; inline;
     function YGraphToImage(AY: Double): Integer; inline;
     function YImageToGraph(AY: Integer): Double; inline;
+    property ScaleValid: Boolean read FScaleValid;
+    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
 
   public
     procedure LockClipRect;
@@ -591,6 +595,7 @@
   FOffset.Y := rY.CalcOffset(FScale.Y);
   rX.UpdateMinMax(@XImageToGraph);
   rY.UpdateMinMax(@YImageToGraph);
+  FScaleValid := True;
 end;
 
 procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
@@ -686,6 +691,8 @@
   FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
 
   FScale := DoublePoint(1, -1);
+  FScaleValid := False;
+  FScaleNeedsSecondPass := False;
 
   Width := DEFAULT_CHART_WIDTH;
   Height := DEFAULT_CHART_HEIGHT;
@@ -878,32 +885,49 @@
 procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
 var
   ldd: TChartLegendDrawingData;
+  tmpExtent: TDoubleRect;
+  tries: Integer;
   s: TBasicChartSeries;
   ts: TBasicChartToolset;
 begin
-  Prepare;
+  ldd.FItems := nil;
+  try
+    Prepare;
 
-  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
+    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
 
-  FClipRect := ARect;
-  with MarginsExternal do begin
-    FClipRect.Left += Left;
-    FClipRect.Top += Top;
-    FClipRect.Right -= Right;
-    FClipRect.Bottom -= Bottom;
-  end;
+    for tries := 1 to 2 do
+    begin
+      FClipRect := ARect;
+      with MarginsExternal do begin
+        FClipRect.Left += Left;
+        FClipRect.Top += Top;
+        FClipRect.Right -= Right;
+        FClipRect.Bottom -= Bottom;
+      end;
 
-  with ClipRect do begin
-    FTitle.Measure(ADrawer, 1, Left, Right, Top);
-    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
-  end;
+      with ClipRect do begin
+        FTitle.Measure(ADrawer, 1, Left, Right, Top);
+        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
+      end;
 
-  ldd.FItems := nil;
-  if Legend.Visible then
-    ldd := PrepareLegend(ADrawer, FClipRect);
+      if Legend.Visible then
+        ldd := PrepareLegend(ADrawer, FClipRect);
 
-  try
-    PrepareAxis(ADrawer);
+      PrepareAxis(ADrawer);
+
+      if not FScaleNeedsSecondPass then break;
+
+      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
+                               // in the Prepare() call above
+      tmpExtent := GetFullExtent; // perform second pass
+      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
+
+      // as in the Prepare() call
+      FLogicalExtent := tmpExtent;
+      FCurrentExtent := FLogicalExtent;
+    end;
+
     if Legend.Visible and not Legend.UseSidebar then
       Legend.Prepare(ldd, FClipRect);
 
@@ -1470,6 +1494,9 @@
   a: TChartAxis;
   s: TBasicChartSeries;
 begin
+  FScaleValid := False;
+  FScaleNeedsSecondPass := False;
+
   for a in AxisList do
     if a.Transformations <> nil then
       a.Transformations.SetChart(Self);
test.diff (4,910 bytes)

Marcin Wiazowski

2019-03-04 02:28

reporter   ~0114610

> With your patch applied, run the fitdemo of TAChart and select the exp test function and the power fitting function - since the fitted exponent parameter is < 0 here the fitting function diverges at x=0, and the incorrectly fitted curve dominates the chart pushing all data points to the bottom.

Well, but this is as it should be in this case - I would rather say, that the current behavior is improper. But if this is a problem, the user may at least:
- display Y axis as logarithmic,
- use Chart.Extent.YMax.



> Another issue is that the fit must be calculated inside the Extent method if valid fit results are not yet available (this is missing in your patch).

I experienced recursive calls to Extent() method, but after tiny change in TFitSeries.CalcXRange(), this doesn't seem to be a problem anymore.



> The earliest call to ExecFit [...] then 1E9 function calls will be made for extent calculation, just because the chart is not yet scaled correctly.

In fact, first call to TFitSeries.Extent() is from TChartSeries.GetBounds(), which is called from TCustomChartSeries.GetGraphBounds(), which is called from TChart.GetFullExtent() - which is called from TChart.Prepare() to obtain ranges, that are later needed for scale calculation:

  for s in Series do begin
    try
      JoinBounds(s.GetGraphBounds);
    except
      s.Active := false;
      raise;
    end;
  end;

The problem is that chart's machinery obtains both X range and Y range at the same time - but this is needed, because TChart.PrepareAxis() operates on both horizontal and vertical dimensions together, to calculate both the scales iteratively.



> provide a public method FitExtent (or similar) which the user can call at runtime to see the entire fitted curve.

I'm not very convinced of this idea, I think that the user, by default, wants to see what he just expects to see.



However, this may be no more needed.

My first idea was: at the moment of TFitSeries.Draw() call, TFitSeries' extent could be updated and chart invalidated; then, when redrawing, the already updated extent would be used. But invalidating the chart is expensive - in particular, all the already drawn series must be drawn again.

So I modified the code a bit, to allow a second pass of the extent calculation; after the first pass, scales are ready, so second pass can use them.

In the attached test.diff, FScaleNeedsSecondPass is only for performance reasons - thanks to it, there is no performance penalty when using series other than TFitSeries. Seems to work properly, please test and see if you like it.

wp

2019-03-04 19:05

developer   ~0114643

Applied, thanks.

Marcin Wiazowski

2019-03-05 01:00

reporter  

Reproduce2.zip (2,322 bytes)

Marcin Wiazowski

2019-03-05 01:00

reporter  

update.diff (5,301 bytes)
Index: components/tachart/tafuncseries.pas
===================================================================
--- components/tachart/tafuncseries.pas	(revision 60589)
+++ components/tachart/tafuncseries.pas	(working copy)
@@ -1799,9 +1799,10 @@
   Result := Source.BasicExtent;
   if IsEmpty or (not Active) then exit;
 
+  // TDrawFuncHelper needs a valid image-to-graph conversion
   if ParentChart = nil then exit;
-  ParentChart.ScaleNeedsSecondPass := True;
-  if not ParentChart.ScaleValid then exit;
+  ParentChart.ExtentCalculationNeedsSecondPass;
+  if not ParentChart.ImageGraphConvInitialized then exit;
 
   if FAutoFit then ExecFit;
 
Index: components/tachart/tagraph.pas
===================================================================
--- components/tachart/tagraph.pas	(revision 60589)
+++ components/tachart/tagraph.pas	(working copy)
@@ -233,8 +233,8 @@
     FOnExtentChanging: TChartEvent;
     FPrevLogicalExtent: TDoubleRect;
     FScale: TDoublePoint;    // Coordinates transformation
-    FScaleValid: Boolean;
-    FScaleNeedsSecondPass: Boolean;
+    FScaleAndOffsetValid: Boolean;
+    FScaleAndOffsetNeedSecondPass: Boolean;
     FSavedClipRect: TRect;
     FClipRectLock: Integer;
 
@@ -361,8 +361,8 @@
     function XImageToGraph(AX: Integer): Double; inline;
     function YGraphToImage(AY: Double): Integer; inline;
     function YImageToGraph(AY: Integer): Double; inline;
-    property ScaleValid: Boolean read FScaleValid;
-    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
+    procedure ExtentCalculationNeedsSecondPass; inline;
+    property ImageGraphConvInitialized: Boolean read FScaleAndOffsetValid;
 
   public
     procedure LockClipRect;
@@ -595,7 +595,7 @@
   FOffset.Y := rY.CalcOffset(FScale.Y);
   rX.UpdateMinMax(@XImageToGraph);
   rY.UpdateMinMax(@YImageToGraph);
-  FScaleValid := True;
+  FScaleAndOffsetValid := True;
 end;
 
 procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
@@ -691,8 +691,8 @@
   FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
 
   FScale := DoublePoint(1, -1);
-  FScaleValid := False;
-  FScaleNeedsSecondPass := False;
+  FScaleAndOffsetValid := False;
+  FScaleAndOffsetNeedSecondPass := False;
 
   Width := DEFAULT_CHART_WIDTH;
   Height := DEFAULT_CHART_HEIGHT;
@@ -884,6 +884,7 @@
 
 procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
 var
+  NewClipRect: TRect;
   ldd: TChartLegendDrawingData;
   tmpExtent: TDoubleRect;
   tries: Integer;
@@ -890,40 +891,40 @@
   s: TBasicChartSeries;
   ts: TBasicChartToolset;
 begin
+  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
+
+  NewClipRect := ARect;
+  with NewClipRect do begin
+    Left += MarginsExternal.Left;
+    Top += MarginsExternal.Top;
+    Right -= MarginsExternal.Right;
+    Bottom -= MarginsExternal.Bottom;
+    FTitle.Measure(ADrawer, 1, Left, Right, Top);
+    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
+  end;
+
   ldd.FItems := nil;
   try
     Prepare;
 
-    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
+    if Legend.Visible then
+      ldd := PrepareLegend(ADrawer, NewClipRect);
 
-    for tries := 1 to 2 do
+    for tries := 3 downto 0 do
     begin
-      FClipRect := ARect;
-      with MarginsExternal do begin
-        FClipRect.Left += Left;
-        FClipRect.Top += Top;
-        FClipRect.Right -= Right;
-        FClipRect.Bottom -= Bottom;
-      end;
+      FClipRect := NewClipRect;
 
-      with ClipRect do begin
-        FTitle.Measure(ADrawer, 1, Left, Right, Top);
-        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
-      end;
-
-      if Legend.Visible then
-        ldd := PrepareLegend(ADrawer, FClipRect);
-
       PrepareAxis(ADrawer);
 
-      if not FScaleNeedsSecondPass then break;
+      if (tries = 0) or (not FScaleAndOffsetNeedSecondPass) then break;
 
-      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
-                               // in the Prepare() call above
-      tmpExtent := GetFullExtent; // perform second pass
-      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
+      if FIsZoomed then break; // In this case, GetFullExtent() has not
+                               // been called in the Prepare() call above
+      tmpExtent := GetFullExtent; // Perform a second pass of extent calculation
+      if tmpExtent = FLogicalExtent then break; // Converged successfully - second
+                                                // pass hasn't changed the extent
 
-      // as in the Prepare() call
+      // As in the Prepare() call above
       FLogicalExtent := tmpExtent;
       FCurrentExtent := FLogicalExtent;
     end;
@@ -1494,8 +1495,8 @@
   a: TChartAxis;
   s: TBasicChartSeries;
 begin
-  FScaleValid := False;
-  FScaleNeedsSecondPass := False;
+  FScaleAndOffsetValid := False;
+  FScaleAndOffsetNeedSecondPass := False;
 
   for a in AxisList do
     if a.Transformations <> nil then
@@ -1881,6 +1882,11 @@
   Result := (AY - FOffset.Y) / FScale.Y;
 end;
 
+procedure TChart.ExtentCalculationNeedsSecondPass;
+begin
+  FScaleAndOffsetNeedSecondPass := True;
+end;
+
 procedure TChart.ZoomFull(AImmediateRecalc: Boolean);
 begin
   if AImmediateRecalc then
update.diff (5,301 bytes)

Marcin Wiazowski

2019-03-05 01:03

reporter   ~0114651

I'm attaching update.diff, that solves some problems that I introduced in test.diff:


1) In TChart.Draw(), in the last "tries" iteration, after the PrepareAxis() call, no more changes to FLogicalExtent or FCurrentExtent should be made - so "break" has been added.


2) There may be a case, when not 2, but 3 "tries" iterations are needed - I'm attaching Reproduce2, that shows this under a debugger. This is because:
- iteration 1: only basic extent is returned, so returned series' Y range is 0 .. 9
- iteration 2: now full extent is returned, so returned series' Y range is 0 .. 30 - but "30" string is longer than "9" string, so width of the vertical chart's axis grows, making the horizontal axis shortened; this modifies FScale.X
- iteration 3: still full extent is returned, so returned series' Y range is 0 .. 30 - now the final FScale.X value is used

For - theoretically possible - extreme cases, I added one more loop iteration, i.e. they are 4 in total - this doesn't hurt us, since in (almost) all cases loop breaks in 2nd or 3rd iteration.


3) Recently, I introduced a ScaleNeedsSecondPass property - this was unfortunate, because someone could think, that he should set it to False - although, once set to True, it should never be set to False programatically. So I changed the property to an ExtentCalculationNeedsSecondPass procedure.


4) I changed some names to more correct or descriptive.


5) I also introduced some optimizations for the "tries" loop.

wp

2019-03-05 13:36

developer   ~0114660

Before applying it some comments:

I am not very fond of the new words "ExtentCalculationNeedsSecondPass" (there may be more than two now..., too long) and "ImageGraphConvInitialized" (too long). I'd prefer "MultiPassScalingNeeded" and "ScalingValid" - "scaling" means both scale factor and offset.

I'd also like to see a property "EnableMultiPassScaling" for TFitSeries because I think that extent calculation over the fitted curve is not always wanted. If you don't add it, I'll add it myself. See the fitdemo example above; to be more explicit: imagine a curve perfectly fitting data between 100 and 1000. When the curve must be drawn down to 0 for some reason, but diverges at x=0 the good fit for x>100 cannot be seen any more because the divergence dominates the scaling.

For my understanding: Why is it harmful when the user changes the FScaleNeedsSecondPass? It is used only within the Chart.Draw where it is even reset to false at entering. He has to override internal routines to effectively change the value such that the new value is considered. And then he maybe has good reason to do so.

Marcin Wiazowski

2019-03-05 14:35

reporter   ~0114663

> I'd prefer "MultiPassScalingNeeded" and "ScalingValid"

Sure.



Some note first: TFitSeries is something between a fully calculated series (like TFuncSeries and TExpressionSeries) and fully-source-based series. So it should share some functionalities of both of them.



> imagine a curve perfectly fitting data between 100 and 1000. When the curve must be drawn down to 0 for some reason, but diverges at x=0 the good fit for x>100 cannot be seen any more because the divergence dominates the scaling.

The problem in fact elsewhere: TFitSeries doesn't have a "DomainExclusions: TIntervalList" property - which is available in calculated series, like TFuncSeries and TExpressionSeries. In fact, TFitSeries uses domain exclusions internally - the object is created in TFitSeries.PrepareIntervals(), but is not available to the user (there is some reason for that).



> I'd also like to see a property "EnableMultiPassScaling" for TFitSeries because I think that extent calculation over the fitted curve is not always wanted.

I have nothing against - but such a solution is already known from calculated series: it's a property named ExtentAutoY. So I think that it should be named ExtentAutoY also here.



> Why is it harmful when the user changes the FScaleNeedsSecondPass?

This is a variable belonging to the whole chart (not to the series), telling the chart that at least one of its series needs a second pass. When we have TFitSeries and TLineSeries on the same chart, TFitSeries may set FScaleNeedsSecondPass to True - but then TLineSeries should not reset it to False - this would prevent TFitSeries from being drawn properly. So series should be able to set chart's FScaleNeedsSecondPass variable, but should not be able to clear it. The algorithm is:
- chart (not series) sets FScaleNeedsSecondPass to False,
- chart iterates over the series and asks them for their extents - every series is allowed to set FScaleNeedsSecondPass to True,
- chart checks if some series caused FScaleNeedsSecondPass to be True - if so, chart repeats the above.

So I changed the property to a procedure - so series is now able to set FScaleNeedsSecondPass, but not to clear it. Otherwise, someone could inherit from TLineSeries and think, that he should set FScaleNeedsSecondPass to False.



Please feel free to change the names as you want.

I'll try to make DomainExclusions publicly available, and also implement ExtentAutoY, and I'll be back here.

wp

2019-03-05 15:17

developer   ~0114664

I think DomainExclusions won't help against the effect of a pole at x=0. Even if nothing is painted exactly at x=0 the function value will become very large when the drawing routine starts right above x=0. Draw y=tan(x) in a TFuncSeries or TExpressionSeries; even if you properly define DomainExclusions at x = pi/2 the function will become very large in the vicinity; the effect of domain exclusions is just to avoid the connecting line from +INF to -INF.

Marcin Wiazowski

2019-03-05 15:33

reporter   ~0114665

But DomainEclusions.Epsilon can be used to enlarge margins around the excluded point.

Please also see "Plotting y = tan(x)" in http://wiki.freepascal.org/TAChart_Tutorial:_Function_Series.

Marcin Wiazowski

2019-03-05 15:54

reporter  

Exclusions.zip (2,327 bytes)

Marcin Wiazowski

2019-03-05 15:55

reporter   ~0114666

Ok, DomainEclusions.AddPoint (with an optional DomainEclusions.Epsilon change) behaves strange, but DomainEclusions.AddRange does its work.

I attached Exclusions.zip to show this.

wp

2019-03-05 15:58

developer   ~0114667

Last edited: 2019-03-05 16:23

View 3 revisions

Many ways leading to Rome...

But I refrain from using domain exclusions for this purpose. There is one reason why they should be available in TFitSeries, though: namely, when the custom fit function is not defined over the entire fit region, e.g. sqrt(1-x^2) for |x| > 1, but the function must be plotted for whatever reasons also for a larger range of x values. In this case, the series must also take care of the fitting routine not running into the undefined range. But in general, I would prefer to keep Domain Exclusions out of TFitSeries - whoever wants to fit such funny funtions will not be happy with the linear fit either, and will have to use an advanced fitting package anyway.

A remark also on the "ExtentAutoY": This has a different purpose in TFuncSeries. If set to true the series tries to find the maximum and minimum function values, but if set to false the extent is not calculated at all and is taken from the axis range. The FitSeries, however, does have data points and "ExtentAutoY" = false should take the extent from the data points only. This is "automatic", too.

Marcin Wiazowski

2019-03-05 19:27

reporter  

update2.diff (8,873 bytes)
Index: components/tachart/tafuncseries.pas
===================================================================
--- components/tachart/tafuncseries.pas	(revision 60594)
+++ components/tachart/tafuncseries.pas	(working copy)
@@ -293,6 +293,8 @@
   strict private
     FAutoFit: Boolean;
     FDrawFitRangeOnly: Boolean;
+    FDomainExclusions: TIntervalList;
+    FExtentAutoY: Boolean;
     FFitEquation: TFitEquation;
     FFitParams: TFitParamArray; // raw values, not transformed!
     FFitRange: TChartRange;
@@ -313,6 +315,7 @@
     function GetParam_tValue(AIndex: Integer): Double;
     function IsFixedParamsStored: Boolean;
     procedure SetDrawFitRangeOnly(AValue: Boolean);
+    procedure SetExtentAutoY(AValue: Boolean);
     procedure SetFitEquation(AValue: TFitEquation);
     procedure SetFitRange(AValue: TChartRange);
     procedure SetFixedParams(AValue: String);
@@ -336,6 +339,7 @@
     function PrepareIntervals: TIntervalList;
     procedure SourceChanged(ASender: TObject); override;
   public
+    procedure Assign(ASource: TPersistent); override;
     constructor Create(AOwner: TComponent); override;
     destructor Destroy; override;
   public
@@ -365,6 +369,7 @@
     property Param_pValue[AIndex: Integer]: Double read GetParam_pValue;
     {$IFEND}
     property Param_tValue[AIndex: Integer]: Double read GetParam_tValue;
+    property DomainExclusions: TIntervalList read FDomainExclusions;
     property FitStatistics: TFitStatistics read FFitStatistics;
     property ConfidenceLevel: Double read FConfidenceLevel write FConfidenceLevel;
     property ErrCode: TFitErrCode read FErrCode;
@@ -375,6 +380,8 @@
     property AxisIndexY;
     property DrawFitRangeOnly: Boolean
       read FDrawFitRangeOnly write SetDrawFitRangeOnly default true;
+    property ExtentAutoY: Boolean
+      read FExtentAutoY write SetExtentAutoY default true;
     property FitEquation: TFitEquation
       read FFitEquation write SetFitEquation default fePolynomial;
     property FitRange: TChartRange read FFitRange write SetFitRange;
@@ -599,7 +606,7 @@
 procedure TCustomFuncSeries.Assign(ASource: TPersistent);
 begin
   if ASource is TCustomFuncSeries then
-    with TFuncSeries(ASource) do begin
+    with TCustomFuncSeries(ASource) do begin
       Self.FDomainExclusions.Assign(FDomainExclusions);
       Self.FExtentAutoY := FExtentAutoY;
       Self.Pen := FPen;
@@ -1617,11 +1624,26 @@
   FFitRange.Intersect(AXMin, AXMax);
 end;
 
+procedure TFitSeries.Assign(ASource: TPersistent);
+begin
+  if ASource is TFitSeries then
+    with TFitSeries(ASource) do begin
+      Self.FDomainExclusions.Assign(FDomainExclusions);
+      Self.FExtentAutoY := FExtentAutoY;
+      Self.Pen := FPen;
+      Self.FStep := FStep;
+    end;
+  inherited Assign(ASource);
+end;
+
 constructor TFitSeries.Create(AOwner: TComponent);
 begin
   inherited Create(AOwner);
   ToolTargets := [nptPoint, nptCustom];
   FAutoFit := true;
+  FDomainExclusions := TIntervalList.Create;
+  FDomainExclusions.OnChange := @StyleChanged;
+  FExtentAutoY := true;
   FFitEquation := fePolynomial;
   FFitRange := TFitSeriesRange.Create(Self);
   FDrawFitRangeOnly := true;
@@ -1636,6 +1658,7 @@
 
 destructor TFitSeries.Destroy;
 begin
+  FreeAndNil(FDomainExclusions);
   FreeAndNil(FPen);
   FreeAndNil(FFitRange);
   FreeAndNil(FFitStatistics);
@@ -1799,9 +1822,11 @@
   Result := Source.BasicExtent;
   if IsEmpty or (not Active) then exit;
 
+  // TDrawFuncHelper needs a valid image-to-graph conversion
+  if not FExtentAutoY then exit;
   if ParentChart = nil then exit;
-  ParentChart.ScaleNeedsSecondPass := True;
-  if not ParentChart.ScaleValid then exit;
+  ParentChart.MultiPassScalingNeeded;
+  if not ParentChart.ScalingValid then exit;
 
   if FAutoFit then ExecFit;
 
@@ -2131,6 +2156,7 @@
 begin
   Result := TIntervalList.Create;
   try
+    Result.Assign(DomainExclusions);
     CalcXRange(xmin, xmax);
     if DrawFitRangeOnly then begin
       Result.AddRange(NegInfinity, xmin);
@@ -2149,6 +2175,13 @@
   UpdateParentChart;
 end;
 
+procedure TFitSeries.SetExtentAutoY(AValue: Boolean);
+begin
+  if FExtentAutoY = AValue then exit;
+  FExtentAutoY := AValue;
+  UpdateParentChart;
+end;
+
 procedure TFitSeries.SetFitEquation(AValue: TFitEquation);
 begin
   if FFitEquation = AValue then exit;
Index: components/tachart/tagraph.pas
===================================================================
--- components/tachart/tagraph.pas	(revision 60594)
+++ components/tachart/tagraph.pas	(working copy)
@@ -233,8 +233,8 @@
     FOnExtentChanging: TChartEvent;
     FPrevLogicalExtent: TDoubleRect;
     FScale: TDoublePoint;    // Coordinates transformation
-    FScaleValid: Boolean;
-    FScaleNeedsSecondPass: Boolean;
+    FScalingValid: Boolean;
+    FMultiPassScalingNeeded: Boolean;
     FSavedClipRect: TRect;
     FClipRectLock: Integer;
 
@@ -361,8 +361,8 @@
     function XImageToGraph(AX: Integer): Double; inline;
     function YGraphToImage(AY: Double): Integer; inline;
     function YImageToGraph(AY: Integer): Double; inline;
-    property ScaleValid: Boolean read FScaleValid;
-    property ScaleNeedsSecondPass: Boolean read FScaleNeedsSecondPass write FScaleNeedsSecondPass;
+    procedure MultiPassScalingNeeded; inline;
+    property ScalingValid: Boolean read FScalingValid;
 
   public
     procedure LockClipRect;
@@ -595,7 +595,7 @@
   FOffset.Y := rY.CalcOffset(FScale.Y);
   rX.UpdateMinMax(@XImageToGraph);
   rY.UpdateMinMax(@YImageToGraph);
-  FScaleValid := True;
+  FScalingValid := True;
 end;
 
 procedure TChart.Clear(ADrawer: IChartDrawer; const ARect: TRect);
@@ -691,8 +691,8 @@
   FGUIConnectorListener := TListener.Create(@FGUIConnector, @StyleChanged);
 
   FScale := DoublePoint(1, -1);
-  FScaleValid := False;
-  FScaleNeedsSecondPass := False;
+  FScalingValid := False;
+  FMultiPassScalingNeeded := False;
 
   Width := DEFAULT_CHART_WIDTH;
   Height := DEFAULT_CHART_HEIGHT;
@@ -884,6 +884,7 @@
 
 procedure TChart.Draw(ADrawer: IChartDrawer; const ARect: TRect);
 var
+  NewClipRect: TRect;
   ldd: TChartLegendDrawingData;
   tmpExtent: TDoubleRect;
   tries: Integer;
@@ -890,40 +891,40 @@
   s: TBasicChartSeries;
   ts: TBasicChartToolset;
 begin
+  ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
+
+  NewClipRect := ARect;
+  with NewClipRect do begin
+    Left += MarginsExternal.Left;
+    Top += MarginsExternal.Top;
+    Right -= MarginsExternal.Right;
+    Bottom -= MarginsExternal.Bottom;
+    FTitle.Measure(ADrawer, 1, Left, Right, Top);
+    FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
+  end;
+
   ldd.FItems := nil;
   try
     Prepare;
 
-    ADrawer.SetRightToLeft(BiDiMode <> bdLeftToRight);
+    if Legend.Visible then
+      ldd := PrepareLegend(ADrawer, NewClipRect);
 
-    for tries := 1 to 2 do
+    for tries := 3 downto 0 do
     begin
-      FClipRect := ARect;
-      with MarginsExternal do begin
-        FClipRect.Left += Left;
-        FClipRect.Top += Top;
-        FClipRect.Right -= Right;
-        FClipRect.Bottom -= Bottom;
-      end;
+      FClipRect := NewClipRect;
 
-      with ClipRect do begin
-        FTitle.Measure(ADrawer, 1, Left, Right, Top);
-        FFoot.Measure(ADrawer, -1, Left, Right, Bottom);
-      end;
-
-      if Legend.Visible then
-        ldd := PrepareLegend(ADrawer, FClipRect);
-
       PrepareAxis(ADrawer);
 
-      if not FScaleNeedsSecondPass then break;
+      if (tries = 0) or (not FMultiPassScalingNeeded) then break;
 
-      if FIsZoomed then break; // GetFullExtent() has not been called in this case,
-                               // in the Prepare() call above
-      tmpExtent := GetFullExtent; // perform second pass
-      if tmpExtent = FLogicalExtent then break; // second pass hasn't changed the extent
+      if FIsZoomed then break; // In this case, GetFullExtent() has not
+                               // been called in the Prepare() call above
+      tmpExtent := GetFullExtent; // Perform a next pass of extent calculation
+      if tmpExtent = FLogicalExtent then break; // Converged successfully - next
+                                                // pass hasn't changed the extent
 
-      // as in the Prepare() call
+      // As in the Prepare() call above
       FLogicalExtent := tmpExtent;
       FCurrentExtent := FLogicalExtent;
     end;
@@ -1494,8 +1495,8 @@
   a: TChartAxis;
   s: TBasicChartSeries;
 begin
-  FScaleValid := False;
-  FScaleNeedsSecondPass := False;
+  FScalingValid := False;
+  FMultiPassScalingNeeded := False;
 
   for a in AxisList do
     if a.Transformations <> nil then
@@ -1881,6 +1882,11 @@
   Result := (AY - FOffset.Y) / FScale.Y;
 end;
 
+procedure TChart.MultiPassScalingNeeded;
+begin
+  FMultiPassScalingNeeded := True;
+end;
+
 procedure TChart.ZoomFull(AImmediateRecalc: Boolean);
 begin
   if AImmediateRecalc then
update2.diff (8,873 bytes)

Marcin Wiazowski

2019-03-05 19:30

reporter   ~0114669

I attached update2.diff. It changes the names, as discussed above.

It implements ExtentAutoY.

I wasn't sure if you finally want to have DomainEclusions or not, so I just implemented it. Seems to work properly, but remove if you don't like it.

wp

2019-03-06 10:26

developer   ~0114676

Applied in r60597, thanks.

I removed DomainExclusions and renamed ExtentAutoY to CombinedExtentY. I set its default value to false, however, because normal real-world fits have many data points and usually fit a low-order polynomial where the risk of an overshoot of the fitted curve like in your examples is small (see fitdemo).

There is another case in which the new iterative extent calculation might be more useful: a BubbleSeries with BubbleRadiusUnits=bruX or bruY (i.e. the radius of the circular bubble is given in x or y axis units); in this case, the extent on the other axis is not calculated correctly at the moment. Assuming the bruX case for the moment, the basic idea is to calculate the radius of the bubble in x direction in image units and to apply that value to the y axis to calculate the extent in image coordinates back to axis units. This calculation has not been possible so far. I can try it myself, but you as the "inventor of the multi-pass extent calculation" should have the right to do it first - if you want, of course. You can attach the patch here although it is off-topic.

Marcin Wiazowski

2019-03-06 11:51

reporter   ~0114678

I'm very unfamiliar with bubble series - so I would be better if you could fix it.

However, instead, in the meantime I'll use the new functionality in some other series, to fix the problem that is there.

Marcin Wiazowski

2019-03-06 12:07

reporter   ~0114679

CombinedExtentY is nice.

One notice: to be more similar to ExtentAutoY, maybe it would be good to rename it to ExtentCombinedY?

Marcin Wiazowski

2019-03-06 16:44

reporter  

comments.diff (917 bytes)
Index: tagraph.pas
===================================================================
--- tagraph.pas	(revision 60597)
+++ tagraph.pas	(working copy)
@@ -918,13 +918,13 @@
 
       if (tries = 0) or (not FMultiPassScalingNeeded) then break;
 
-      // FIsZoom=true: GetFullExtent has not been called in Prepare() above
+      // FIsZoomed=true: GetFullExtent() has not been called in Prepare() above
       if FIsZoomed then break;
 
       // Perform a next pass of extent calculation
       tmpExtent := GetFullExtent;
-      // Converged successfully - next pass has not changed the extent --> break
-      if tmpExtent = FLogicalExtent then break;
+      // Converged successfully if next pass has not changed the extent --> break
+      if tmpExtent = FLogicalExtent then break;                                               
 
       // As in the Prepare() call above
       FLogicalExtent := tmpExtent;
comments.diff (917 bytes)

Marcin Wiazowski

2019-03-06 16:44

reporter   ~0114685

I attached comments.diff, for tiny comment adjustments.

wp

2019-03-06 17:52

developer   ~0114686

I am not a native English speaker, so I may be wrong. But "ExtentCombinedY" sounds wrong to me in this particular context. "ExtentAutoY" sounds wrong to me, too, but this was created before I joined the team, and it's not worth to break user code.

Nevertheless, I renamed it to "UseCombinedExtentY" to make clear that it is a boolean variable, not another DoubleRect like LogicalExtent or FullExtent etc.

Marcin Wiazowski

2019-03-06 17:58

reporter   ~0114687

Ok. Thanks!

Issue History

Date Modified Username Field Change
2019-03-03 21:19 Marcin Wiazowski New Issue
2019-03-03 21:19 Marcin Wiazowski File Added: Reproduce.zip
2019-03-03 21:19 Marcin Wiazowski File Added: Reproduce.png
2019-03-03 21:20 Marcin Wiazowski File Added: experiment.diff
2019-03-03 22:57 wp Assigned To => wp
2019-03-03 22:57 wp Status new => assigned
2019-03-03 23:39 wp Note Added: 0114608
2019-03-03 23:39 wp LazTarget => -
2019-03-03 23:39 wp Status assigned => feedback
2019-03-04 02:25 Marcin Wiazowski File Added: test.diff
2019-03-04 02:28 Marcin Wiazowski Note Added: 0114610
2019-03-04 02:28 Marcin Wiazowski Status feedback => assigned
2019-03-04 19:05 wp Note Added: 0114643
2019-03-04 19:06 wp Fixed in Revision => 60586
2019-03-04 19:06 wp LazTarget - => 2.2
2019-03-04 19:06 wp Status assigned => resolved
2019-03-04 19:06 wp Resolution open => fixed
2019-03-04 19:06 wp Target Version => 2.2
2019-03-05 01:00 Marcin Wiazowski File Added: Reproduce2.zip
2019-03-05 01:00 Marcin Wiazowski File Added: update.diff
2019-03-05 01:03 Marcin Wiazowski Note Added: 0114651
2019-03-05 01:03 Marcin Wiazowski Status resolved => assigned
2019-03-05 01:03 Marcin Wiazowski Resolution fixed => reopened
2019-03-05 13:36 wp Note Added: 0114660
2019-03-05 14:35 Marcin Wiazowski Note Added: 0114663
2019-03-05 15:17 wp Note Added: 0114664
2019-03-05 15:33 Marcin Wiazowski Note Added: 0114665
2019-03-05 15:54 Marcin Wiazowski File Added: Exclusions.zip
2019-03-05 15:55 Marcin Wiazowski Note Added: 0114666
2019-03-05 15:58 wp Note Added: 0114667
2019-03-05 16:14 wp Note Edited: 0114667 View Revisions
2019-03-05 16:23 wp Note Edited: 0114667 View Revisions
2019-03-05 19:27 Marcin Wiazowski File Added: update2.diff
2019-03-05 19:30 Marcin Wiazowski Note Added: 0114669
2019-03-06 10:26 wp Note Added: 0114676
2019-03-06 10:26 wp Fixed in Revision 60586 => 60586, 60597
2019-03-06 10:26 wp Status assigned => resolved
2019-03-06 10:26 wp Resolution reopened => fixed
2019-03-06 11:51 Marcin Wiazowski Note Added: 0114678
2019-03-06 11:51 Marcin Wiazowski Status resolved => assigned
2019-03-06 11:51 Marcin Wiazowski Resolution fixed => reopened
2019-03-06 12:07 Marcin Wiazowski Note Added: 0114679
2019-03-06 16:44 Marcin Wiazowski File Added: comments.diff
2019-03-06 16:44 Marcin Wiazowski Note Added: 0114685
2019-03-06 17:52 wp Note Added: 0114686
2019-03-06 17:52 wp Fixed in Revision 60586, 60597 => 60586, 60597, 60598
2019-03-06 17:52 wp Status assigned => resolved
2019-03-06 17:52 wp Resolution reopened => fixed
2019-03-06 17:58 Marcin Wiazowski Note Added: 0114687
2019-03-06 17:58 Marcin Wiazowski Status resolved => closed