RFR: 8334900: IOOBE when adding data to a Series of a BarChart that already contains data [v3]
Andy Goryachev
angorya at openjdk.org
Mon Jul 15 16:44:57 UTC 2024
On Sat, 13 Jul 2024 10:00:31 GMT, Markus Mack <mmack at openjdk.org> wrote:
>> This PR is a fix for another IOOBE that I discovered while working on #1476.
>>
>> The PR simplifies the code for adding a series that already contains data by adding the data points one-by-one.
>> As far as I can see no attempt was previously made to optimize the bulk operation except for some trivial O(1) operations, so this should have no noticable performance impact.
>>
>> Accidentally this fixes another bug related to the missing "negative" style class when negative data values are added.
>>
>> Also, the PR aligns the handling of duplicate categories with the behavior clarified in #1476, when there are duplicates in the data that was already in the series before the series was added to the chart.
>>
>> Note a change was made to the createTestSeries() test method, letting it start at index 1, avoiding the duplicate data items resulting from multiplying by 0.
>> Without this change `testSeriesRemoveAnimatedStyleClasses` would fail because it counts the number of plot children, where the duplicates are now removed.
>
> Markus Mack has updated the pull request incrementally with one additional commit since the last revision:
>
> fix "negative" style class when series is changed
I am going to ask this question - unrelated to this PR, but in the context of this PR.
Consider the case of a context menu, where the application needs to know the data point from the screen/local coordinates.
Currently, we can get the Node from the Data instance (XYChart.Data.node property), but not the other way around - that is, given the chart, and given the PickResult of a MouseEvent, it is impossible to get the data point corresponding to a Node. In other words, we have the model->view path but not the view->model one.
In theory, the app developer could monitor the `node` property of Data and use `Node.getProperties()` map to set the Data instance on the node, or they could iterate through all the visible data points, matching the node with the data point, so it is not a big issue.
I am interested in your opinion. What do you think?
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1488#issuecomment-2228944603
More information about the openjfx-dev
mailing list