Skip to content

Commit

Permalink
Auto merge of #20071 - paulrouget:typedsize, r=glennw
Browse files Browse the repository at this point in the history
Use typed coordinates more

Requires #19895

We use Size2D and Point2D across compositing, constellation and script, loosing the type of pixels we use (DevicePixel, DeviceIndepententPixel or CSSPixel) along the way, which might lead to bugs like `window.outerHeight` not taking into account the page zoom (using DeviceIndepententPixel instead of CSSPixel).

This should make the situation a bit better.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because we can't zoom in a test

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20071)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Mar 16, 2018
2 parents 74d6a91 + fef0506 commit fc90e61
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 133 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 29 additions & 26 deletions components/compositing/compositor.rs
Expand Up @@ -21,7 +21,7 @@ use script_traits::{MouseButton, MouseEventType, ScrollState, TouchEventType, To
use script_traits::{UntrustedNodeAddress, WindowSizeData, WindowSizeType};
use script_traits::CompositorEvent::{MouseMoveEvent, MouseButtonEvent, TouchEvent};
use servo_config::opts;
use servo_geometry::DeviceIndependentPixel;
use servo_geometry::{DeviceIndependentPixel, DeviceUintLength};
use std::collections::HashMap;
use std::env;
use std::fs::File;
Expand All @@ -34,7 +34,7 @@ use style_traits::viewport::ViewportConstraints;
use time::{now, precise_time_ns, precise_time_s};
use touch::{TouchHandler, TouchAction};
use webrender;
use webrender_api::{self, DeviceUintRect, DeviceUintSize, HitTestFlags, HitTestResult};
use webrender_api::{self, DeviceIntPoint, DevicePoint, DeviceUintRect, DeviceUintSize, HitTestFlags, HitTestResult};
use webrender_api::{LayoutVector2D, ScrollEventPhase, ScrollLocation};
use windowing::{self, MouseWindowEvent, WebRenderDebugOption, WindowMethods};

Expand Down Expand Up @@ -202,7 +202,7 @@ struct ScrollZoomEvent {
/// Scroll by this offset, or to Start or End
scroll_location: ScrollLocation,
/// Apply changes to the frame at this location
cursor: TypedPoint2D<i32, DevicePixel>,
cursor: DeviceIntPoint,
/// The scroll event phase.
phase: ScrollEventPhase,
/// The number of OS events that have been coalesced together into this one event.
Expand Down Expand Up @@ -275,15 +275,15 @@ impl RenderTargetInfo {
}
}

fn initialize_png(gl: &gl::Gl, width: usize, height: usize) -> RenderTargetInfo {
fn initialize_png(gl: &gl::Gl, width: DeviceUintLength, height: DeviceUintLength) -> RenderTargetInfo {
let framebuffer_ids = gl.gen_framebuffers(1);
gl.bind_framebuffer(gl::FRAMEBUFFER, framebuffer_ids[0]);

let texture_ids = gl.gen_textures(1);
gl.bind_texture(gl::TEXTURE_2D, texture_ids[0]);

gl.tex_image_2d(gl::TEXTURE_2D, 0, gl::RGB as gl::GLint, width as gl::GLsizei,
height as gl::GLsizei, 0, gl::RGB, gl::UNSIGNED_BYTE, None);
gl.tex_image_2d(gl::TEXTURE_2D, 0, gl::RGB as gl::GLint, width.get() as gl::GLsizei,
height.get() as gl::GLsizei, 0, gl::RGB, gl::UNSIGNED_BYTE, None);
gl.tex_parameter_i(gl::TEXTURE_2D, gl::TEXTURE_MAG_FILTER, gl::NEAREST as gl::GLint);
gl.tex_parameter_i(gl::TEXTURE_2D, gl::TEXTURE_MIN_FILTER, gl::NEAREST as gl::GLint);

Expand All @@ -297,8 +297,8 @@ fn initialize_png(gl: &gl::Gl, width: usize, height: usize) -> RenderTargetInfo
gl.bind_renderbuffer(gl::RENDERBUFFER, depth_rb);
gl.renderbuffer_storage(gl::RENDERBUFFER,
gl::DEPTH_COMPONENT24,
width as gl::GLsizei,
height as gl::GLsizei);
width.get() as gl::GLsizei,
height.get() as gl::GLsizei);
gl.framebuffer_renderbuffer(gl::FRAMEBUFFER,
gl::DEPTH_ATTACHMENT,
gl::RENDERBUFFER,
Expand Down Expand Up @@ -647,9 +647,11 @@ impl<Window: WindowMethods> IOCompositor<Window> {
device_pixel_ratio: dppx,
initial_viewport: initial_viewport,
};

let top_level_browsing_context_id = self.root_pipeline.as_ref().map(|pipeline| {
pipeline.top_level_browsing_context_id
});

let msg = ConstellationMsg::WindowSize(top_level_browsing_context_id, data, size_type);

if let Err(e) = self.constellation_chan.send(msg) {
Expand Down Expand Up @@ -728,7 +730,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
}
}

fn hit_test_at_point(&self, point: TypedPoint2D<f32, DevicePixel>) -> HitTestResult {
fn hit_test_at_point(&self, point: DevicePoint) -> HitTestResult {
let dppx = self.page_zoom * self.hidpi_factor();
let scaled_point = (point / dppx).to_untyped();

Expand All @@ -742,7 +744,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {

}

pub fn on_mouse_window_move_event_class(&mut self, cursor: TypedPoint2D<f32, DevicePixel>) {
pub fn on_mouse_window_move_event_class(&mut self, cursor: DevicePoint) {
if opts::get().convert_mouse_to_touch {
self.on_touch_move(TouchId(0), cursor);
return
Expand All @@ -751,7 +753,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
self.dispatch_mouse_window_move_event_class(cursor);
}

fn dispatch_mouse_window_move_event_class(&mut self, cursor: TypedPoint2D<f32, DevicePixel>) {
fn dispatch_mouse_window_move_event_class(&mut self, cursor: DevicePoint) {
let root_pipeline_id = match self.get_root_pipeline_id() {
Some(root_pipeline_id) => root_pipeline_id,
None => return,
Expand Down Expand Up @@ -783,7 +785,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
&self,
event_type: TouchEventType,
identifier: TouchId,
point: TypedPoint2D<f32, DevicePixel>)
point: DevicePoint)
{
let results = self.hit_test_at_point(point);
if let Some(item) = results.items.first() {
Expand All @@ -804,7 +806,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
pub fn on_touch_event(&mut self,
event_type: TouchEventType,
identifier: TouchId,
location: TypedPoint2D<f32, DevicePixel>) {
location: DevicePoint) {
match event_type {
TouchEventType::Down => self.on_touch_down(identifier, location),
TouchEventType::Move => self.on_touch_move(identifier, location),
Expand All @@ -813,12 +815,12 @@ impl<Window: WindowMethods> IOCompositor<Window> {
}
}

fn on_touch_down(&mut self, identifier: TouchId, point: TypedPoint2D<f32, DevicePixel>) {
fn on_touch_down(&mut self, identifier: TouchId, point: DevicePoint) {
self.touch_handler.on_touch_down(identifier, point);
self.send_touch_event(TouchEventType::Down, identifier, point);
}

fn on_touch_move(&mut self, identifier: TouchId, point: TypedPoint2D<f32, DevicePixel>) {
fn on_touch_move(&mut self, identifier: TouchId, point: DevicePoint) {
match self.touch_handler.on_touch_move(identifier, point) {
TouchAction::Scroll(delta) => {
match point.cast() {
Expand Down Expand Up @@ -849,22 +851,22 @@ impl<Window: WindowMethods> IOCompositor<Window> {
}
}

fn on_touch_up(&mut self, identifier: TouchId, point: TypedPoint2D<f32, DevicePixel>) {
fn on_touch_up(&mut self, identifier: TouchId, point: DevicePoint) {
self.send_touch_event(TouchEventType::Up, identifier, point);

if let TouchAction::Click = self.touch_handler.on_touch_up(identifier, point) {
self.simulate_mouse_click(point);
}
}

fn on_touch_cancel(&mut self, identifier: TouchId, point: TypedPoint2D<f32, DevicePixel>) {
fn on_touch_cancel(&mut self, identifier: TouchId, point: DevicePoint) {
// Send the event to script.
self.touch_handler.on_touch_cancel(identifier, point);
self.send_touch_event(TouchEventType::Cancel, identifier, point);
}

/// <http://w3c.github.io/touch-events/#mouse-events>
fn simulate_mouse_click(&mut self, p: TypedPoint2D<f32, DevicePixel>) {
fn simulate_mouse_click(&mut self, p: DevicePoint) {
let button = MouseButton::Left;
self.dispatch_mouse_window_move_event_class(p);
self.dispatch_mouse_window_event_class(MouseWindowEvent::MouseDown(button, p));
Expand All @@ -874,7 +876,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {

pub fn on_scroll_event(&mut self,
delta: ScrollLocation,
cursor: TypedPoint2D<i32, DevicePixel>,
cursor: DeviceIntPoint,
phase: TouchEventType) {
match phase {
TouchEventType::Move => self.on_scroll_window_event(delta, cursor),
Expand All @@ -889,7 +891,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {

fn on_scroll_window_event(&mut self,
scroll_location: ScrollLocation,
cursor: TypedPoint2D<i32, DevicePixel>) {
cursor: DeviceIntPoint) {
let event_phase = match (self.scroll_in_progress, self.in_scroll_transaction) {
(false, None) => ScrollEventPhase::Start,
(false, Some(last_scroll)) if last_scroll.elapsed() > Duration::from_millis(80) =>
Expand All @@ -908,7 +910,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {

fn on_scroll_start_window_event(&mut self,
scroll_location: ScrollLocation,
cursor: TypedPoint2D<i32, DevicePixel>) {
cursor: DeviceIntPoint) {
self.scroll_in_progress = true;
self.pending_scroll_zoom_events.push(ScrollZoomEvent {
magnification: 1.0,
Expand All @@ -921,7 +923,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {

fn on_scroll_end_window_event(&mut self,
scroll_location: ScrollLocation,
cursor: TypedPoint2D<i32, DevicePixel>) {
cursor: DeviceIntPoint) {
self.scroll_in_progress = false;
self.pending_scroll_zoom_events.push(ScrollZoomEvent {
magnification: 1.0,
Expand Down Expand Up @@ -1254,8 +1256,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
fn composite_specific_target(&mut self,
target: CompositeTarget)
-> Result<Option<Image>, UnableToComposite> {
let (width, height) =
(self.frame_size.width as usize, self.frame_size.height as usize);
let (width, height) = (self.frame_size.width_typed(), self.frame_size.height_typed());
if !self.window.prepare_for_composite(width, height) {
return Err(UnableToComposite::WindowUnprepared)
}
Expand Down Expand Up @@ -1375,9 +1376,11 @@ impl<Window: WindowMethods> IOCompositor<Window> {

fn draw_img(&self,
render_target_info: RenderTargetInfo,
width: usize,
height: usize)
width: DeviceUintLength,
height: DeviceUintLength)
-> RgbImage {
let width = width.get() as usize;
let height = height.get() as usize;
// For some reason, OSMesa fails to render on the 3rd
// attempt in headless mode, under some conditions.
// I think this can only be some kind of synchronization
Expand Down
14 changes: 7 additions & 7 deletions components/compositing/compositor_thread.rs
Expand Up @@ -6,7 +6,6 @@

use SendableFrameTree;
use compositor::CompositingReason;
use euclid::{Point2D, Size2D};
use gfx_traits::Epoch;
use ipc_channel::ipc::IpcSender;
use msg::constellation_msg::{Key, KeyModifiers, KeyState, PipelineId, TopLevelBrowsingContextId};
Expand All @@ -20,7 +19,7 @@ use std::sync::mpsc::{Receiver, Sender};
use style_traits::cursor::CursorKind;
use style_traits::viewport::ViewportConstraints;
use webrender;
use webrender_api;
use webrender_api::{self, DeviceIntPoint, DeviceUintSize};


/// Used to wake up the event loop, provided by the servo port/embedder.
Expand Down Expand Up @@ -119,15 +118,16 @@ pub enum EmbedderMsg {
/// Alerts the embedder that the current page has changed its title.
ChangePageTitle(TopLevelBrowsingContextId, Option<String>),
/// Move the window to a point
MoveTo(TopLevelBrowsingContextId, Point2D<i32>),
MoveTo(TopLevelBrowsingContextId, DeviceIntPoint),
/// Resize the window to size
ResizeTo(TopLevelBrowsingContextId, Size2D<u32>),
ResizeTo(TopLevelBrowsingContextId, DeviceUintSize),
/// Get Window Informations size and position
GetClientWindow(TopLevelBrowsingContextId, IpcSender<(Size2D<u32>, Point2D<i32>)>),
GetClientWindow(TopLevelBrowsingContextId,
IpcSender<(DeviceUintSize, DeviceIntPoint)>),
/// Get screen size (pixel)
GetScreenSize(TopLevelBrowsingContextId, IpcSender<(Size2D<u32>)>),
GetScreenSize(TopLevelBrowsingContextId, IpcSender<(DeviceUintSize)>),
/// Get screen available size (pixel)
GetScreenAvailSize(TopLevelBrowsingContextId, IpcSender<(Size2D<u32>)>),
GetScreenAvailSize(TopLevelBrowsingContextId, IpcSender<(DeviceUintSize)>),
/// Wether or not to follow a link
AllowNavigation(TopLevelBrowsingContextId, ServoUrl, IpcSender<bool>),
/// Sends an unconsumed key event back to the embedder.
Expand Down
37 changes: 17 additions & 20 deletions components/compositing/windowing.rs
Expand Up @@ -5,26 +5,25 @@
//! Abstract windowing methods. The concrete implementations of these can be found in `platform/`.

use compositor_thread::EventLoopWaker;
use euclid::{Point2D, Size2D};
use euclid::{TypedScale, TypedPoint2D, TypedSize2D};
use euclid::TypedScale;
use gleam::gl;
use ipc_channel::ipc::IpcSender;
use msg::constellation_msg::{Key, KeyModifiers, KeyState, TopLevelBrowsingContextId, TraversalDirection};
use net_traits::net_error_list::NetError;
use script_traits::{LoadData, MouseButton, TouchEventType, TouchId};
use servo_geometry::DeviceIndependentPixel;
use servo_geometry::{DeviceIndependentPixel, DeviceUintLength};
use servo_url::ServoUrl;
use std::fmt::{Debug, Error, Formatter};
use std::rc::Rc;
use style_traits::DevicePixel;
use style_traits::cursor::CursorKind;
use webrender_api::{DeviceUintSize, DeviceUintRect, ScrollLocation};
use webrender_api::{DeviceIntPoint, DevicePoint, DeviceUintSize, DeviceUintRect, ScrollLocation};

#[derive(Clone)]
pub enum MouseWindowEvent {
Click(MouseButton, TypedPoint2D<f32, DevicePixel>),
MouseDown(MouseButton, TypedPoint2D<f32, DevicePixel>),
MouseUp(MouseButton, TypedPoint2D<f32, DevicePixel>),
Click(MouseButton, DevicePoint),
MouseDown(MouseButton, DevicePoint),
MouseUp(MouseButton, DevicePoint),
}

/// Various debug and profiling flags that WebRender supports.
Expand Down Expand Up @@ -55,12 +54,12 @@ pub enum WindowEvent {
/// Sent when a mouse hit test is to be performed.
MouseWindowEventClass(MouseWindowEvent),
/// Sent when a mouse move.
MouseWindowMoveEventClass(TypedPoint2D<f32, DevicePixel>),
MouseWindowMoveEventClass(DevicePoint),
/// Touch event: type, identifier, point
Touch(TouchEventType, TouchId, TypedPoint2D<f32, DevicePixel>),
Touch(TouchEventType, TouchId, DevicePoint),
/// Sent when the user scrolls. The first point is the delta and the second point is the
/// origin.
Scroll(ScrollLocation, TypedPoint2D<i32, DevicePixel>, TouchEventType),
Scroll(ScrollLocation, DeviceIntPoint, TouchEventType),
/// Sent when the user zooms.
Zoom(f32),
/// Simulated "pinch zoom" gesture for non-touch platforms (e.g. ctrl-scrollwheel).
Expand Down Expand Up @@ -126,21 +125,19 @@ pub trait WindowMethods {
fn framebuffer_size(&self) -> DeviceUintSize;
/// Returns the position and size of the window within the rendering area.
fn window_rect(&self) -> DeviceUintRect;
/// Returns the size of the window in density-independent "px" units.
fn size(&self) -> TypedSize2D<f32, DeviceIndependentPixel>;
/// Presents the window to the screen (perhaps by page flipping).
fn present(&self);

/// Return the size of the window with head and borders and position of the window values
fn client_window(&self, ctx: TopLevelBrowsingContextId) -> (Size2D<u32>, Point2D<i32>);
/// Return the size of the screen (pixel)
fn screen_size(&self, ctx: TopLevelBrowsingContextId) -> Size2D<u32>;
/// Return the available size of the screen (pixel)
fn screen_avail_size(&self, ctx: TopLevelBrowsingContextId) -> Size2D<u32>;
fn client_window(&self, ctx: TopLevelBrowsingContextId) -> (DeviceUintSize, DeviceIntPoint);
/// Return the size of the screen.
fn screen_size(&self, ctx: TopLevelBrowsingContextId) -> DeviceUintSize;
/// Return the available size of the screen.
fn screen_avail_size(&self, ctx: TopLevelBrowsingContextId) -> DeviceUintSize;
/// Set the size inside of borders and head
fn set_inner_size(&self, ctx: TopLevelBrowsingContextId, size: Size2D<u32>);
fn set_inner_size(&self, ctx: TopLevelBrowsingContextId, size: DeviceUintSize);
/// Set the window position
fn set_position(&self, ctx: TopLevelBrowsingContextId, point: Point2D<i32>);
fn set_position(&self, ctx: TopLevelBrowsingContextId, point: DeviceIntPoint);
/// Set fullscreen state
fn set_fullscreen_state(&self, ctx: TopLevelBrowsingContextId, state: bool);

Expand Down Expand Up @@ -170,7 +167,7 @@ pub trait WindowMethods {
/// Requests that the window system prepare a composite. Typically this will involve making
/// some type of platform-specific graphics context current. Returns true if the composite may
/// proceed and false if it should not.
fn prepare_for_composite(&self, width: usize, height: usize) -> bool;
fn prepare_for_composite(&self, width: DeviceUintLength, height: DeviceUintLength) -> bool;

/// Sets the cursor to be used in the window.
fn set_cursor(&self, cursor: CursorKind);
Expand Down
1 change: 1 addition & 0 deletions components/geometry/Cargo.toml
Expand Up @@ -14,4 +14,5 @@ app_units = "0.6"
euclid = "0.17"
malloc_size_of = { path = "../malloc_size_of" }
malloc_size_of_derive = { path = "../malloc_size_of_derive" }
style_traits = { path = "../style_traits" }
webrender_api = { git = "https://github.com/servo/webrender" }
6 changes: 5 additions & 1 deletion components/geometry/lib.rs
Expand Up @@ -5,16 +5,20 @@
extern crate app_units;
extern crate euclid;
extern crate malloc_size_of;
extern crate style_traits;
#[macro_use] extern crate malloc_size_of_derive;
extern crate webrender_api;

use app_units::{Au, MAX_AU, MIN_AU};
use euclid::{Point2D, Rect, Size2D};
use euclid::{Length, Point2D, Rect, Size2D};
use std::f32;
use style_traits::DevicePixel;
use webrender_api::{LayoutPoint, LayoutRect, LayoutSize};

// Units for use with euclid::length and euclid::scale_factor.

pub type DeviceUintLength = Length<u32, DevicePixel>;

/// A normalized "pixel" at the default resolution for the display.
///
/// Like the CSS "px" unit, the exact physical size of this unit may vary between devices, but it
Expand Down

0 comments on commit fc90e61

Please sign in to comment.