[REVIEW REQUEST] RT-33442: isSelected in TableViewSelectionModel is called too many times

[Stephen F Northover]

If it is a noop, will it not break people when the system calls it and 
it does nothing?

Further, what about discontinuous selections (ie. one item, then an 
unselected item, then a million selected items in a block).  You can do 
this on native Windows using Shift+Ctrl+Select but I see that this is 
not supported in the FX table.

Steve

On 2013-10-17 11:28 PM, Jonathan Giles wrote:
> Hi all,
>
> I know we're very late in the release, but I'm proposing the addition of
> one new method to the TableSelectionModel abstract class, which will
> have implementations provided by TableView and TreeTableView. The method
> added into TableSelectionModel will be a no-op method by default (to
> hopefully not break anyone), and I propose it should have the following
> method signature:
>
> public void selectRange(int minRow, int minColumn, int maxRow, int
> maxColumn)
>
> This method will be used internally, and by any interested end user, to
> signal to the TableView/TreeTableView control to select all cells within
> the range of (minRow, minColumn) -> (maxRow, maxColumn), inclusive.
>
> The reason why I want to introduce this is summed up well in RT-33442
> and RT-33619. In short, given a big enough table, a shift+click can
> essentially cause the TableView to hang whilst it selects all the cells
> within the range. This is basically due to all the selection events
> overloading the system. The benefit of introducing this method is that I
> can make the selection essentially an atomic operation, which will
> result in drastically less event noise.
>
> To better understand what I mean, there is a patch attached to RT-33442
> (rt33442_2.patch). This patch introduces this change, as well as a
> number of other implementation changes. In doing so the selection
> performance in this use case goes from crashing my test application
> (presumably it would complete, but I give up after five minutes of
> waiting) to taking under 1 second.
>
> There is still more testing to be done, but after applying this patch
> all controls unit tests continue to pass. Also, as noted in the jira
> issue, this patch is incomplete as I have not ported the patch to also
> cover TreeTableView.
>
> So, after all that, should we add selectRange(minRow, minColumn, maxRow,
> maxColumn) to JavaFX 8.0? If so, does anyone have an improved method
> signature they can recommend? I will of course be writing unit tests to
> cover this method and testing to ensure there are no regressions as best
> I can.
>
> Thanks,
> -- Jonathan