View Issue Details

IDProjectCategoryView StatusLast Update
0035360LazarusTAChartpublic2019-04-12 23:13
ReporterMarcin WiazowskiAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN)Product Build60918 
Target Version2.2Fixed in Version 
Summary0035360: TAChart: missing Notify() calls added to TListChartSource
DescriptionTo reproduce the problem, launch the attached application and press the "Test!" button - it modifies data sources, but nothing happens. Then minimize the application's window and restore it again - now charts start to show the current data properly.

This is due to missing Notify() calls in TListChartSource's SetXList(), SetYList() and Sort() methods. The attached patch fixes the issue.

As I checked, all the other TListChartSource's methods, that modify data source's contents, make the Notify() calls properly.
TagsNo tags attached.
Fixed in Revision60923
LazTarget2.2
WidgetsetWin32/Win64
Attached Files
  • Reproduce.zip (2,474 bytes)
  • patch.diff (2,618 bytes)
    Index: components/tachart/tasources.pas
    ===================================================================
    --- components/tachart/tasources.pas	(revision 60918)
    +++ components/tachart/tasources.pas	(working copy)
    @@ -522,11 +522,20 @@
         raise EXListEmptyError.Create('AddXListYList: XList is empty');
       if Length(AY) = 0 then
         raise EYListEmptyError.Create('AddXListYList: YList is empty');
    -  Result := Add(AX[0], AY[0], ALabel, AColor);
    -  if Length(AX) > 1 then
    -    SetXList(Result, AX[1..High(AX)]);
    -  if Length(AY) > 1 then
    -    SetYList(Result, AY[1..High(AY)]);
    +
    +  Inc(FUpdateCount); // Optimization: prevent useless notifications.
    +                     // We don't call BeginUpdate() to achieve this,
    +                     // to avoid invalidating the caches.
    +  try
    +    Result := Add(AX[0], AY[0], ALabel, AColor);
    +    if Length(AX) > 1 then
    +      SetXList(Result, AX[1..High(AX)]);
    +    if Length(AY) > 1 then
    +      SetYList(Result, AY[1..High(AY)]);
    +  finally
    +    Dec(FUpdateCount);
    +  end;
    +  Notify;
     end;
     
     function TListChartSource.AddXYList(
    @@ -535,9 +544,18 @@
     begin
       if Length(AY) = 0 then
         raise EYListEmptyError.Create('AddXYList: Y List is empty');
    -  Result := Add(AX, AY[0], ALabel, AColor);
    -  if Length(AY) > 1 then
    -    SetYList(Result, AY[1..High(AY)]);
    +
    +  Inc(FUpdateCount); // Optimization: prevent useless notifications.
    +                     // We don't call BeginUpdate() to achieve this,
    +                     // to avoid invalidating the caches.
    +  try
    +    Result := Add(AX, AY[0], ALabel, AColor);
    +    if Length(AY) > 1 then
    +      SetYList(Result, AY[1..High(AY)]);
    +  finally
    +    Dec(FUpdateCount);
    +  end;
    +  Notify;
     end;
     
     procedure TListChartSource.Clear; inline;
    @@ -689,10 +707,7 @@
     begin
       if FSorted = AValue then exit;
       FSorted := AValue;
    -  if Sorted then begin
    -    Sort;
    -    Notify;
    -  end;
    +  if Sorted then Sort;
     end;
     
     procedure TListChartSource.SetText(AIndex: Integer; AValue: String);
    @@ -728,6 +743,7 @@
         for i := 0 to Min(High(AXList), High(XList)) do
           XList[i] := AXList[i];
       FXListExtentIsValid := false;
    +  Notify;
     end;
     
     function TListChartSource.SetXValue(AIndex: Integer; AValue: Double): Integer;
    @@ -796,6 +812,7 @@
           YList[i] := AYList[i];
       FCumulativeExtentIsValid := false;
       FYListExtentIsValid := false;
    +  Notify;
     end;
     
     procedure TListChartSource.SetYValue(AIndex: Integer; AValue: Double);
    @@ -850,6 +867,7 @@
     procedure TListChartSource.Sort;
     begin
       FData.Sort(@CompareDataItemX);
    +  Notify;
     end;
     
     procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
    
    patch.diff (2,618 bytes)

Activities

Marcin Wiazowski

2019-04-10 23:58

reporter  

Reproduce.zip (2,474 bytes)

Marcin Wiazowski

2019-04-10 23:59

reporter  

patch.diff (2,618 bytes)
Index: components/tachart/tasources.pas
===================================================================
--- components/tachart/tasources.pas	(revision 60918)
+++ components/tachart/tasources.pas	(working copy)
@@ -522,11 +522,20 @@
     raise EXListEmptyError.Create('AddXListYList: XList is empty');
   if Length(AY) = 0 then
     raise EYListEmptyError.Create('AddXListYList: YList is empty');
-  Result := Add(AX[0], AY[0], ALabel, AColor);
-  if Length(AX) > 1 then
-    SetXList(Result, AX[1..High(AX)]);
-  if Length(AY) > 1 then
-    SetYList(Result, AY[1..High(AY)]);
+
+  Inc(FUpdateCount); // Optimization: prevent useless notifications.
+                     // We don't call BeginUpdate() to achieve this,
+                     // to avoid invalidating the caches.
+  try
+    Result := Add(AX[0], AY[0], ALabel, AColor);
+    if Length(AX) > 1 then
+      SetXList(Result, AX[1..High(AX)]);
+    if Length(AY) > 1 then
+      SetYList(Result, AY[1..High(AY)]);
+  finally
+    Dec(FUpdateCount);
+  end;
+  Notify;
 end;
 
 function TListChartSource.AddXYList(
@@ -535,9 +544,18 @@
 begin
   if Length(AY) = 0 then
     raise EYListEmptyError.Create('AddXYList: Y List is empty');
-  Result := Add(AX, AY[0], ALabel, AColor);
-  if Length(AY) > 1 then
-    SetYList(Result, AY[1..High(AY)]);
+
+  Inc(FUpdateCount); // Optimization: prevent useless notifications.
+                     // We don't call BeginUpdate() to achieve this,
+                     // to avoid invalidating the caches.
+  try
+    Result := Add(AX, AY[0], ALabel, AColor);
+    if Length(AY) > 1 then
+      SetYList(Result, AY[1..High(AY)]);
+  finally
+    Dec(FUpdateCount);
+  end;
+  Notify;
 end;
 
 procedure TListChartSource.Clear; inline;
@@ -689,10 +707,7 @@
 begin
   if FSorted = AValue then exit;
   FSorted := AValue;
-  if Sorted then begin
-    Sort;
-    Notify;
-  end;
+  if Sorted then Sort;
 end;
 
 procedure TListChartSource.SetText(AIndex: Integer; AValue: String);
@@ -728,6 +743,7 @@
     for i := 0 to Min(High(AXList), High(XList)) do
       XList[i] := AXList[i];
   FXListExtentIsValid := false;
+  Notify;
 end;
 
 function TListChartSource.SetXValue(AIndex: Integer; AValue: Double): Integer;
@@ -796,6 +812,7 @@
       YList[i] := AYList[i];
   FCumulativeExtentIsValid := false;
   FYListExtentIsValid := false;
+  Notify;
 end;
 
 procedure TListChartSource.SetYValue(AIndex: Integer; AValue: Double);
@@ -850,6 +867,7 @@
 procedure TListChartSource.Sort;
 begin
   FData.Sort(@CompareDataItemX);
+  Notify;
 end;
 
 procedure TListChartSource.UpdateCachesAfterAdd(AX, AY: Double);
patch.diff (2,618 bytes)

wp

2019-04-11 11:03

developer   ~0115423

Done, thank you.

Marcin Wiazowski

2019-04-12 00:25

reporter   ~0115431

Cosmetic issue: I can see, that comment (about optimization) in TListChartSource.AddXListYList() has been shortened; in this case, also same change should be made below, in TListChartSource.AddXYList()...

wp

2019-04-12 09:31

developer   ~0115437

ok

Marcin Wiazowski

2019-04-12 23:13

reporter   ~0115454

Thanks.

Issue History

Date Modified Username Field Change
2019-04-10 23:58 Marcin Wiazowski New Issue
2019-04-10 23:58 Marcin Wiazowski File Added: Reproduce.zip
2019-04-10 23:59 Marcin Wiazowski File Added: patch.diff
2019-04-11 00:19 Maxim Ganetsky Assigned To => wp
2019-04-11 00:19 Maxim Ganetsky Status new => assigned
2019-04-11 11:03 wp Fixed in Revision => 60923
2019-04-11 11:03 wp LazTarget => 2.2
2019-04-11 11:03 wp Note Added: 0115423
2019-04-11 11:03 wp Status assigned => resolved
2019-04-11 11:03 wp Resolution open => fixed
2019-04-11 11:03 wp Target Version => 2.2
2019-04-12 00:25 Marcin Wiazowski Note Added: 0115431
2019-04-12 00:25 Marcin Wiazowski Status resolved => assigned
2019-04-12 00:25 Marcin Wiazowski Resolution fixed => reopened
2019-04-12 09:31 wp Note Added: 0115437
2019-04-12 09:31 wp Status assigned => resolved
2019-04-12 09:31 wp Resolution reopened => fixed
2019-04-12 23:13 Marcin Wiazowski Note Added: 0115454
2019-04-12 23:13 Marcin Wiazowski Status resolved => closed