On Thu, 20 Jun 2024 16:34:23 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This PR provides the test case given in the JBS issue, and a simple fix for
>> the index calculation when inserting data after previous data with duplicate
>> categories.
>>
>> Also, I've added a comment to `BarChart`s javadoc, clarifying the behavior
>> that was apparently assumed (but broken) previously.
>>
>> The index lookup is skipped for performance reasons if there are no
>> duplicates, corresponding to the previous implementation.
>> Further optimizations would be possible, but probably are not really helpful
>> without more extensive changes. The previous code already loops over all
>> categories to check if they are present, typically nested in a loop adding
>> many data items, thus already scaling quadratically when adding lots of
>> mostly unique data points.
>
> modules/javafx.controls/src/main/java/javafx/scene/chart/BarChart.java line
> 215:
>
>> 213: if (!categoryAxis.getCategories().contains(category)) {
>> 214: // find category index in case data contains duplicate
>> categories
>> 215: int i = series.getData().size() !=
>> categoryAxis.getCategories().size() ? series.getItemIndex(item) :
>
> this is clever: little impact on performance in the case of non-duplicate
> categories (the most common case).
However, I don't see the issue if I just add a duplicate data point after the
chart is rendered (the code jumps to line 218 in the master branch).
This makes me think that perhaps the fix should include fixing the logic of
adding category to the right hashtables at the right moment. What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1476#discussion_r1647868437
