`List` does not always propagate `ViewContextChanged`

[CryZe]

If the list is reasonably dynamic and the elements inside change, then ViewContextChanged is not always propagated to all the elements. I have yet to figure out when / why this happens.

[xarvic]

It should only be propagated to widgets, which changed their origin. For an example if your list contains 10 elements and you add one, only the last one should receive ViewContextChanged, but if you scroll the list every element should receive ViewContextChanged. If that's not the case, could give an example when it breaks?

[CryZe]

I haven't looked deeper into it, but I have a "list of settings" where each setting can be a different widget. And depending which "setting" you choose it changes the settings. So each row will have changing widgets. And I believe if it changes a widget that's when ViewContextChanged never happens, essentially breaking all the combo boxes. This also is a recent bug as they used to work.

https://user-images.githubusercontent.com/1451630/208157432-c00d3296-9b24-4ab0-bb8a-ab38c55722fd.mp4

[CryZe]

It looks like a WidgetPod has a wrong children_view_context_changed or view_context_changed. Removing the if origin != self.state.origin in WidgetPod::set_origin fixes the bug. Though that's probably not the correct fix.

So it seems like a Widget inside the WidgetPod gets created, but since the WidgetPod's origin doesn't change, it doesn't properly propagate the event? Maybe something like that? I don't fully understand what I'm looking at xD

That actually explains why the "Accuracy" combo box is never affected, as it slightly moves a bit.

[xarvic]

Yes that makes sense. I guess the correct fix would be setting the initial value of view_context_changed to true. That way every Widget receives ViewContextChanged at least once.

[xStrom]

I would like to reproduce this issue myself. Best would be a piece of code I could run as a Druid example.

I tried recreating this manually already but have failed at doing so. So if an example code can't be provided, I would at least like to better understand what is going on here.

From the video it seems that at first it works properly. Is that "Vertical" dropdown widget cached and/or hidden as opposed to recreated? To put it another way, does a good widget state get ruined, or does the 2nd instance widget creation run through a different code path?

[xStrom]

I took a deeper look at this and managed to reproduce the issue and thoroughly understand it. The short of it is that #2311 should indeed be the correct fix for this.

The problem is that WidgetState::parent_window_origin only gets set when handling LifeCycle::ViewContextChanged. In simple cases this is not a problem, which is also why this issue wasn't caught by looking at the existing examples.

It works with simple/static cases, because some ancestor widget's set_origin gets called and that widget's view_context_changed gets set to true. Then the LifeCycle::ViewContextChanged gets also sent to all of its descendants.

For the problem to occur we need all the ancestor widgets to be unchanged. So in the example above, the List has its origin set at the start and that's why it works the first time. The second time the List origin doesn't change and so it does not generate a LifeCycle::ViewContextChanged.

Additionally for the problem to occur we need the problematic widget to have an origin of Point::ORIGIN. So things like the accuracy dropdown in the video still work fine, because the set_origin called on them isn't Point::ORIGIN and so their view_context_changed gets set to true. Crucially this only ensures LifeCycle::ViewContextChanged to this widget and its descendants, but not its siblings.

So in the buggy case, we have an ancestor chain with unchanged origins (e.g. List still has its old origin) and the problematic widget itself has a genuine origin of Point::ORIGIN. No LifeCycle::ViewContextChanged ever arrives here because no widget in the whole tree leading up to this problematic widget requested it.

The solution of starting with view_context_changed set to true will solve this specific issue. This helps ensure that WidgetState::parent_window_origin gets properly set even if all the ancestors are already old & stable and the new widget has a genuine origin of Point::ORIGIN.

---

For the record I will also include a simple example program that will reproduce this issue.

#![windows_subsystem = "windows"]

use druid::im::Vector;
use druid::widget::{Button, Click, ControllerHost, List};
use druid::{
    AppLauncher, BoxConstraints, Data, Env, Event, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx,
    PaintCtx, Point, Size, UpdateCtx, Widget, WidgetPod, WindowDesc,
};

#[derive(Clone, Data)]
struct AppState {
    list_data: Vector<String>,
    btn_data: usize,
}

pub fn main() {
    let state = AppState {
        list_data: (0..2).map(|n| n.to_string()).collect(),
        btn_data: 0,
    };

    let window = WindowDesc::new(make_ui())
        .title("List origin bug")
        .window_size(Size {
            width: 350.0,
            height: 500.0,
        });

    AppLauncher::with_window(window)
        .log_to_console()
        .launch(state)
        .unwrap();
}

fn make_ui() -> impl Widget<AppState> {
    let root = Root::new();
    root
}

struct Root {
    master_button: WidgetPod<usize, ControllerHost<Button<usize>, Click<usize>>>,
    bug_list: WidgetPod<Vector<String>, List<String>>,
}

impl Root {
    fn new() -> Root {
        Root {
            master_button: WidgetPod::new(Button::new("Master").on_click(|_ctx, data, _env| {
                *data += 1;
                tracing::debug!("Master state changed to: {}", data);
            })),
            bug_list: WidgetPod::new(List::new(|| {
                Button::dynamic(|s: &String, _env| s.clone()).on_click(|ctx, data, _env| {
                    tracing::debug!("Button {} window origin: {:?}", data, ctx.window_origin());
                })
            })),
        }
    }
}

impl Widget<AppState> for Root {
    fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut AppState, env: &Env) {
        self.master_button
            .event(ctx, event, &mut data.btn_data, env);

        match data.btn_data % 2 {
            0 => {
                data.list_data = (data.btn_data..data.btn_data + 2)
                    .map(|n| n.to_string())
                    .collect();
            }
            1 => {
                data.list_data = Vector::new();
            }
            _ => unreachable!(),
        }

        self.bug_list.event(ctx, event, &mut data.list_data, env);
    }

    fn lifecycle(&mut self, ctx: &mut LifeCycleCtx, event: &LifeCycle, data: &AppState, env: &Env) {
        self.master_button
            .lifecycle(ctx, event, &data.btn_data, env);
        self.bug_list.lifecycle(ctx, event, &data.list_data, env);
    }

    fn update(&mut self, ctx: &mut UpdateCtx, _old_data: &AppState, data: &AppState, env: &Env) {
        self.master_button.update(ctx, &data.btn_data, env);
        self.bug_list.update(ctx, &data.list_data, env);
    }

    fn layout(
        &mut self,
        ctx: &mut LayoutCtx,
        bc: &BoxConstraints,
        data: &AppState,
        env: &Env,
    ) -> Size {
        self.master_button
            .layout(ctx, &bc.loosen(), &data.btn_data, env);
        self.master_button.set_origin(ctx, Point::new(0., 0.));

        self.bug_list
            .layout(ctx, &bc.loosen(), &data.list_data, env);
        self.bug_list.set_origin(ctx, Point::new(0., 50.));
        bc.constrain((350.0, 500.0))
    }

    fn paint(&mut self, ctx: &mut PaintCtx, data: &AppState, env: &Env) {
        self.master_button.paint(ctx, &data.btn_data, env);
        self.bug_list.paint(ctx, &data.list_data, env);
    }
}

[CryZe]

Thank you so much :)