From ab7f7b3754ed90470e2d3d68d0d4f3cc4d40408c Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 21 Sep 2022 22:23:07 -0700 Subject: [PATCH 1/4] Added on_scroll to mouse_event_handler and fixed the uniform list scroll implementation --- .../gpui/src/elements/mouse_event_handler.rs | 11 ++++- crates/gpui/src/elements/uniform_list.rs | 47 ++++++++++++------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/crates/gpui/src/elements/mouse_event_handler.rs b/crates/gpui/src/elements/mouse_event_handler.rs index bb958ce99c2728eed3e0908ee040db5b0ccfaa29..5b3b9b13f6c77ae2f3fe2b517a3136e83729e4e0 100644 --- a/crates/gpui/src/elements/mouse_event_handler.rs +++ b/crates/gpui/src/elements/mouse_event_handler.rs @@ -7,7 +7,8 @@ use crate::{ platform::CursorStyle, scene::{ ClickRegionEvent, CursorRegion, DownOutRegionEvent, DownRegionEvent, DragRegionEvent, - HandlerSet, HoverRegionEvent, MoveRegionEvent, UpOutRegionEvent, UpRegionEvent, + HandlerSet, HoverRegionEvent, MoveRegionEvent, ScrollWheelRegionEvent, UpOutRegionEvent, + UpRegionEvent, }, DebugContext, Element, ElementBox, Event, EventContext, LayoutContext, MeasurementContext, MouseButton, MouseRegion, MouseState, PaintContext, RenderContext, SizeConstraint, View, @@ -122,6 +123,14 @@ impl MouseEventHandler { self } + pub fn on_scroll( + mut self, + handler: impl Fn(ScrollWheelRegionEvent, &mut EventContext) + 'static, + ) -> Self { + self.handlers = self.handlers.on_scroll(handler); + self + } + pub fn with_hoverable(mut self, is_hoverable: bool) -> Self { self.hoverable = is_hoverable; self diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 103cb00d8cc1f46d77d85928e95aea666c589bed..718a9fe8a242da71d512bb22d6d160a81a54589b 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -6,7 +6,8 @@ use crate::{ }, json::{self, json}, presenter::MeasurementContext, - ElementBox, RenderContext, ScrollWheelEvent, View, + scene::ScrollWheelRegionEvent, + ElementBox, MouseRegion, RenderContext, ScrollWheelEvent, View, }; use json::ToJson; use std::{cell::RefCell, cmp, ops::Range, rc::Rc}; @@ -50,6 +51,7 @@ pub struct UniformList { padding_top: f32, padding_bottom: f32, get_width_from_item: Option, + view_id: usize, } impl UniformList { @@ -77,6 +79,7 @@ impl UniformList { padding_top: 0., padding_bottom: 0., get_width_from_item: None, + view_id: cx.handle().id(), } } @@ -96,7 +99,7 @@ impl UniformList { } fn scroll( - &self, + state: UniformListState, _: Vector2F, mut delta: Vector2F, precise: bool, @@ -107,7 +110,7 @@ impl UniformList { delta *= 20.; } - let mut state = self.state.0.borrow_mut(); + let mut state = state.0.borrow_mut(); state.scroll_top = (state.scroll_top - delta.y()).max(0.0).min(scroll_max); cx.notify(); @@ -283,6 +286,28 @@ impl Element for UniformList { ) -> Self::PaintState { cx.scene.push_layer(Some(bounds)); + cx.scene.push_mouse_region( + MouseRegion::new::(self.view_id, 0, bounds).on_scroll({ + let scroll_max = layout.scroll_max; + let state = self.state.clone(); + move |ScrollWheelRegionEvent { + platform_event: + ScrollWheelEvent { + position, + delta, + precise, + .. + }, + .. + }, + cx| { + if !Self::scroll(state.clone(), position, delta, precise, scroll_max, cx) { + cx.propogate_event(); + } + } + }), + ); + let mut item_origin = bounds.origin() - vec2f( 0., @@ -300,7 +325,7 @@ impl Element for UniformList { fn dispatch_event( &mut self, event: &Event, - bounds: RectF, + _: RectF, _: RectF, layout: &mut Self::LayoutState, _: &mut Self::PaintState, @@ -311,20 +336,6 @@ impl Element for UniformList { handled = item.dispatch_event(event, cx) || handled; } - if let Event::ScrollWheel(ScrollWheelEvent { - position, - delta, - precise, - .. - }) = event - { - if bounds.contains_point(*position) - && self.scroll(*position, *delta, *precise, layout.scroll_max, cx) - { - handled = true; - } - } - handled } From f4d4ea41232cc55a9612ad6c66ca8983201274f5 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 21 Sep 2022 23:26:42 -0700 Subject: [PATCH 2/4] WIP fixing scrollable flex --- crates/gpui/src/elements/flex.rs | 141 +++++++++++++---------- crates/gpui/src/elements/uniform_list.rs | 2 +- 2 files changed, 81 insertions(+), 62 deletions(-) diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index e68810fb360527dd2e047d12c98f75632bac6105..bc3b352b6cb141e7d8d51e397db4ff47d1fb1aa0 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -1,10 +1,10 @@ -use std::{any::Any, f32::INFINITY, ops::Range}; +use std::{any::Any, cell::Cell, f32::INFINITY, ops::Range, rc::Rc}; use crate::{ json::{self, ToJson, Value}, presenter::MeasurementContext, Axis, DebugContext, Element, ElementBox, ElementStateHandle, Event, EventContext, - LayoutContext, MouseMovedEvent, PaintContext, RenderContext, ScrollWheelEvent, SizeConstraint, + LayoutContext, MouseRegion, PaintContext, RenderContext, ScrollWheelEvent, SizeConstraint, Vector2FExt, View, }; use pathfinder_geometry::{ @@ -13,7 +13,7 @@ use pathfinder_geometry::{ }; use serde_json::json; -#[derive(Default)] +#[derive(Default, Clone, Copy)] struct ScrollState { scroll_to: Option, scroll_position: f32, @@ -22,7 +22,7 @@ struct ScrollState { pub struct Flex { axis: Axis, children: Vec, - scroll_state: Option>, + scroll_state: Option<(ElementStateHandle>>, usize)>, } impl Flex { @@ -52,9 +52,15 @@ impl Flex { Tag: 'static, V: View, { - let scroll_state = cx.default_element_state::(element_id); - scroll_state.update(cx, |scroll_state, _| scroll_state.scroll_to = scroll_to); - self.scroll_state = Some(scroll_state); + let scroll_state_handle = + cx.default_element_state::>>(element_id); + let scroll_state_cell = scroll_state_handle.read(cx); + let mut scroll_state = scroll_state_cell.get(); + scroll_state.scroll_to = scroll_to; + scroll_state_cell.set(scroll_state); + + self.scroll_state = Some((scroll_state_handle, cx.handle().id())); + self } @@ -100,6 +106,38 @@ impl Flex { } } } + + fn handle_scroll( + e: ScrollWheelEvent, + axis: Axis, + scroll_state: Rc>, + remaining_space: f32, + ) -> bool { + let precise = e.precise; + let delta = e.delta; + if remaining_space < 0. { + let mut delta = match axis { + Axis::Horizontal => { + if delta.x() != 0. { + delta.x() + } else { + delta.y() + } + } + Axis::Vertical => delta.y(), + }; + if !precise { + delta *= 20.; + } + + let mut old_state = scroll_state.get(); + old_state.scroll_position -= delta; + scroll_state.set(old_state); + + return true; + } + return false; + } } impl Extend for Flex { @@ -202,9 +240,9 @@ impl Element for Flex { } if let Some(scroll_state) = self.scroll_state.as_ref() { - scroll_state.update(cx, |scroll_state, _| { - if let Some(scroll_to) = scroll_state.scroll_to.take() { - let visible_start = scroll_state.scroll_position; + scroll_state.0.update(cx, |scroll_state, _| { + if let Some(scroll_to) = scroll_state.get().scroll_to.take() { + let visible_start = scroll_state.get().scroll_position; let visible_end = visible_start + size.along(self.axis); if let Some(child) = self.children.get(scroll_to) { let child_start: f32 = self.children[..scroll_to] @@ -212,16 +250,20 @@ impl Element for Flex { .map(|c| c.size().along(self.axis)) .sum(); let child_end = child_start + child.size().along(self.axis); + + let mut old_state = scroll_state.get(); if child_start < visible_start { - scroll_state.scroll_position = child_start; + old_state.scroll_position = child_start; } else if child_end > visible_end { - scroll_state.scroll_position = child_end - size.along(self.axis); + old_state.scroll_position = child_end - size.along(self.axis); } + scroll_state.set(old_state); } } - scroll_state.scroll_position = - scroll_state.scroll_position.min(-remaining_space).max(0.); + let mut old_state = scroll_state.get(); + old_state.scroll_position = old_state.scroll_position.min(-remaining_space).max(0.); + scroll_state.set(old_state); }); } @@ -242,9 +284,30 @@ impl Element for Flex { cx.scene.push_layer(Some(bounds)); } + if let Some(scroll_state) = &self.scroll_state { + cx.scene.push_mouse_region( + MouseRegion::new::(scroll_state.1, 0, bounds) + .on_scroll({ + let axis = self.axis; + let scroll_state = scroll_state.0.read(cx).clone(); + move |e, cx| { + if Self::handle_scroll( + e.platform_event, + axis, + scroll_state.clone(), + remaining_space, + ) { + cx.propogate_event(); + } + } + }) + .on_move(|_, _| { /* Eat move events so they don't propogate */ }), + ); + } + let mut child_origin = bounds.origin(); if let Some(scroll_state) = self.scroll_state.as_ref() { - let scroll_position = scroll_state.read(cx).scroll_position; + let scroll_position = scroll_state.0.read(cx).get().scroll_position; match self.axis { Axis::Horizontal => child_origin.set_x(child_origin.x() - scroll_position), Axis::Vertical => child_origin.set_y(child_origin.y() - scroll_position), @@ -278,9 +341,9 @@ impl Element for Flex { fn dispatch_event( &mut self, event: &Event, - bounds: RectF, _: RectF, - remaining_space: &mut Self::LayoutState, + _: RectF, + _: &mut Self::LayoutState, _: &mut Self::PaintState, cx: &mut EventContext, ) -> bool { @@ -288,50 +351,6 @@ impl Element for Flex { for child in &mut self.children { handled = child.dispatch_event(event, cx) || handled; } - if !handled { - if let &Event::ScrollWheel(ScrollWheelEvent { - position, - delta, - precise, - .. - }) = event - { - if *remaining_space < 0. && bounds.contains_point(position) { - if let Some(scroll_state) = self.scroll_state.as_ref() { - scroll_state.update(cx, |scroll_state, cx| { - let mut delta = match self.axis { - Axis::Horizontal => { - if delta.x() != 0. { - delta.x() - } else { - delta.y() - } - } - Axis::Vertical => delta.y(), - }; - if !precise { - delta *= 20.; - } - - scroll_state.scroll_position -= delta; - - handled = true; - cx.notify(); - }); - } - } - } - } - - if !handled { - if let &Event::MouseMoved(MouseMovedEvent { position, .. }) = event { - // If this is a scrollable flex, and the mouse is over it, eat the scroll event to prevent - // propogating it to the element below. - if self.scroll_state.is_some() && bounds.contains_point(position) { - handled = true; - } - } - } handled } diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 718a9fe8a242da71d512bb22d6d160a81a54589b..6bc35c06922638c8221805d7a945b0c5f747ec65 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -287,7 +287,7 @@ impl Element for UniformList { cx.scene.push_layer(Some(bounds)); cx.scene.push_mouse_region( - MouseRegion::new::(self.view_id, 0, bounds).on_scroll({ + MouseRegion::new::(self.view_id, 0, visible_bounds).on_scroll({ let scroll_max = layout.scroll_max; let state = self.state.clone(); move |ScrollWheelRegionEvent { From dd7259c83288cb3cd5de5ae4bf4a35dfbb254447 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 22 Sep 2022 09:35:52 -0700 Subject: [PATCH 3/4] Finished fixing flex scrolls --- crates/gpui/src/elements/flex.rs | 115 +++++++++++++------------------ 1 file changed, 47 insertions(+), 68 deletions(-) diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index bc3b352b6cb141e7d8d51e397db4ff47d1fb1aa0..227f946ac6529e9d4e33f8acb5c535cc1805699d 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -4,8 +4,7 @@ use crate::{ json::{self, ToJson, Value}, presenter::MeasurementContext, Axis, DebugContext, Element, ElementBox, ElementStateHandle, Event, EventContext, - LayoutContext, MouseRegion, PaintContext, RenderContext, ScrollWheelEvent, SizeConstraint, - Vector2FExt, View, + LayoutContext, PaintContext, RenderContext, SizeConstraint, Vector2FExt, View, }; use pathfinder_geometry::{ rect::RectF, @@ -13,16 +12,16 @@ use pathfinder_geometry::{ }; use serde_json::json; -#[derive(Default, Clone, Copy)] +#[derive(Default)] struct ScrollState { - scroll_to: Option, - scroll_position: f32, + scroll_to: Cell>, + scroll_position: Cell, } pub struct Flex { axis: Axis, children: Vec, - scroll_state: Option<(ElementStateHandle>>, usize)>, + scroll_state: Option<(ElementStateHandle>, usize)>, } impl Flex { @@ -52,15 +51,9 @@ impl Flex { Tag: 'static, V: View, { - let scroll_state_handle = - cx.default_element_state::>>(element_id); - let scroll_state_cell = scroll_state_handle.read(cx); - let mut scroll_state = scroll_state_cell.get(); - scroll_state.scroll_to = scroll_to; - scroll_state_cell.set(scroll_state); - - self.scroll_state = Some((scroll_state_handle, cx.handle().id())); - + let scroll_state = cx.default_element_state::>(element_id); + scroll_state.read(cx).scroll_to.set(scroll_to); + self.scroll_state = Some((scroll_state, cx.handle().id())); self } @@ -106,38 +99,6 @@ impl Flex { } } } - - fn handle_scroll( - e: ScrollWheelEvent, - axis: Axis, - scroll_state: Rc>, - remaining_space: f32, - ) -> bool { - let precise = e.precise; - let delta = e.delta; - if remaining_space < 0. { - let mut delta = match axis { - Axis::Horizontal => { - if delta.x() != 0. { - delta.x() - } else { - delta.y() - } - } - Axis::Vertical => delta.y(), - }; - if !precise { - delta *= 20.; - } - - let mut old_state = scroll_state.get(); - old_state.scroll_position -= delta; - scroll_state.set(old_state); - - return true; - } - return false; - } } impl Extend for Flex { @@ -241,8 +202,8 @@ impl Element for Flex { if let Some(scroll_state) = self.scroll_state.as_ref() { scroll_state.0.update(cx, |scroll_state, _| { - if let Some(scroll_to) = scroll_state.get().scroll_to.take() { - let visible_start = scroll_state.get().scroll_position; + if let Some(scroll_to) = scroll_state.scroll_to.take() { + let visible_start = scroll_state.scroll_position.get(); let visible_end = visible_start + size.along(self.axis); if let Some(child) = self.children.get(scroll_to) { let child_start: f32 = self.children[..scroll_to] @@ -250,20 +211,23 @@ impl Element for Flex { .map(|c| c.size().along(self.axis)) .sum(); let child_end = child_start + child.size().along(self.axis); - - let mut old_state = scroll_state.get(); if child_start < visible_start { - old_state.scroll_position = child_start; + scroll_state.scroll_position.set(child_start); } else if child_end > visible_end { - old_state.scroll_position = child_end - size.along(self.axis); + scroll_state + .scroll_position + .set(child_end - size.along(self.axis)); } - scroll_state.set(old_state); } } - let mut old_state = scroll_state.get(); - old_state.scroll_position = old_state.scroll_position.min(-remaining_space).max(0.); - scroll_state.set(old_state); + scroll_state.scroll_position.set( + scroll_state + .scroll_position + .get() + .min(-remaining_space) + .max(0.), + ); }); } @@ -286,28 +250,43 @@ impl Element for Flex { if let Some(scroll_state) = &self.scroll_state { cx.scene.push_mouse_region( - MouseRegion::new::(scroll_state.1, 0, bounds) + crate::MouseRegion::new::(scroll_state.1, 0, bounds) .on_scroll({ - let axis = self.axis; let scroll_state = scroll_state.0.read(cx).clone(); + let axis = self.axis; move |e, cx| { - if Self::handle_scroll( - e.platform_event, - axis, - scroll_state.clone(), - remaining_space, - ) { + if remaining_space < 0. { + let mut delta = match axis { + Axis::Horizontal => { + if e.delta.x() != 0. { + e.delta.x() + } else { + e.delta.y() + } + } + Axis::Vertical => e.delta.y(), + }; + if !e.precise { + delta *= 20.; + } + + scroll_state + .scroll_position + .set(scroll_state.scroll_position.get() - delta); + + cx.notify(); + } else { cx.propogate_event(); } } }) - .on_move(|_, _| { /* Eat move events so they don't propogate */ }), - ); + .on_move(|_, _| { /* Capture move events */ }), + ) } let mut child_origin = bounds.origin(); if let Some(scroll_state) = self.scroll_state.as_ref() { - let scroll_position = scroll_state.0.read(cx).get().scroll_position; + let scroll_position = scroll_state.0.read(cx).scroll_position.get(); match self.axis { Axis::Horizontal => child_origin.set_x(child_origin.x() - scroll_position), Axis::Vertical => child_origin.set_y(child_origin.y() - scroll_position), From 4761898d9b14765f5658ce987b495920a8c2f246 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 22 Sep 2022 10:31:29 -0700 Subject: [PATCH 4/4] removed the last dispatch_event I could find --- crates/gpui/src/elements/list.rs | 39 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 57436c256bd9ee241dc78d07206ba8c3655e2c54..d752a52a1666110a7fc1cc2160988aac86429212 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -5,8 +5,8 @@ use crate::{ }, json::json, presenter::MeasurementContext, - DebugContext, Element, ElementBox, ElementRc, Event, EventContext, LayoutContext, PaintContext, - RenderContext, ScrollWheelEvent, SizeConstraint, View, ViewContext, + DebugContext, Element, ElementBox, ElementRc, Event, EventContext, LayoutContext, MouseRegion, + PaintContext, RenderContext, SizeConstraint, View, ViewContext, }; use std::{cell::RefCell, collections::VecDeque, ops::Range, rc::Rc}; use sum_tree::{Bias, SumTree}; @@ -263,6 +263,22 @@ impl Element for List { ) { cx.scene.push_layer(Some(bounds)); + cx.scene + .push_mouse_region(MouseRegion::new::(10, 0, bounds).on_scroll({ + let state = self.state.clone(); + let height = bounds.height(); + let scroll_top = scroll_top.clone(); + move |e, cx| { + state.0.borrow_mut().scroll( + &scroll_top, + height, + e.platform_event.delta, + e.platform_event.precise, + cx, + ) + } + })); + let state = &mut *self.state.0.borrow_mut(); for (mut element, origin) in state.visible_elements(bounds, scroll_top) { element.paint(origin, visible_bounds, cx); @@ -312,20 +328,6 @@ impl Element for List { drop(cursor); state.items = new_items; - if let Event::ScrollWheel(ScrollWheelEvent { - position, - delta, - precise, - .. - }) = event - { - if bounds.contains_point(*position) - && state.scroll(scroll_top, bounds.height(), *delta, *precise, cx) - { - handled = true; - } - } - handled } @@ -527,7 +529,7 @@ impl StateInner { mut delta: Vector2F, precise: bool, cx: &mut EventContext, - ) -> bool { + ) { if !precise { delta *= 20.; } @@ -554,9 +556,6 @@ impl StateInner { let visible_range = self.visible_range(height, scroll_top); self.scroll_handler.as_mut().unwrap()(visible_range, cx); } - cx.notify(); - - true } fn scroll_top(&self, logical_scroll_top: &ListOffset) -> f32 {