Skip to content

Add Multi-Select #835

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

johannesherschel
Copy link

This PR adds support for multi-select.

Dear ImGui 1.91.0 added a multi-select system for multi-selection idioms such as Shift/Ctrl + mouse/keyboard, box-selection and more. See the Dear ImGui wiki for details. This PR adds the corresponding bindings.

Additions and changes

Multi-select is used with a builder, and there are shortcut functions for common cases. MultiSelectFlags can be used for configuration. Tokens are used to interact with a multi-selection, including access to MultiSelectIo structs created by Dear ImGui, which contain all information about selection changes as well as additional details. Apart from the basic bindings for the Dear ImGui API, MultiSelectIo also includes an iterator over selection changes for individual items (RequestItems), which is mainly intended to simplify user implementations of SelectionStorage.

Selection data is maintained by user code. The SelectionStorage trait is used for storage types for selection data. SelectionBasicStorage is a self-contained implementation provided by Dear ImGui, and SelectionExternalStorage can wrap a storage provided by the user.

This PR also includes an addition to ListClipper to support multi-select with clipping, and a fix for empty ImVectors required by the multi-select implementation.

Lastly, I updated the Cargo.toml files of imgui and imgui-sys to actually use the MSRV set in the workspace. Rust version 1.81.0 introduced a change for extern "C" functions so that uncaught panics now cause an abort instead of UB. The callbacks in SelectionBasicStorage and SelectionExternalStorage rely on this behavior for soundness.

Progress of implementation

The core multi-select system is fully functional. There are a few details left to add or improve, but I wanted to open this as a draft PR to get feedback on the API and implementation. In particular, I would appreciate feedback on these issues:

  • The current implementation does not allow nested multi-select, which results from the pointer lifetimes given by Dear ImGui for BeginMultiSelect and EndMultiSelect. A nested BeginMultiSelect would invalidate the pointer returned from the previous one. I don't know if there is a good way to encode this with Rust lifetimes, so it is checked dynamically with Ui::multi_select_lock. The other lifetime limits (EndMultiSelect and end of frame) are enforced statically. We could allow nesting by just cloning the returned structs instead of referencing them, but for ImGuiMultiSelectIO::RangeSrcReset, the user actually needs write access.
  • The callbacks for SelectionBasicStorage and SelectionExternalStorage unfortunately require a 'static lifetime. This is currently required for soundness, but also prevents some valid uses, so a sound way to relax these bounds would be desirable.
  • ImGuiSelectionBasicStorage stores ImGuiIDs, but uses them as plain ints. The current imgui-rs bindings for ImGuiID don't allow simple casts, which makes ids hard to use here, so SelectionBasicStorage simply uses u32 instead.

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