Skip to content
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.

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

View widgets: trait revision, reliable keys #164

Merged
merged 8 commits into from
Feb 23, 2021
Merged

View widgets: trait revision, reliable keys #164

merged 8 commits into from
Feb 23, 2021

Conversation

dhardy
Copy link
Collaborator

@dhardy dhardy commented Feb 23, 2021

This replaces the Accessor trait with SingleData, SingleDataMut, ListData, ListDataMut. In a way these are simpler; they also have a type of iterator (a ridiculous vec generator until GATs allow real iterators).

Keys are now the same for a list and its filtered version, so list selection now works properly on filtered lists.

Also moves derive(Widget) before its attributes (avoids a warning with recent Rustc).


This code needs significant optimisation (some enabled by GATs, some to do with filtering, probably some other opts too), but appears to work fine other than perf.

One notable possibility: update_widgets gets called whenever scrolling occurs, which may be multiple times per frame — especially since rendering appears to only happen after event handling is finished. With fast scrolling this can essentially block drawing until the mouse stops moving. We could try solving this by only calling update_widgets after receiving all events (via ManagerState::update), but in this case state would be incorrect if other event handlers referring to the list's children somehow get called before update.

@dhardy
Copy link
Collaborator Author

dhardy commented Feb 23, 2021

Under further testing with release optimisations things appear sufficiently fast that drawing is not blocked even with rapid scrolling via the scrollbar, so it may be the update_widgets performance isn't a big issue.

Even with release opts, filtering is slow with >10'000 items, so optimisations are still desired for the filter (possibly only filtering entries during iteration/retrieval, though this requires we need to use an upper-bound when calling len or some such — possible since even the scrollbar size is independent of len above some threshold).

@dhardy dhardy mentioned this pull request Feb 23, 2021
4 tasks
@dhardy dhardy merged commit cec212e into master Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant