Implement change detection in egui through Deref/DerefMut traits. #7735
+153
−16
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #7734. This PR modifies the widgets to provide a non-allocating, zero-cost abstraction to better integrate with Bevy but stay backwards API compatible and generate the same code.
The core of the problem is that widgets take
&mut state. Let's look at e.g. the checkbox. The constructor takeschecked: &'a mut booland returnsCheckbox<'a>. In Bevy, modification (DerefMut) has an action, it marks the state as changed. So the standard practice is to resort to tricks such as:This prevents always marking the state as changed which would happen if we unconditionally took the
&mut state. This PR modifies the code to passDerefMut<Target = T>instead of&mut T. We then callderef()when we read the value andderef_mut()when we write it. That way reads don't mark the state as changed. These are implicitely called when we do*valueso we don't even have to change the widget implementation.I've changed the function signature for checkbox() to be DerefMut. The stored state went from:
To:
I've done the same with
DragValue,Slider,TextEditand the simpler functions inUisuch astoggle_value(),radio_value(),selectable_value(),drag_angle(),drag_angle_tau()and the colors functions.I will admit that in addition to integrating seamlessly with Bevy's ECS I have a more sinister motive—I want to layer a caching system which allows for variable frame rate, not using the CPU when neither the data nor the user input changed. Something like SwiftUI and Xilem. I think a caching immediate-mode renderer can be built piecemeal from a plain immediate-mode UI framework like egui.
Again, I want to underline that the idea is that the API surface stays the same, that the generated code stays identical and the abstraction that's introduced does not allocate.
Q&A
The internal complexity is small, while the benefit for advanced use cases is significant.
AsMut<T>is a fundamental Rust trait. It's essentially a contract "I can give you a mutable reference to the inner T".Open Questions
I've kept all the same functions with the same signature (e.g.
toggle_value(state: &mut bool)) but added new functions with suffix_state(toggle_value_state(state: DerefMut<Target = bool>)). This is becauseDerefMutis notCopyso this code would break:The second call would trigger "value used after moving". It could be fixed with reborrowing:
But that's ugly and would break existing call sites. I opted for adding
_stateas a suffix but that's long and doesn't describe the purpose well.DerefMutvsAsMutThe latter doesn't participate in Deref coercion and would be a bit more involved to get to work.
#[inline]It would make sense to add
#[inline]to the generic _state functions to make sure this is literally zero cost. I don't know what the project conventions around this are.To Do
Popup currently has an enum
OpenKindof open states:Open,Closed,Bool(&mut bool),Memory(...). It would make sense to introduceState(Box<dyn DerefMut<Target = bool>>)but that's trivial, I didn't use it in my code and wanted to check the direction before continuing. I'd argue thatWindowwould also benefit from using the same enum instead of the currentOption<&mut bool>, whereNone==Closed.mainThe branch doesn't include the latest changes on
mainso if the direction is good, I'll rebase it.I don't know where. The example that I have is very much Bevy related:
On my machine,
./scripts/checkfails. I probably took the main branch at the wrong time. But no new failures are introduced.However there are no tests to check that the change was triggered only when needed. This shold be addressed.