View Issue Details
ID  Project  Category  View Status  Date Submitted  Last Update 

0034898  Lazarus  TAChart  public  20190118 21:54  20190119 03:16 
Reporter  Marcin Wiazowski  Assigned To  wp  
Priority  normal  Severity  minor  Reproducibility  always 
Status  closed  Resolution  fixed  
Product Version  2.1 (SVN)  Product Build  60103  
Target Version  2.0  Fixed in Version  
Summary  0034898: TAChart: improper calculations in TChart.YImageToGraph  
Description  Problem described here is related to 0034819, but has been described separately here, to avoid mess in the original report. In TAGraph.pas, there are two functions declared: function TChart.XImageToGraph(AX: Integer): Double; begin if AX >= MAX_COORD then Result := Infinity * SgnInt(FScale.X) else if AX <= MAX_COORD then Result := Infinity * SgnInt(FScale.X) else Result := (AX  FOffset.X) / FScale.X; end; function TChart.YImageToGraph(AY: Integer): Double; begin if AY >= MAX_COORD then Result := Infinity * SgnInt(FScale.Y) else if AY <= MAX_COORD then Result := +Infinity * SgnInt(FScale.Y) else Result := (AY  FOffset.Y) / FScale.Y; end; The idea is:  for very large, positive values, return +INF,  for very large, negative values, return INF,  in other cases perfrom the calculations. There is a mistake in TChart.YImageToGraph: "+Infinity" should be changed to "Infinity". Regards  
Tags  No tags attached.  
Fixed in Revision  60104, 60105  
LazTarget  2.0  
Widgetset  Win32/Win64  
Attached Files 


> There is a mistake in TChart.YImageToGraph: "+Infinity" should be changed to "Infinity". Why? On the y axis, FScale.Y is negative, and therefore, the expression Result := +Infinity * SgnInt(FScale.Y) is negative. 

Let's assume for the moment, that FOffset = 0 and that FScale = 2 (so it is negative). Let's assume first, that AY is equal to 1000. In this case, it is in the MAX_COORD .. MAX_COORD range, so result will be: Result := (AY  FOffset.Y) / FScale.Y so: Result := (1000  0) / (2) so: Result := 500 so Result is positive. For AY = 1000000000, condition "AY <= MAX_COORD" is met, so calculation currently is: Result := +Infinity * SgnInt(FScale.Y) so: Result := +Infinity * (1) so: Result := Infinity so Result is negative, although it still should be positive. Please note, that there is no such problem with AY = +1000 and AY = +1000000000  both results will be negative in this case. Regards 

But I can see another problem now: Result's sign depends not only on FScale's sign, but also on FOffset value  in particular, (AY  FOffset.Y) may give either positive, or negative value. So functions should be like: function TChart.XImageToGraph(AX: Integer): Double; begin if (AX < MAX_COORD) or (AX > MAX_COORD) then Result := Infinity * SgnInt(AX  FOffset.X) * SgnInt(FScale.X) else Result := (AX  FOffset.X) / FScale.X; end; function TChart.YImageToGraph(AY: Integer): Double; begin if (AY < MAX_COORD) or (AY > MAX_COORD) then Result := Infinity * SgnInt(AY  FOffset.Y) * SgnInt(FScale.Y) else Result := (AY  FOffset.Y) / FScale.Y; end; 

No, too many thoughts for something which never will happen. These changes were introduced in r60075 where I fixed the bug that the zero level of an area series was suddenly drawn at the top of the chart when the axis was logarithmic  it should have been at Inf because log(0) is Inf. The coordinate transformation made this to MaxInt pixels which is correct. But the canvas does not seem to like coordinate > about 100 millions because the area was drawn inverted. From this reason I truncated the the pixel coordinate at the MAX_COORD which still works correctly. For the opposite conversion, screen to graph coordinate, I thought that the Inf value should be restored, but I already knew that this case will never happen: the screen coordinates will already be clipped somewhere before they become this large. I think it's not worth to put many thoughts into this case, and I'll revert ImageToGraph to the former code (GraphToImage, of course, will remain changed). And the coordinate transformations are called again and again, so we should not waste cycles in them. 

Well, I don't have so impressive knowledge about TAChart's internals, so I can only agree. 

Fixed in r60105. 
Date Modified  Username  Field  Change 

20190118 21:54  Marcin Wiazowski  New Issue  
20190118 23:01  wp  Assigned To  => wp 
20190118 23:01  wp  Status  new => assigned 
20190118 23:03  wp  Note Added: 0113464  
20190118 23:13  wp  LazTarget  =>  
20190118 23:13  wp  Status  assigned => feedback 
20190118 23:32  Marcin Wiazowski  Note Added: 0113465  
20190118 23:32  Marcin Wiazowski  Status  feedback => assigned 
20190118 23:42  Marcin Wiazowski  Note Added: 0113467  
20190119 00:29  wp  Note Added: 0113469  
20190119 00:32  wp  Fixed in Revision  => 60104, 60105 
20190119 00:32  wp  LazTarget   => 2.0 
20190119 00:32  wp  Status  assigned => resolved 
20190119 00:32  wp  Resolution  open => fixed 
20190119 00:32  wp  Target Version  => 2.0 
20190119 00:33  wp  Note Edited: 0113469  View Revisions 
20190119 00:36  Marcin Wiazowski  Note Added: 0113470  
20190119 03:16  Marcin Wiazowski  Note Added: 0113473  
20190119 03:16  Marcin Wiazowski  Status  resolved => closed 