Skip to content

Virtual table improvements/fixes #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 19, 2018
Merged

Virtual table improvements/fixes #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ef03298
Fix error when there are no rows
DanTup Sep 18, 2018
f0b7bc4
Don't clone the list of data provided
DanTup Sep 18, 2018
683b72a
Update comments about perf
DanTup Sep 18, 2018
0ef3231
Remove duplicated code that wires up handlers a second time
DanTup Sep 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions lib/logging/logging.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ class LoggingScreen extends Screen {
// in the table, and only show the defaults?
loggingTable.onSelect
.listen((LogData selection) => logDetailsUI.data = selection);

serviceInfo.onConnectionAvailable.listen(_handleConnectionStart);
if (serviceInfo.hasConnection) {
_handleConnectionStart(serviceInfo.service);
}
serviceInfo.onConnectionClosed.listen(_handleConnectionStop);
}

CoreElement _createTableView() {
Expand Down Expand Up @@ -174,15 +168,13 @@ class LoggingScreen extends Screen {

List<LogData> data = <LogData>[];
void _log(LogData log) {
// TODO(devoncarew): make this much more efficient
// TODO(dantup): Maybe add to a small buffer and then after xms insert
// that full buffer into the list here to avoid a list rebuild on every single
// insert.
// Or maybe append to the end of the list and reverse index-based operations?

// Build a new list that has 1 item more (clamped at kMaxLogItemsLength)
// and insert this new item at the start, followed by the required number
// of items from the old data.
// of items from the old data. This is faster than insert(0, log).
//
// If this turns out to be too slow, we can make it faster (saving around
// 30-40% of the time) by just .add()ing to the list and having a flag on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems worth doing - appending to the list and reversing the way we index through it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey, I'll do in a separate PR 👍

// the table to reverse data for rendering ([length-index] when reading data).
final int totalItems = (data.length + 1).clamp(0, kMaxLogItemsLength);
data = List<LogData>(totalItems)
..[0] = log
Expand Down
9 changes: 5 additions & 4 deletions lib/tables.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class Table<T> extends Object with SetStateMixin {
if (anchorAlternatingRowsToBottom && differenceInRowCount % 2 == 1) {
_offsetRowColor = !_offsetRowColor;
}
this.rows = rows.toList();
this.rows = rows;

if (_thead == null) {
_thead = new CoreElement('thead')
Expand Down Expand Up @@ -198,18 +198,19 @@ class Table<T> extends Object with SetStateMixin {
int currentRowIndex = 0;

// Calculate the subset of rows to render based on scroll position.
final int totalRows = rows?.length ?? 0;
final int firstVisibleRow =
((element.scrollTop - _thead.offsetHeight) / rowHeight).floor();
final int numVisibleRows = (element.offsetHeight / rowHeight).ceil() + 1;
final int highestPossibleFirstRenderedRow =
rows == null ? 0 : rows.length - (numVisibleRows + 1);
(totalRows - (numVisibleRows + 1)).clamp(0, totalRows);
firstRenderedRowInclusive =
firstVisibleRow.clamp(0, highestPossibleFirstRenderedRow);
// Calculate the last rendered row. +2 is for:
// 1) because it's exclusive so needs to be one higher
// 2) because we need to render the extra partially-visible row
lastRenderedRowExclusive = (firstRenderedRowInclusive + numVisibleRows + 2)
.clamp(0, rows?.length ?? 0);
lastRenderedRowExclusive =
(firstRenderedRowInclusive + numVisibleRows + 2).clamp(0, totalRows);

// Add a spacer row to fill up the content off-screen.
final double spacerBeforeHeight = firstRenderedRowInclusive * rowHeight;
Expand Down