| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0035077 | Lazarus | TAChart | public | 2019-02-13 18:47 | 2019-03-03 23:54 |
| Reporter | Marcin Wiazowski | Assigned To | wp | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Product Version | 2.1 (SVN) | Product Build | 60414 | ||
| Target Version | 2.2 | Fixed in Version | |||
| Summary | 0035077: TAChart: line series is drawn improperly for multi-value data sources | ||||
| Description | There is a problem in TLineSeries, when using multi-value data source: every label and every error bar is drawn multiple times, overwriting other, already drawn chart elements - in particular pointers. In TLineSeries.Draw(), there is a loop: DrawSingleLineInStack(ADrawer, 0); <==== FIRST CALL (FOR Y0) for i := 0 to Source.YCount - 2 do begin if Source.IsYErrorIndex(i+1) then Continue; UpdateGraphPoints(i, FStacked); DrawSingleLineInStack(ADrawer, i + 1); <==== NEXT CALLS (FOR Y1, Y2, ...) end; In each call to TLineSeries.DrawSingleLineInStack(..., AIndex) we have: case FColorEach of ceNone, cePoint: DrawDefaultLines; <==== draws a line requested by the AIndex parameter else DrawColoredLines; <==== draws a line requested by the AIndex parameter end; DrawErrorBars(ADrawer); <==== draws ALL error bars EACH TIME, regardless of the AIndex parameter DrawLabels(ADrawer); <==== draws ALL marks EACH TIME, regardless of the AIndex parameter if ShowPoints then <==== draws a pointer requested by the AIndex parameter DrawPointers(ADrawer, AIndex, FColorEach in [cePoint, cePointAndLineBefore, cePointAndLineAfter]); The attached "Reproduce" application uses DoubleBuffered = False for the chart, so each drawing phase can be seen immediately. To see the problem: - launch the application and move its window to such a location, that the window is fully visible - place a breakpoint at the beginning of the TLineSeries.DrawSingleLineInStack() method - minimize the application's window and restore it again - it will be repainted, so breakpoint will be reached - step over (F8) the TLineSeries.DrawSingleLineInStack() code (it will be called 4 times) and observe chart's behavior. Drawing phases will be: 1) pink horizontal line is drawn for Y0 = 2; error bars are drawn for Y0 = 2; marks are drawn for ALL Y values (should be drawn only for Y0 = 2); two green pointers are drawn for Y0 = 2 2) pink horizontal line is drawn for Y1 = 4; error bars are drawn for Y0 = 2 (should not be drawn anymore), so pink horizontal line for Y1 = 4 and green pointers for Y0 = 2 become partially overwritten; marks are drawn for ALL Y values (should be drawn only for Y1 = 4); two green pointers are drawn for Y1 = 4 3) similarly to 2) 4) similarly to 2) Final result can be seen on the attached Reproduce.png image: - mark links (short red lines) are (re)drawn OVER green pointers for Y = 2, 4 and 6 (but not 8) - although all of them should be always located BELOW the pointers - error bars (black lines) are (re)drawn OVER green pointers for Y = 2 and 4 - although they should be located BELOW the pointers - error bars (black lines) are (re)drawn OVER the pink horizontal line for Y = 4 - although they should be located BELOW the line To see a line series that is drawn properly, change data source's YCount from 4 to 1. The cause of the problem is: "DrawErrorBars(ADrawer)" and "DrawLabels(ADrawer)" calls internally ignore the AIndex parameter. Regards | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | 60420, 60423, 60448 | ||||
| LazTarget | 2.2 | ||||
| Widgetset | Win32/Win64 | ||||
| Attached Files |
| ||||
|
|
Reproduce.zip (3,419 bytes) |
|
|
Reproduce.png (6,130 bytes) |
|
|
No, all this is correct. Although DrawErrorBars and DrawLabels do not have AIndex as a parameter they get different plot data in FGraphPoints from the preceding UpdateGraphPoints call which filters the y values belonging to AIndex. Not very clear code, I know... The main problem is a documentation error: You simply should not do this. If you use a line series with 4 y values you'll get error bars only for y0 - that's the way it is implemented, the functions TCustomChartSource.GetYErrorBarValues and -Limits only consider y0. I am not saying that it is not possible to extend it to error bars for all y values, but then the user might get the idea to stack error bar series, who knows, and so the entire stacking story would have to be re-written, and so I decided to stay with a single y value for error bars only to make life easier. I do not consider this to be a serious drawback because the user still can split a multi-y-value lineseries into several single-valued series. I just updated the wiki saying that only y0 can be displayed by the current code. I am resolving this as "won't fix". |
|
|
Maybe I wasn't clear enough... I also think that error bars should be drawn only for Y0 - using same error values for different Y values doesn't make much sense. Since Y0 is always in use, and Y1, Y2, Y3 etc. are optional, error bars should be drawn for Y0. All I wanted to say is that: - error bars should be drawn only once, i.e. only when handling Y0 value - currently they are drawn 4 times, but each time at Y0 level! This causes drawing issues. The simplest fix I can see would be just: if AIndex = 0 then <==== ADDED DrawErrorBars(ADrawer); - similarly, each mark (label) should be drawn only once - currently, each mark (for example "aaa 2") is drawn 4 times! When handling Y0 (AIndex = 0), all 8 marks are drawn (i.e. two at Y0 level, two at Y1 level, two at Y2 level and two at Y3 level). When handling Y1 (AIndex = 1) - once again, all 8 marks are drawn. Similarly for Y2 and Y3. So, in total, mark drawing is executed 32 times instead of 8 times. This repainting alters already drawn pointers (green squares) - because mark links (red short lines) overwrite the pointers, that have been drawn in the meantime. In other words, each chart element that should be drawn is drawn, but some of them - i.e. error bars and labels - are drawn multiple times instead of just once - and ONLY THIS should be fixed. Drawing same elements multiple times causes visual issues (and slows drawing down). The "To see the problem" steps, that I listed above, seem to be the easiest way to see the visual issues immediately when they are appearing. |
|
|
Thanks for clarification. |
|
|
Almost fixed - there is still one problem: "aaa 2", "aaa 4", "aaa 6", "aaa 8" and "bbb 8" are drawn as expected - but "bbb 2", "bbb 4" and "bbb 6" are NOT drawn at all. In TBasicPointSeries.DrawLabels(), there is: if (AYIndex >= 0) then begin if si < AYIndex then Continue else if si > AYIndex then exit; <==== EXITS THE OUTER LOOP PREMATURELY end; The "exit" clause exits TWO loops, and this causes the problem. Try changing just to: if (AYIndex >= 0) and (si <> AYIndex) then Continue; |
|
|
Done. Of course the "exit" is nonsense, it should be "break" because I don't want the calculations for the upper levels of the same data point be done. |
|
|
All the problems described above are fixed now. Thanks to your patch, execution time of a SpeedTest application, attached to 0035031, has been dramatically reduced - from about 1860 ms to about 610 ms here! |
|
|
Reproduce2.zip (3,724 bytes) |
|
|
Reproduce2.png (17,033 bytes) |
|
|
I found some additional, related problems - please see the attached Reproduce2 application and the image. Problem 1: For TBSplineSeries, TCubicSplineSeries, TFitSeries and TBubbleSeries, only a line/bubbles for Y0 is/are drawn - although marks are drawn for all Y0, Y1, Y2 and Y3. Problem 2: In TFitSeries.Draw(), there is: DrawLabels(ADrawer); DrawErrorBars(ADrawer); DrawPointers(ADrawer); so error bars are painted OVER marks - for example, at the attached Reproduce2 image, at Chart 3, the "aaa 2" mark is overwritten. The code should be changed to: DrawErrorBars(ADrawer); DrawLabels(ADrawer); DrawPointers(ADrawer); Problem 3: In TBSplineSeries, TCubicSplineSeries, TFitSeries and TBubbleSeries, the "MarkPositions" property is public - although it should be PUBLISHED. |
|
|
> execution time of a SpeedTest application, attached to 0035031, has been > dramatically reduced - from about 1860 ms to about 610 ms here! Can't resist: Are you a sales man? In a "real" application this reduction will not be noticable... |
|
|
:D Of course, I'm not a sales man: - I'm a born engineer, so it's just nice for me to see, that solving some problems give us - just for free - 5.5x faster execution time (reduction from 3370 ms to 610 ms), - About real-life applications: on my chart, I have currently about 15 kinds of possible events - each kind using its own Y value on the chart. So, when using multi-value data source with YCount = 15, thanks to your last patch, I can reduce chart drawing time about 15x - starting from about 400 - 1500 ms (depending on the machine). Reducing drawing time from 1500 ms to about 100 - 150 ms *IS* a noticeable improvement, isn't it? Regards! |
|
|
- BSplineSeries: done (r60430), open issue with ignoring Style. - CubicSplineSeries: r60433, draws only the 1st y value (there is no reason for this limitation but my laziness to dive into the spline stuff again...) - FitSeries: r60433, 60434. Also uses only the 1st y value. I will probably stick to that because having several fit curves would require making the statistical analysis of the fit results available for each curve, and it's rather confusing already now with a single curve. - BubbleSeries: r60437 fixes positioning of the 2nd y value label (which is the bubble radius) at the bubble perimeter. MarkPositions does not work logically, the inside/outside options should refer to the center of the bubble, not the center of the viewport. AutoMargin must be updated for the BubbleRadiusUnit variants - but I don't know if it's worth the effort. - r60432: Unpublished Marks and MarkPositions in the elemental series and published them again wherever useful, well, almost everywhere, but now I have better control over these properties. - To do: check marks for Polar, OHLC and Box/Whisker. |
|
|
r60443: there are two issues in TBSplineSeries.Draw(): - a call to "DrawErrorBars(ADrawer)" in a loop seems to be redundant and should be removed - cosmetic: "splineend" -> "splineEnd" |
|
|
Done |
|
|
Reproduce4.zip (3,457 bytes) |
|
|
Reproduce4.png (7,451 bytes) |
|
|
Reproduce5.zip (3,490 bytes) |
|
|
Reproduce5.png (13,766 bytes) |
|
|
Side note first: please take a look at 0035125, I attached an additional patch, and asked some questions there (I couldn't reopen, because it's your report, not mine). Problem 1, 2 and 3: fixed. > BSplineSeries: done (r60430), open issue with ignoring Style. PROBLEM 4: Style is now properly applied to lines and marks, but for pointers only partially - see gray pointers in Reproduce 4. > CubicSplineSeries: r60433, draws only the 1st y value [...] > FitSeries: r60433, 60434. Also uses only the 1st y value [...] Good choice. > BubbleSeries [...] PROBLEM 5: Marks for bubble series, for lmpInside and lmpPositive, are currently drawn in a weird and unpredictable way - see Reproduce 5. > r60443: there are two issues in TBSplineSeries.Draw() [...] Fixed. |
|
|
> PROBLEM 4: Style is now properly applied to lines and marks, but for pointers > only partially - see gray pointers in Reproduce 4. Fixed ----- > PROBLEM 5: Marks for bubble series, for lmpInside and lmpPositive, are currently > drawn in a weird and unpredictable way - see Reproduce 5. That's ok. Your bubbles are too big, you don't see the starting points of the marks because of the overlaps. Select a smaller radius (Y2 column in DataPoints editor) to avoid the overlap. The first data point (X, Y1) refers to the bubble centers - that's where the marker starts. The second data point (Y2) is the radius, its label begins at the perimeter of the circle/ellipse. Because I wanted to avoid overlapping labels in the lmpInside mode I decided to move the Y2 mark to the opposite position on the perimeter. Therefore, the words "inside" and "outside" are a bit out of place here because one of the labels is always outside and the other one inside the bubble. And there is an issue with margin calculation in one of the BubbleRadiusUnits options which I am too lazy to fix ATM. |
|
|
Reproduce6.zip (3,726 bytes) |
|
|
Reproduce6.gif (34,626 bytes) |
|
|
Again, side note first: still some issue in 0035125. Problem 4: fixed. Problem 5: nothing to fix. Still some issues - see Reproduce6. All series share the same ListChartSource, having XCount = 2 and YCount = 4. Problem 6a: On Chart1 and Chart2, X2 values from data source are taken into account for horizontal extent calculation. Interestingly, after clicking the "Stacked" checkbox above the Chart2, problem disappears there. Problem 6b: On Chart3 and Chart4, move the mouse pointer, for example, to coordinates X=2 Y=8 or X=3 Y=9. Apparently, invisible points (having Y = Y2, Y3, Y4) are selected with CrosshairTool. The piece of code below seems to solve the problem, but I'm not sure, if this is a right solution: in TCubicSplineSeries.GetNearestPoint() and TFitSeries.GetNearestPoint(), place this: Result := inherited GetNearestPoint(AParams, AResults); // ADDED: if Result and (AResults.FYIndex <> 0) then begin Result := false; AResults.FYIndex := -1; // should be here? exit; end; |
|
|
Ad problem 6a: In TCustomChartSource.ExtentCumulative(), used for stacked series, there is: Result := Extent; if YCount < 2 then exit; In TCustomChartSource.ExtentList(), used for non-stacked series, there is: Result := Extent; if (YCount < 2) and (XCount < 2) then exit; This causes the difference when toggling Chart2's checkbox. BTW: Currently, there is a TCustomChartSource.BasicExtent() function, which uses only X0 and Y0 for calculations. Maybe a general solution would be useful, something like: TCustomChartSource.Extent(AXCount, AYCount : Integer). So, for TBSplineSeries and TLineSeries, something like this could be used: XXX.ExtentCumulative(): Result := Extent(1, YCount); XXX.ExtentList(): Result := Extent(1, YCount); |
|
|
The title of this report was "TLineSeries does not draw properly for multi-value data sources", and this has been fixed according to your original decription. Normally I am willing to handle related small bugs within the same report. But after dealing with TBSplineSeries, TCubicSplineSeries, TBubbleSeries, TFitSeries, we entered now the area of ChartTools. I must say STOP here, before this thread (like others) will never end: Please open new reports for other bugs. Nobody will ever be able to understand what's going on here. To complete this thread and to answer your points before I resolve this report hopefully for the last time: - TLineSeries and all other series (except for TFieldSeries) were never thought to work correctly with XCount > 1. - The series have a property ToolTargets. When you include nptXList and nptYList in the set the tools will use the value in these lists as data. This behavior therefore is intended (well, not very nice for those series which do not display multiple values, I agree) I will not fix these issues for the moment, there are more important things to do. (And if they were in separate reports there were a high chance that I would find them again, but having them at the end of a long thread I am not very optimistic...) |
|
|
Fixed 6a. I think I keep 6b. Since the effect can easily be avoided by using XCount = 1 or ToolTargets without XLists and YLists this may also be considered to be a feature. Maybe somebody can take advantage of it (it requires some imagination, though). |
|
|
Problem 6a: fix confirmed. Problem 6b: ok. Indeed, I made many of my reports large, by reopening them again and again with related issues - I'll try to avoid this. Thanks! |
