slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
16.93k stars 565 forks source link

ScrollView scrolling doesn't always cause "mouse leave" events on child items #1107

Closed sztomi closed 2 years ago

sztomi commented 2 years ago

I've created a menubar that is largely based on the iot demo and I added it to a ScrollView. I noticed that when scrolling the view, the child items will get confused about their mouseover state. It looks like it's only the "mouse leave" event that is missing, because items that are scrolled under the mouse will be updated to appear "active".

https://user-images.githubusercontent.com/240594/160241894-92f76ce8-fe25-4f6f-8f43-f1169b4fb4e5.mp4

The code I used is largely copied from iot example, with slight modifications only:

export MenuItem := Rectangle {
  property <image> icon <=> i.source;
  property <string> name <=> t.text;
  property <bool> active;
  property <bool> is-sub;
  property <int> index;
  callback clicked <=> ma.clicked;

  background: active ? rgba(100%, 100%, 100%, 14%) : ma.has-hover ? rgba(100%, 100%, 100%, 9%) : transparent;
  ma := TouchArea {}
  HorizontalLayout {
    alignment: start;
    spacing: 6px;
    padding: 8px;
    padding-left: is-sub ? 30px : 10px;
    padding-right: 30px;
    i := Image {
      width: 14px;
      height: source.height * 1px;
    }
    t := Text {
      color: white;
      font-size: Skin.MediumFont;
    }
  }
}

export MenuBar := Rectangle {
  callback logout_clicked <=> logout.clicked;
  callback item_clicked(int);
  property <int> active: 0;
  property <length> viewport-height;
  property <[MenuItemData]> menu-items;

  background: Skin.menuBar;
  width: 200px;

  ScrollView {
    VerticalLayout {
      padding-left: 0px;
      padding-top: 35px;
      padding-right: 0px;
      padding-bottom: 12px;
      spacing: 8px;
      VerticalLayout {
        padding-left: 50px;
        padding-top: 0px;
        padding-right: 50px;
        padding-bottom: 54px;

        Image {
          source: @image-url("images/main-icon.png");
          height: source.height * 1px;
        }
      }

      for entry[idx] in root.menu-items : MenuItem {
        name: entry.title;
        index: idx;
        //icon: entry.icon;
        active: root.active == idx;
        is-sub: entry.is-sub;
        clicked => { root.item-clicked(self.index) }
      }

      Rectangle {}

      logout := MenuItem {
        name: "Logout";
        icon: @image-url("images/logout.png");
      }
    }
  }
}
sztomi commented 2 years ago

From what I can tell diving into the implementation of Flickable and ScrollView, this might require a bit of refactoring. The current architecture allows passing through the same event to children that parents received, but that doesn't help here: the child items of the scrollview don't need to react to a mousewheel event (that is something the parent container needs to handle and not pass on, which is what rightly happens currently). But the children need to know that they were moved, so the hover state has potentially changed.

ogoffart commented 2 years ago

The problem here is that we send the mouse event (and thus the exit event) when the mouse move, but we do not do that if the item itself is moved. The items that are under the mouse and might need to get the input event when they move are stored in Window::mouse_input_state

I see a few solution to solve this. But none of them is really great.

  1. We use a PropertyTracker in the MouseInputState that tracks the geometry of all items under the mouse, then if it becomes dirty, we do the same algorithm as what we do when we recieve a mouse event and pottentially re-send a fake mouse event to the items with the new relative coordinate (we can store the absolute location of the mouse in the MouseInputState). The problem is that the PropertyTracker might not be cheap, and so we might want to disable that for devices with touch screen that don't need it like MCU. Also this does not help if items that were not under the mouse are now moved under the mouse.

  2. We special-case the Flickable. We add code in the flickable that once it is moved, it will send the exist and enter event to the items that have the mouse (with the absolute location of the mouse in the MouseInputState). That would solve the problem for the flickable, but not if the items are moved for another reason.

  3. Before rendering each frame, we check that the list of items in the mouse_input_state is still accurate. But that it might also not be cheap CPU-wise to do that for each frame.

In both these solutions, we need to store the absolute cursor position in the MouseInputState, and we need to call the part of the code from process_mouse_input that sends the exit events.

sztomi commented 2 years ago

I think special-casing Flickable would be a reasonable 80% solution, at least until a better way is found.

Without knowing much about the internals of slint, would it be possible to introduce an artificial event that tells items that they were moved on the screen? That would be cheap on the Flickable side, and items in the container could decide how they want to handle it (if at all). For example, TouchAreas that read has-hover would be able to handle it. And I don't think the absolute mouse coordinates would necessarily have to be passed in this event, since that's something that can be queried and cached when needed.

ogoffart commented 2 years ago

I think special-casing Flickable would be a reasonable 80% solution, at least until a better way is found.

Yes, that's right.

would it be possible to introduce an artificial event that tells items that they were moved on the screen?

Sure it would. From the Flickable::input_event you have access from the WindowRc, from which you can call process_mouse_input or create a new function that sent the mouse exit events (Because calling process_mouse_input recursively might not be ideal)

levrik commented 2 years ago

@ogoffart I got it working somehow. It's enough for my project atm but it feels like a rather hacky solution. Would you be interested in a PR anyway?

https://user-images.githubusercontent.com/9491603/168012405-f40b14e6-e074-4f7c-a4ba-afe12130872b.mp4

ogoffart commented 2 years ago

Looks nice!

Yes we would be interested by a PR, thanks

levrik commented 2 years ago

@ogoffart Here's the draft PR: https://github.com/slint-ui/slint/pull/1263 I've put some open questions as I'm not fully familiar with the code base and all concepts so there might be better solutions to this.

sztomi commented 2 years ago

THANK YOU! :heart:

levrik commented 2 years ago

@sztomi You're welcome!