#### View Issue Details

ID Project Category View Status Date Submitted Last Update 0034898 Lazarus TAChart public 2019-01-18 21:54 2019-01-19 03:16 Marcin Wiazowski wp normal minor always closed fixed 2.1 (SVN) 60103 2.0 0034898: TAChart: improper calculations in TChart.YImageToGraph 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 No tags attached. 60104, 60105 2.0 Win32/Win64 Attached Files

#### Activities

 2019-01-18 23:03 developer   ~0113464 > 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. 2019-01-18 23:32 reporter   ~0113465 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 2019-01-18 23:42 reporter   ~0113467 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; 2019-01-19 00:29 developer   ~0113469 Last edited: 2019-01-19 00:33View 2 revisions 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. 2019-01-19 00:36 reporter   ~0113470 Well, I don't have so impressive knowledge about TAChart's internals, so I can only agree. 2019-01-19 03:16 reporter   ~0113473 Fixed in r60105.

#### Issue History

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