From 7ab5b4a170f0cb158f1d5d91effde37e4d9809cc Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 23 Jan 2017 07:26:00 -0800 Subject: [PATCH] servo: Merge #14141 - Implement home end key scrolling (from samuknet:home-end-key-scroll2); r=glennw * Refactor all scroll related code to use a new `ScrollLocation` struct which can either be a `delta` (as before) or a `Start` or `End` request, to represent the desire to scroll to the start and end of the page. Effectively, everywhere a delta was used, there is now a `ScrollLocation` struct instead. * Add key press listeners for HOME and END keys so as to cause a scroll to be queued with `ScrollLocation::Start` (in HOME case) or `ScrollLocation::End` (in END case). * These changes depend on added support for the new `ScrollLocation` in webrender and webrender_traits. See https://github.com/servo/webrender/pull/540. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ x] These changes fix #13082 (github issue number if applicable). - [ ] There are tests for these changes OR - [x] These changes do not require tests because scrolling I/O Source-Repo: https://github.com/servo/servo Source-Revision: 1706ffd6e5a02f26f69970b3b41536a8a85ef6fe --- servo/Cargo.lock | 2 + servo/components/compositing/compositor.rs | 97 +++++++++++++++------- servo/components/compositing/windowing.rs | 3 +- servo/ports/cef/Cargo.toml | 5 ++ servo/ports/cef/browser_host.rs | 3 +- servo/ports/cef/lib.rs | 1 + servo/ports/glutin/Cargo.toml | 4 + servo/ports/glutin/lib.rs | 2 + servo/ports/glutin/window.rs | 48 +++++++---- 9 files changed, 118 insertions(+), 47 deletions(-) diff --git a/servo/Cargo.lock b/servo/Cargo.lock index eac25e165965..1cd9a8192a05 100644 --- a/servo/Cargo.lock +++ b/servo/Cargo.lock @@ -674,6 +674,7 @@ dependencies = [ "servo_geometry 0.0.1", "servo_url 0.0.1", "style_traits 0.0.1", + "webrender_traits 0.11.0 (git+https://github.com/servo/webrender)", "x11 2.11.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1037,6 +1038,7 @@ dependencies = [ "servo_url 0.0.1", "style_traits 0.0.1", "user32-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "webrender_traits 0.11.0 (git+https://github.com/servo/webrender)", "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "x11 2.11.0 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/servo/components/compositing/compositor.rs b/servo/components/compositing/compositor.rs index bc3e0a3c4362..49b92f8e1711 100644 --- a/servo/components/compositing/compositor.rs +++ b/servo/components/compositing/compositor.rs @@ -39,7 +39,7 @@ use style_traits::viewport::ViewportConstraints; use time::{precise_time_ns, precise_time_s}; use touch::{TouchHandler, TouchAction}; use webrender; -use webrender_traits::{self, ScrollEventPhase, ServoScrollRootId, LayoutPoint}; +use webrender_traits::{self, ScrollEventPhase, ServoScrollRootId, LayoutPoint, ScrollLocation}; use windowing::{self, MouseWindowEvent, WindowEvent, WindowMethods, WindowNavigateMsg}; #[derive(Debug, PartialEq)] @@ -227,8 +227,8 @@ pub struct IOCompositor { struct ScrollZoomEvent { /// Change the pinch zoom level by this factor magnification: f32, - /// Scroll by this offset - delta: TypedPoint2D, + /// Scroll by this offset, or to Start or End + scroll_location: ScrollLocation, /// Apply changes to the frame at this location cursor: TypedPoint2D, /// The scroll event phase. @@ -1027,7 +1027,10 @@ impl IOCompositor { match self.touch_handler.on_touch_move(identifier, point) { TouchAction::Scroll(delta) => { match point.cast() { - Some(point) => self.on_scroll_window_event(delta, point), + Some(point) => self.on_scroll_window_event(ScrollLocation::Delta( + webrender_traits::LayerPoint::from_untyped( + &delta.to_untyped())), + point), None => error!("Point cast failed."), } } @@ -1035,7 +1038,8 @@ impl IOCompositor { let cursor = TypedPoint2D::new(-1, -1); // Make sure this hits the base layer. self.pending_scroll_zoom_events.push(ScrollZoomEvent { magnification: magnification, - delta: scroll_delta, + scroll_location: ScrollLocation::Delta(webrender_traits::LayerPoint::from_untyped( + &scroll_delta.to_untyped())), cursor: cursor, phase: ScrollEventPhase::Move(true), event_count: 1, @@ -1096,7 +1100,7 @@ impl IOCompositor { } fn on_scroll_window_event(&mut self, - delta: TypedPoint2D, + scroll_location: ScrollLocation, cursor: TypedPoint2D) { let event_phase = match (self.scroll_in_progress, self.in_scroll_transaction) { (false, None) => ScrollEventPhase::Start, @@ -1107,7 +1111,7 @@ impl IOCompositor { self.in_scroll_transaction = Some(Instant::now()); self.pending_scroll_zoom_events.push(ScrollZoomEvent { magnification: 1.0, - delta: delta, + scroll_location: scroll_location, cursor: cursor, phase: event_phase, event_count: 1, @@ -1115,12 +1119,12 @@ impl IOCompositor { } fn on_scroll_start_window_event(&mut self, - delta: TypedPoint2D, + scroll_location: ScrollLocation, cursor: TypedPoint2D) { self.scroll_in_progress = true; self.pending_scroll_zoom_events.push(ScrollZoomEvent { magnification: 1.0, - delta: delta, + scroll_location: scroll_location, cursor: cursor, phase: ScrollEventPhase::Start, event_count: 1, @@ -1128,12 +1132,12 @@ impl IOCompositor { } fn on_scroll_end_window_event(&mut self, - delta: TypedPoint2D, + scroll_location: ScrollLocation, cursor: TypedPoint2D) { self.scroll_in_progress = false; self.pending_scroll_zoom_events.push(ScrollZoomEvent { magnification: 1.0, - delta: delta, + scroll_location: scroll_location, cursor: cursor, phase: ScrollEventPhase::End, event_count: 1, @@ -1146,14 +1150,34 @@ impl IOCompositor { // Batch up all scroll events into one, or else we'll do way too much painting. let mut last_combined_event: Option = None; for scroll_event in self.pending_scroll_zoom_events.drain(..) { - let this_delta = scroll_event.delta; let this_cursor = scroll_event.cursor; + + let this_delta = match scroll_event.scroll_location { + ScrollLocation::Delta(delta) => delta, + ScrollLocation::Start | ScrollLocation::End => { + // If this is an event which is scrolling to the start or end of the page, + // disregard other pending events and exit the loop. + last_combined_event = Some(scroll_event); + break; + } + }; + if let Some(combined_event) = last_combined_event { if combined_event.phase != scroll_event.phase { - let delta = (combined_event.delta / self.scale).to_untyped(); + let combined_delta = match combined_event.scroll_location { + ScrollLocation::Delta(delta) => delta, + ScrollLocation::Start | ScrollLocation::End => { + // If this is an event which is scrolling to the start or end of the page, + // disregard other pending events and exit the loop. + last_combined_event = Some(scroll_event); + break; + } + }; + let delta = (TypedPoint2D::from_untyped(&combined_delta.to_untyped()) / self.scale) + .to_untyped(); + let delta = webrender_traits::LayerPoint::from_untyped(&delta); let cursor = (combined_event.cursor.to_f32() / self.scale).to_untyped(); - let delta = webrender_traits::LayerPoint::from_untyped(&delta); let location = webrender_traits::ScrollLocation::Delta(delta); let cursor = webrender_traits::WorldPoint::from_untyped(&cursor); self.webrender_api.scroll(location, cursor, combined_event.phase); @@ -1165,7 +1189,8 @@ impl IOCompositor { (last_combined_event @ &mut None, _) => { *last_combined_event = Some(ScrollZoomEvent { magnification: scroll_event.magnification, - delta: this_delta, + scroll_location: ScrollLocation::Delta(webrender_traits::LayerPoint::from_untyped( + &this_delta.to_untyped())), cursor: this_cursor, phase: scroll_event.phase, event_count: 1, @@ -1177,30 +1202,41 @@ impl IOCompositor { // fling. This causes events to get bunched up occasionally, causing // nasty-looking "pops". To mitigate this, during a fling we average // deltas instead of summing them. - let old_event_count = - ScaleFactor::new(last_combined_event.event_count as f32); - last_combined_event.event_count += 1; - let new_event_count = - ScaleFactor::new(last_combined_event.event_count as f32); - last_combined_event.delta = - (last_combined_event.delta * old_event_count + this_delta) / - new_event_count; + if let ScrollLocation::Delta(delta) = last_combined_event.scroll_location { + let old_event_count = + ScaleFactor::new(last_combined_event.event_count as f32); + last_combined_event.event_count += 1; + let new_event_count = + ScaleFactor::new(last_combined_event.event_count as f32); + last_combined_event.scroll_location = ScrollLocation::Delta( + (delta * old_event_count + this_delta) / + new_event_count); + } } (&mut Some(ref mut last_combined_event), _) => { - last_combined_event.delta = last_combined_event.delta + this_delta; - last_combined_event.event_count += 1 + if let ScrollLocation::Delta(delta) = last_combined_event.scroll_location { + last_combined_event.scroll_location = ScrollLocation::Delta(delta + this_delta); + last_combined_event.event_count += 1 + } } } } // TODO(gw): Support zoom (WR issue #28). if let Some(combined_event) = last_combined_event { - let delta = (combined_event.delta / self.scale).to_untyped(); - let delta = webrender_traits::LayoutPoint::from_untyped(&delta); + let scroll_location = match combined_event.scroll_location { + ScrollLocation::Delta(delta) => { + let scaled_delta = (TypedPoint2D::from_untyped(&delta.to_untyped()) / self.scale) + .to_untyped(); + let calculated_delta = webrender_traits::LayoutPoint::from_untyped(&scaled_delta); + ScrollLocation::Delta(calculated_delta) + }, + // Leave ScrollLocation unchanged if it is Start or End location. + sl @ ScrollLocation::Start | sl @ ScrollLocation::End => sl, + }; let cursor = (combined_event.cursor.to_f32() / self.scale).to_untyped(); - let location = webrender_traits::ScrollLocation::Delta(delta); let cursor = webrender_traits::WorldPoint::from_untyped(&cursor); - self.webrender_api.scroll(location, cursor, combined_event.phase); + self.webrender_api.scroll(scroll_location, cursor, combined_event.phase); self.waiting_for_results_of_scroll = true } @@ -1295,7 +1331,7 @@ impl IOCompositor { fn on_pinch_zoom_window_event(&mut self, magnification: f32) { self.pending_scroll_zoom_events.push(ScrollZoomEvent { magnification: magnification, - delta: TypedPoint2D::zero(), // TODO: Scroll to keep the center in view? + scroll_location: ScrollLocation::Delta(TypedPoint2D::zero()), // TODO: Scroll to keep the center in view? cursor: TypedPoint2D::new(-1, -1), // Make sure this hits the base layer. phase: ScrollEventPhase::Move(true), event_count: 1, @@ -1693,6 +1729,7 @@ impl IOCompositor { } } + /// Why we performed a composite. This is used for debugging. #[derive(Copy, Clone, PartialEq, Debug)] pub enum CompositingReason { diff --git a/servo/components/compositing/windowing.rs b/servo/components/compositing/windowing.rs index ca34aeddd5b9..39b117c85b6d 100644 --- a/servo/components/compositing/windowing.rs +++ b/servo/components/compositing/windowing.rs @@ -16,6 +16,7 @@ use servo_geometry::ScreenPx; use servo_url::ServoUrl; use std::fmt::{Debug, Error, Formatter}; use style_traits::cursor::Cursor; +use webrender_traits::ScrollLocation; #[derive(Clone)] pub enum MouseWindowEvent { @@ -62,7 +63,7 @@ pub enum WindowEvent { Touch(TouchEventType, TouchId, TypedPoint2D), /// Sent when the user scrolls. The first point is the delta and the second point is the /// origin. - Scroll(TypedPoint2D, TypedPoint2D, TouchEventType), + Scroll(ScrollLocation, TypedPoint2D, TouchEventType), /// Sent when the user zooms. Zoom(f32), /// Simulated "pinch zoom" gesture for non-touch platforms (e.g. ctrl-scrollwheel). diff --git a/servo/ports/cef/Cargo.toml b/servo/ports/cef/Cargo.toml index 3ca91e933f8d..16d1396e933f 100644 --- a/servo/ports/cef/Cargo.toml +++ b/servo/ports/cef/Cargo.toml @@ -35,6 +35,11 @@ servo_geometry = {path = "../../components/geometry"} servo_url = {path = "../../components/url"} style_traits = {path = "../../components/style_traits"} +[dependencies.webrender_traits] +git = "https://github.com/servo/webrender" +default-features = false +features = ["serde_derive", "ipc"] + [target.'cfg(target_os="macos")'.dependencies] objc = "0.2" cocoa = "0.5" diff --git a/servo/ports/cef/browser_host.rs b/servo/ports/cef/browser_host.rs index cf272f3e10b9..8d0c2d806f20 100644 --- a/servo/ports/cef/browser_host.rs +++ b/servo/ports/cef/browser_host.rs @@ -8,6 +8,7 @@ use interfaces::{CefBrowser, CefBrowserHost, CefClient, cef_browser_t, cef_brows use types::cef_event_flags_t::{EVENTFLAG_ALT_DOWN, EVENTFLAG_CONTROL_DOWN, EVENTFLAG_SHIFT_DOWN}; use types::cef_key_event_type_t::{KEYEVENT_CHAR, KEYEVENT_KEYDOWN, KEYEVENT_KEYUP, KEYEVENT_RAWKEYDOWN}; use types::{cef_mouse_button_type_t, cef_mouse_event, cef_rect_t, cef_key_event, cef_window_handle_t}; +use webrender_traits::ScrollLocation; use wrappers::CefWrap; use compositing::windowing::{WindowEvent, MouseWindowEvent}; @@ -471,7 +472,7 @@ full_cef_class_impl! { let delta_y: c_int = delta_y; let delta = TypedPoint2D::new(delta_x as f32, delta_y as f32); let origin = TypedPoint2D::new((*event).x as i32, (*event).y as i32); - this.downcast().send_window_event(WindowEvent::Scroll(delta, + this.downcast().send_window_event(WindowEvent::Scroll(ScrollLocation::Delta(delta), origin, TouchEventType::Move)) }} diff --git a/servo/ports/cef/lib.rs b/servo/ports/cef/lib.rs index 94e61baca554..69f3471cdab0 100644 --- a/servo/ports/cef/lib.rs +++ b/servo/ports/cef/lib.rs @@ -28,6 +28,7 @@ extern crate style_traits; extern crate net_traits; extern crate msg; +extern crate webrender_traits; extern crate libc; diff --git a/servo/ports/glutin/Cargo.toml b/servo/ports/glutin/Cargo.toml index 09557e32ed30..c25d0ede3823 100644 --- a/servo/ports/glutin/Cargo.toml +++ b/servo/ports/glutin/Cargo.toml @@ -23,6 +23,10 @@ servo_config = {path = "../../components/config"} servo_url = {path = "../../components/url"} style_traits = {path = "../../components/style_traits"} +[dependencies.webrender_traits] +git = "https://github.com/servo/webrender" +default_features = false + [target.'cfg(any(target_os = "linux", target_os = "macos"))'.dependencies] osmesa-sys = "0.1.2" diff --git a/servo/ports/glutin/lib.rs b/servo/ports/glutin/lib.rs index dd9ebef40873..f6b075a569d5 100644 --- a/servo/ports/glutin/lib.rs +++ b/servo/ports/glutin/lib.rs @@ -22,6 +22,8 @@ extern crate servo_config; extern crate servo_geometry; extern crate servo_url; extern crate style_traits; +extern crate webrender_traits; + #[cfg(target_os = "windows")] extern crate winapi; #[cfg(target_os = "windows")] extern crate user32; #[cfg(target_os = "windows")] extern crate gdi32; diff --git a/servo/ports/glutin/window.rs b/servo/ports/glutin/window.rs index e8565474ca37..66b4b83996b8 100644 --- a/servo/ports/glutin/window.rs +++ b/servo/ports/glutin/window.rs @@ -42,6 +42,7 @@ use std::sync::mpsc::{Sender, channel}; use style_traits::cursor::Cursor; #[cfg(target_os = "windows")] use user32; +use webrender_traits::ScrollLocation; #[cfg(target_os = "windows")] use winapi; @@ -425,8 +426,9 @@ impl Window { MouseScrollDelta::LineDelta(dx, dy) => (dx, dy * LINE_HEIGHT), MouseScrollDelta::PixelDelta(dx, dy) => (dx, dy), }; + let scroll_location = ScrollLocation::Delta(TypedPoint2D::new(dx, dy)); let phase = glutin_phase_to_touch_event_type(phase); - self.scroll_window(dx, dy, phase); + self.scroll_window(scroll_location, phase); }, Event::Touch(touch) => { use script_traits::TouchId; @@ -461,16 +463,19 @@ impl Window { } /// Helper function to send a scroll event. - fn scroll_window(&self, mut dx: f32, mut dy: f32, phase: TouchEventType) { + fn scroll_window(&self, scroll_location: ScrollLocation, phase: TouchEventType) { // Scroll events snap to the major axis of movement, with vertical // preferred over horizontal. - if dy.abs() >= dx.abs() { - dx = 0.0; - } else { - dy = 0.0; + if let ScrollLocation::Delta(mut delta) = scroll_location { + if delta.y.abs() >= delta.x.abs() { + delta.x = 0.0; + } else { + delta.y = 0.0; + } } + let mouse_pos = self.mouse_pos.get(); - let event = WindowEvent::Scroll(TypedPoint2D::new(dx as f32, dy as f32), + let event = WindowEvent::Scroll(scroll_location, TypedPoint2D::new(mouse_pos.x as i32, mouse_pos.y as i32), phase); self.event_queue.borrow_mut().push(event); @@ -1034,33 +1039,46 @@ impl WindowMethods for Window { (NONE, None, Key::PageDown) | (NONE, Some(' '), _) => { - self.scroll_window(0.0, + let scroll_location = ScrollLocation::Delta(TypedPoint2D::new(0.0, -self.framebuffer_size() .to_f32() .to_untyped() - .height + 2.0 * LINE_HEIGHT, + .height + 2.0 * LINE_HEIGHT)); + self.scroll_window(scroll_location, TouchEventType::Move); } (NONE, None, Key::PageUp) | (SHIFT, Some(' '), _) => { - self.scroll_window(0.0, + let scroll_location = ScrollLocation::Delta(TypedPoint2D::new(0.0, self.framebuffer_size() .to_f32() .to_untyped() - .height - 2.0 * LINE_HEIGHT, + .height - 2.0 * LINE_HEIGHT)); + self.scroll_window(scroll_location, TouchEventType::Move); } + + (NONE, None, Key::Home) => { + self.scroll_window(ScrollLocation::Start, TouchEventType::Move); + } + + (NONE, None, Key::End) => { + self.scroll_window(ScrollLocation::End, TouchEventType::Move); + } + (NONE, None, Key::Up) => { - self.scroll_window(0.0, 3.0 * LINE_HEIGHT, TouchEventType::Move); + self.scroll_window(ScrollLocation::Delta(TypedPoint2D::new(0.0, 3.0 * LINE_HEIGHT)), + TouchEventType::Move); } (NONE, None, Key::Down) => { - self.scroll_window(0.0, -3.0 * LINE_HEIGHT, TouchEventType::Move); + self.scroll_window(ScrollLocation::Delta(TypedPoint2D::new(0.0, -3.0 * LINE_HEIGHT)), + TouchEventType::Move); } (NONE, None, Key::Left) => { - self.scroll_window(LINE_HEIGHT, 0.0, TouchEventType::Move); + self.scroll_window(ScrollLocation::Delta(TypedPoint2D::new(LINE_HEIGHT, 0.0)), TouchEventType::Move); } (NONE, None, Key::Right) => { - self.scroll_window(-LINE_HEIGHT, 0.0, TouchEventType::Move); + self.scroll_window(ScrollLocation::Delta(TypedPoint2D::new(-LINE_HEIGHT, 0.0)), TouchEventType::Move); } (CMD_OR_CONTROL, Some('r'), _) => { if let Some(true) = PREFS.get("shell.builtin-key-shortcuts.enabled").as_boolean() {