I'm late to this discussion, and I don't see anything wrong with the selectRange() API, but perhaps another option worth considering would be to add a DisableSelectionEvents or CoalesceSelectionEvents attribute.
The only advantage to that I can think of is that it could allow bulk selection of non-contiguous cells which might otherwise also exhibit the slowdown. jeff On Oct 21, 2013, at 8:35 AM, Stephen F Northover <[email protected]> wrote: > Hi Johnathan, > > 1) Is it possible to do the optimization without adding API (ugly but safe)? > 2) Another alternative (ugly) is to add the API but make it return a boolean > indicating whether it happened or not. > 3) It seem really weird to me that you can't convert from Column to index and > back again. What about getColumns()? > > Steve > > On 2013-10-21 5:04 AM, Jonathan Giles wrote: >> The only reason why we tended to prefer TableColumn* in the API over ints is >> that the TableColumn approach more clearly defined what columns were meant >> to be selected (both because you could read directly back from the argument >> list and because there was never any chance that the int positions could >> become corrupted if the column order were to ever change in the middle of an >> operation and the int values would no longer be valid). >> >> In general it is a relatively weak argument (but an argument nonetheless), >> so I'm not massively concerned one way or the other, except from an API >> consistency point of view, where the TableColumn approach definitely is more >> consistent. I figure I'm about to call it a night here, so I'll wait and see >> what feedback is received over night and will hopefully have the go-ahead >> from the community to finish this changeset off and push tomorrow :-) >> -- Jonathan >> On 21/10/2013 9:53 p.m., Johan Vos wrote: >>> I would prefer to have the selectRange (int, int, int, int) call. I know >>> TableColumn* is used heavily in the API, but it doesn't sound right to me >>> to use a primitive (int) for a row-selector and an Object for a >>> column-selector. >>> Having a no-op on the abstract call sounds ok, everybody happy. >>> >>> Leaving the API as it is now is not really an option IMHO, as performance >>> indeed becomes a problem for large tables. >>> >>> - Johan >>> >>> >>> 2013/10/20 Jonathan Giles <[email protected] >>> <mailto:[email protected]>> >>> >>> On 19/10/2013 8:24 a.m., Jonathan Giles wrote: >>> > On 19/10/2013 2:13 a.m., Stephen F Northover wrote: >>> >> If it is a noop, will it not break people when the system >>> calls it and >>> >> it does nothing? >>> > Only the abstract TableSelectionModel is a no-op, the >>> implementations >>> > (one each for TableView and TreeTableView) will both override and >>> > provide the implementation that I already have in the patch for >>> > TableView. I would rather have the implementation in the base >>> class but >>> > this is not feasible due to requirements in looking up column >>> indices. >>> I looked into providing a default implementation in >>> TableSelectionModel, >>> but the problem is is that there just isn't enough data at this >>> level of >>> the API. >>> >>> In short, the API could take two forms: >>> selectRange(int minRow, int minColumn, int maxRow, int maxColumn) >>> selectRange(int minRow, TableColumnBase minColumn, int maxRow, >>> TableColumnBase maxColumn) >>> >>> Either implementation is possible (although at present I've >>> implemented >>> the first approach, but for consistency it is probably better we >>> use the >>> second approach as all other API uses TableColumn* to represent >>> columns, >>> not ints). >>> >>> The problem is is that neither approach gives me what I want for a >>> default implementation, which would take the following form: >>> for (int row = minRow; row <= maxRow; row++) { >>> for (int col = minColumn; col <= maxColumn; col++) { >>> TableColumnBase column = getColumn(col); >>> select(row, column); >>> } >>> } >>> >>> With the first API option (all ints), I am lacking the getColumn(..) >>> method to convert column indices to TableColumn instances. >>> With the second API option, I have no way of converting the two >>> TableColumnBase instances back into ints, and then I have the same >>> problem as with the first API. >>> >>> Therefore, my options as far as I can see (and I'm happy to be wrong) >>> are to either not introduce this API and keep the performance >>> poor (for >>> now - hopefully we'll come up with a better solution in time) or to >>> introduce one of the two methods above and then have a no-op >>> implementation clearly documented in TableSelectionModel (with >>> the two >>> existing implementations in JavaFX 8.0 both implementing a far >>> more high >>> performance approach than we have currently). >>> >>> Thoughts? >>> -- Jonathan >>> >>> >>> >> >
