-
Notifications
You must be signed in to change notification settings - Fork 127
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
Design new Widget
trait
#7
Comments
One other thing to discuss now: damage regions. Because of limitations in WebGPU, it is difficult to plumb that down to the compositor, but I also know from experience that if we don't enforce it, the logic will bit-rot. My current thinking is to render the whole scene, but use the damage region to blit only the changed parts to the final render texture for presentation. (see this Zulip thread for lots more detail about that blit) There is one fairly subtle consideration about damage regions that is new. If a container culls some of its children because they don't intersect the damage region, then the resulting scene fragment is not valid with a different damage region. I see two approaches:
I'm honestly not sure which of these two approaches is best. The second is more complex and puts a greater burden on widget implementations, but may be more efficient. |
Should we maybe open an issue for long-term exploration of layout mechanisms? So that when the time is ripe, we can try to improve upon the current Flutter style. |
I'm in favor of a somewhat more focused issue that specifically queries what layout ideas we can import from SwiftUI (including the fragment implemented in the previous xilem prototype). To my mind, that's primarily GeometryReader and named alignments. Another question I'm running into is how (and whether) to adapt the In Druid, Another possible direction is to have invalidation move into a new My inclination for now is to preserve Druid mechanisms as much as possible, and propose changes later. However, now is an opportunity to rethink things, which I don't want to entirely pass up. |
I had some ideas about the I have no idea whether Flutter or SwitfUI style layout is the better fit for Xilem. But either way, i am in favor of a I also think we should combine fn event(&mut self, cx: &mut EventCx) { //the event is part of the cx
match cx.event() {
RawEvent::MouseDown(mouse_event: &MouseEvent, cx: &mut MouseCx) => { // the MouseCx contains all methods of EventCx + some mouse specific ones
cx.set_active(true);
cx.set_handled();
}
RawEvent::MouseUp(mouse_event: &MouseEvent, cx: &mut MouseCx) => {
if cx.is_active() && cx.is_hot() {
cx.set_active(false);
// do something
}
}
RawEvent::BuildFocusChain(cx: &mut FocusChainCx) => {
cx.register_for_focus();
}
}
cx.set_active(true); // this would be a compiler error
let active = cx.is_active(); // while this would be ok
} In this case you could merge |
Hmm, I'm realizing this is a bit deeper water than I originally planned. Let me gather my thoughts a bit. First, I do think Second, I'd like to put tweaks to the layout protocol aside. I do think there is scope for improvement, but for the most part it feels separate. Flutter has a bunch of extra mechanisms (implicit size etc) on top of the core BoxConstraints mechanism, and we can sort through how much of that we want to copy verbatim, and how much we want to change. Third, I'm skeptical that My inclination here is to do a first cut that stays very close to the original Druid code, then consider changes from that. The closer we stay to the original trait, the less cognitive load there is to adapt widgets. That said, this is also an opportunity to make improvements without having to make big refactoring patches. |
As discussed in office hours, in terms of the events, I think separating it out when there are separate logic paths (like broadcast vs to one widget vs commands) would probably be easy to port, and would be conceptually easier when creating a new widget. New people, when creating widgets that contain widgets, may not realize the different possibilities and just propagate them blindly otherwise. And in terms of the other functions, I think it's most important to make it efficient in the case of large scroll lists, where the behavior of hidden widgets matters, but must be cheap. In my use case, a chat app, I need to process a lot of text fields in a scroll area, which is very computationally expensive when it comes to layout and sometimes painting. |
This is true for druid's event method, but if we combine event and lifecycle, this no longer holds. Many lifecycle events target multiple widgets. In my opinion the thing that unifies event and lifecycle is, that there is no need for interaction between child and parent to propagate the events correctly and therefore we can send them all in one method. I think it would be ideal to have methods for everything, every widget has to do and put the rest into event.
@jaredoconnell, could you explain the benefit of having seperate methods? I unfortunately did not have time to come to the office hours. So i hope i don't sound ignorant, but my understanding is, that this is actually the thing which made druid so elegant. You can just send your events to your children and their |
Closing this as the immediate issue is solved by #26. There will be another issue soon for followup. |
New tracy image:  New log tracing file: <details><summary>An overview of the new logs</summary> <p> ``` 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}: masonry::passes::update: RootWidget received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}: masonry::passes::update: SizedBox received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Prose{id=#1}: masonry::passes::update: Prose received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Label{id=#2}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}:VariableLabel{id=#4}: masonry::passes::update: VariableLabel received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}: masonry::passes::update: Button received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}:Label{id=#9}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}: masonry::passes::update: Button received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}:Label{id=#11}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}: masonry::passes::update: Button received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}:Label{id=#13}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}: masonry::passes::update: Button received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}:Label{id=#15}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}: masonry::passes::update: Portal received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}: masonry::passes::update: SizedBox received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Prose{id=#18}: masonry::passes::update: Prose received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Label{id=#19}: masonry::passes::update: Label received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}: masonry::passes::update: Flex received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}:VariableLabel{id=#21}: masonry::passes::update: VariableLabel received Update::WidgetAdded 14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#31}: masonry::passes::update: SizedBox received Update::WidgetAdded ``` </p> </details> This was originally an experiment into caching spans, but I determined that was non-viable due to the pass names.
An important step in building out the widget tree is to get the trait right. Further iteration is possible, but getting it basically right at this time will save hassle later.
The high-level approach is to start with the Druid Widget trait and make some changes.
The
data
argument goes away (as does being generic onT
). This is a natural consequence of the Xilem architecture.The
env
argument goes away, I think. This is actually a much bigger question, as we rely on env to retrieve style data, so probably deserves some thought about what will replace that.The
paint
method evolves to generate a piet-gpu scene fragment rather than drawing into a Piet render context.The trait also needs to update the accessibility tree. In the prototype Druid integration for AccessKit, there an
accessibility()
method that generates the node, but that's based on generating the entire tree every time rather than doing incremental update. This needs to be worked out in detail.As discussed in today's office hours, we will not be making significant changes to layout at this time, so those methods will basically be the same as Druid's existing Flutter-like mechanism.
The
update
methodRight now the
update
method isn't doing much, and if #6 gets adopted, it will do even less. In current Druid, one of its other roles is to respond toenv
changes (if theme or styling changes), but that may well go away as well.A potential role may be to update the accessibility tree, but likely that should be a separate method, as it should be lazy (only called when a screen reader is connected).
So one of the questions which needs to be decided is whether to keep the
update
method or remove it. An excellent way to help resolve it is to go through the existing use cases in Druid and see if any really motivate keeping it.The
lifecycle
methodWe can also consider combining
lifecycle
andevent
, as the motivation for having them separate may no longer be compelling. I consider this a lower priority, as I'm sure it's fine either way, but again if we do decide to change this from Druid, it would be less work to do it now than later.The text was updated successfully, but these errors were encountered: