Skip to content

Commit

Permalink
Bug 1744842 - Use LayoutVector2D to set scroll offset (the off main-t…
Browse files Browse the repository at this point in the history
…hread part). r=gw,botond

This includes some other changes;

1) Rename relevant functions
  scroll_node_with_id -> set_scroll_offset
  scroll_node -> set_scroll_offset
  set_scroll_origin -> set_scroll_offset
2) Drop ScrollClamping argument
  In Gecko we didn't use ScrollClamping::ToContentBounds at all
3) The order of arguments of scroll_node_with_id

Differential Revision: https://phabricator.services.mozilla.com/D133145
  • Loading branch information
hiikezoe committed Dec 9, 2021
1 parent b3777e6 commit 52d73fd
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 83 deletions.
2 changes: 1 addition & 1 deletion gfx/layers/apz/src/APZCTreeManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ void APZCTreeManager::SampleForWebRender(const Maybe<VsyncId>& aVsyncId,
LayoutDevicePoint asyncScrollDelta = -layerTranslation / resolution;
aTxn.UpdateScrollPosition(wr::AsPipelineId(apzc->GetGuid().mLayersId),
apzc->GetGuid().mScrollId,
wr::ToLayoutPoint(asyncScrollDelta));
wr::ToLayoutVector2D(asyncScrollDelta));

#if defined(MOZ_WIDGET_ANDROID)
// Send the root frame metrics to java through the UIController
Expand Down
4 changes: 2 additions & 2 deletions gfx/webrender_bindings/WebRenderAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,8 @@ void TransactionWrapper::AppendTransformProperties(
void TransactionWrapper::UpdateScrollPosition(
const wr::WrPipelineId& aPipelineId,
const layers::ScrollableLayerGuid::ViewID& aScrollId,
const wr::LayoutPoint& aScrollPosition) {
wr_transaction_scroll_layer(mTxn, aPipelineId, aScrollId, aScrollPosition);
const wr::LayoutVector2D& aScrollOffset) {
wr_transaction_scroll_layer(mTxn, aPipelineId, aScrollId, aScrollOffset);
}

void TransactionWrapper::UpdateIsTransformAsyncZooming(uint64_t aAnimationId,
Expand Down
2 changes: 1 addition & 1 deletion gfx/webrender_bindings/WebRenderAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class TransactionWrapper final {
void UpdateScrollPosition(
const wr::WrPipelineId& aPipelineId,
const layers::ScrollableLayerGuid::ViewID& aScrollId,
const wr::LayoutPoint& aScrollPosition);
const wr::LayoutVector2D& aScrollOffset);
void UpdateIsTransformAsyncZooming(uint64_t aAnimationId, bool aIsZooming);

private:
Expand Down
4 changes: 2 additions & 2 deletions gfx/webrender_bindings/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1993,10 +1993,10 @@ pub extern "C" fn wr_transaction_scroll_layer(
txn: &mut Transaction,
pipeline_id: WrPipelineId,
scroll_id: u64,
new_scroll_origin: LayoutPoint,
new_scroll_offset: LayoutVector2D,
) {
let scroll_id = ExternalScrollId(scroll_id, pipeline_id);
txn.scroll_node_with_id(new_scroll_origin, scroll_id, ScrollClamping::NoClamping);
txn.set_scroll_offset(scroll_id, new_scroll_offset);
}

#[no_mangle]
Expand Down
5 changes: 2 additions & 3 deletions gfx/wr/example-compositor/compositor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,9 @@ fn main() {
}
Invalidations::Scrolling => {
let d = 0.5 - 0.5 * (2.0 * f32::consts::PI * 5.0 * time).cos();
txn.scroll_node_with_id(
LayoutPoint::new(0.0, (d * 100.0).round()),
txn.set_scroll_offset(
scroll_id,
ScrollClamping::NoClamping,
LayoutPoint::new(0.0, (d * 100.0).round()),
);
}
}
Expand Down
18 changes: 8 additions & 10 deletions gfx/wr/examples/scrolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const EXT_SCROLL_ID_CONTENT: u64 = 2;

struct App {
cursor_position: WorldPoint,
scroll_origin: LayoutPoint,
scroll_offset: LayoutVector2D,
}

impl Example for App {
Expand Down Expand Up @@ -181,12 +181,11 @@ impl Example for App {
};

if let Some(offset) = offset {
self.scroll_origin += offset;
self.scroll_offset += offset;

txn.scroll_node_with_id(
self.scroll_origin,
txn.set_scroll_offset(
ExternalScrollId(EXT_SCROLL_ID_CONTENT, PipelineId::dummy()),
ScrollClamping::ToContentBounds,
self.scroll_offset,
);
txn.generate_frame(0, RenderReasons::empty());
}
Expand All @@ -201,12 +200,11 @@ impl Example for App {
winit::MouseScrollDelta::PixelDelta(pos) => (pos.x as f32, pos.y as f32),
};

self.scroll_origin += LayoutVector2D::new(dx, dy);
self.scroll_offset += LayoutVector2D::new(dx, dy);

txn.scroll_node_with_id(
self.scroll_origin,
txn.set_scroll_offset(
ExternalScrollId(EXT_SCROLL_ID_CONTENT, PipelineId::dummy()),
ScrollClamping::ToContentBounds,
self.scroll_offset,
);

txn.generate_frame(0, RenderReasons::empty());
Expand Down Expand Up @@ -235,7 +233,7 @@ impl Example for App {
fn main() {
let mut app = App {
cursor_position: WorldPoint::zero(),
scroll_origin: LayoutPoint::zero(),
scroll_offset: LayoutVector2D::zero(),
};
boilerplate::main_wrapper(&mut app, None);
}
17 changes: 6 additions & 11 deletions gfx/wr/webrender/src/render_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::api::{BlobImageData, BlobImageKey, ImageData, ImageDescriptor, ImageK
use crate::api::{BlobImageParams, BlobImageRequest, BlobImageResult, AsyncBlobImageRasterizer, BlobImageHandler};
use crate::api::{DocumentId, PipelineId, PropertyBindingId, PropertyBindingKey, ExternalEvent};
use crate::api::{HitTestResult, HitTesterRequest, ApiHitTester, PropertyValue, DynamicProperties};
use crate::api::{ScrollClamping, TileSize, NotificationRequest, DebugFlags};
use crate::api::{TileSize, NotificationRequest, DebugFlags};
use crate::api::{GlyphDimensionRequest, GlyphIndexRequest, GlyphIndex, GlyphDimensions};
use crate::api::{FontInstanceOptions, FontInstancePlatformOptions, FontVariation, RenderReasons};
use crate::api::DEFAULT_TILE_SIZE;
Expand Down Expand Up @@ -332,17 +332,12 @@ impl Transaction {
/// pre-scrolled offsets as provided in the display list. Larger `origin`
/// values will cause the layer to be scrolled further towards the end of
/// the scroll range.
/// If the ScrollClamping argument is set to clamp, the scroll position
/// is clamped to what WebRender understands to be the bounds of the
/// scroll range, based on the sizes of the scrollable content and the
/// scroll port.
pub fn scroll_node_with_id(
pub fn set_scroll_offset(
&mut self,
origin: LayoutPoint,
id: ExternalScrollId,
clamp: ScrollClamping,
offset: LayoutVector2D,
) {
self.frame_ops.push(FrameMsg::ScrollNodeWithId(origin, id, clamp));
self.frame_ops.push(FrameMsg::SetScrollOffset(id, offset));
}

/// Set the current quality / performance settings for this document.
Expand Down Expand Up @@ -802,7 +797,7 @@ pub enum FrameMsg {
///
RequestHitTester(Sender<Arc<dyn ApiHitTester>>),
///
ScrollNodeWithId(LayoutPoint, ExternalScrollId, ScrollClamping),
SetScrollOffset(ExternalScrollId, LayoutVector2D),
///
ResetDynamicProperties,
///
Expand Down Expand Up @@ -832,7 +827,7 @@ impl fmt::Debug for FrameMsg {
FrameMsg::UpdateEpoch(..) => "FrameMsg::UpdateEpoch",
FrameMsg::HitTest(..) => "FrameMsg::HitTest",
FrameMsg::RequestHitTester(..) => "FrameMsg::RequestHitTester",
FrameMsg::ScrollNodeWithId(..) => "FrameMsg::ScrollNodeWithId",
FrameMsg::SetScrollOffset(..) => "FrameMsg::SetScrollOffset",
FrameMsg::ResetDynamicProperties => "FrameMsg::ResetDynamicProperties",
FrameMsg::AppendDynamicProperties(..) => "FrameMsg::AppendDynamicProperties",
FrameMsg::AppendDynamicTransformProperties(..) => "FrameMsg::AppendDynamicTransformProperties",
Expand Down
15 changes: 7 additions & 8 deletions gfx/wr/webrender/src/render_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use api::{DebugFlags, BlobImageHandler, Parameter, BoolParameter};
use api::{DocumentId, ExternalScrollId, HitTestResult};
use api::{IdNamespace, PipelineId, RenderNotifier, ScrollClamping};
use api::{IdNamespace, PipelineId, RenderNotifier};
use api::{NotificationRequest, Checkpoint, QualitySettings};
use api::{PrimitiveKeyKind, RenderReasons};
use api::units::*;
Expand Down Expand Up @@ -394,10 +394,10 @@ impl Document {
FrameMsg::RequestHitTester(tx) => {
tx.send(self.shared_hit_tester.clone()).unwrap();
}
FrameMsg::ScrollNodeWithId(origin, id, clamp) => {
profile_scope!("ScrollNodeWithScrollId");
FrameMsg::SetScrollOffset(id, offset) => {
profile_scope!("SetScrollOffset");

if self.scroll_node(origin, id, clamp) {
if self.set_scroll_offset(id, offset) {
self.hit_tester_is_valid = false;
self.frame_is_valid = false;
}
Expand Down Expand Up @@ -512,13 +512,12 @@ impl Document {
}

/// Returns true if the node actually changed position or false otherwise.
pub fn scroll_node(
pub fn set_scroll_offset(
&mut self,
origin: LayoutPoint,
id: ExternalScrollId,
clamp: ScrollClamping
offset: LayoutVector2D,
) -> bool {
self.spatial_tree.scroll_node(origin, id, clamp)
self.spatial_tree.set_scroll_offset(id, offset)
}

/// Update the state of tile caches when a new scene is being swapped in to
Expand Down
25 changes: 3 additions & 22 deletions gfx/wr/webrender/src/spatial_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use api::{ExternalScrollId, PipelineId, PropertyBinding, PropertyBindingId, ReferenceFrameKind, ScrollClamping, ScrollLocation};
use api::{ExternalScrollId, PipelineId, PropertyBinding, PropertyBindingId, ReferenceFrameKind, ScrollLocation};
use api::{TransformStyle, StickyOffsetBounds, SpatialTreeItemKey};
use api::units::*;
use crate::internal_types::PipelineInstanceId;
Expand Down Expand Up @@ -319,7 +319,7 @@ impl SpatialNode {
self.children.push(child);
}

pub fn set_scroll_origin(&mut self, origin: &LayoutPoint, clamp: ScrollClamping) -> bool {
pub fn set_scroll_offset(&mut self, offset: &LayoutVector2D) -> bool {
let scrolling = match self.node_type {
SpatialNodeType::ScrollFrame(ref mut scrolling) => scrolling,
_ => {
Expand All @@ -328,26 +328,7 @@ impl SpatialNode {
}
};

let normalized_offset = match clamp {
ScrollClamping::ToContentBounds => {
let scrollable_size = scrolling.scrollable_size;
let scrollable_width = scrollable_size.width;
let scrollable_height = scrollable_size.height;

if scrollable_height <= 0. && scrollable_width <= 0. {
return false;
}

let origin = LayoutPoint::new(origin.x.max(0.0), origin.y.max(0.0));
LayoutVector2D::new(
(-origin.x).max(-scrollable_width).min(0.0),
(-origin.y).max(-scrollable_height).min(0.0),
)
}
ScrollClamping::NoClamping => LayoutPoint::zero() - *origin,
};

let new_offset = normalized_offset - scrolling.external_scroll_offset;
let new_offset = -*offset - scrolling.external_scroll_offset;

if new_offset == scrolling.offset {
return false;
Expand Down
13 changes: 6 additions & 7 deletions gfx/wr/webrender/src/spatial_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use api::{ExternalScrollId, PropertyBinding, ReferenceFrameKind, TransformStyle, PropertyBindingId};
use api::{PipelineId, ScrollClamping, SpatialTreeItemKey};
use api::{PipelineId, SpatialTreeItemKey};
use api::units::*;
use euclid::Transform3D;
use crate::gpu_types::TransformPalette;
Expand Down Expand Up @@ -1066,21 +1066,20 @@ impl SpatialTree {
self.root_reference_frame_index
}

pub fn scroll_node(
pub fn set_scroll_offset(
&mut self,
origin: LayoutPoint,
id: ExternalScrollId,
clamp: ScrollClamping
offset: LayoutVector2D,
) -> bool {
let mut did_scroll_node = false;
let mut did_change = false;

self.visit_nodes_mut(|_, node| {
if node.matches_external_id(id) {
did_scroll_node |= node.set_scroll_origin(&origin, clamp);
did_change |= node.set_scroll_offset(&offset);
}
});

did_scroll_node
did_change
}

pub fn update_tree(
Expand Down
9 changes: 0 additions & 9 deletions gfx/wr/webrender_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,6 @@ impl ExternalEvent {
}
}

/// Describe whether or not scrolling should be clamped by the content bounds.
#[derive(Copy, Clone, Deserialize, Serialize)]
pub enum ScrollClamping {
///
ToContentBounds,
///
NoClamping,
}

/// A handler to integrate WebRender with the thread that contains the `Renderer`.
pub trait RenderNotifier: Send {
///
Expand Down
4 changes: 2 additions & 2 deletions gfx/wr/wrench/src/wrench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ impl Wrench {
&mut self,
frame_number: u32,
display_lists: Vec<(PipelineId, BuiltDisplayList)>,
scroll_offsets: &HashMap<ExternalScrollId, LayoutPoint>,
scroll_offsets: &HashMap<ExternalScrollId, LayoutVector2D>,
) {
let root_background_color = Some(ColorF::new(1.0, 1.0, 1.0, 1.0));

Expand All @@ -577,7 +577,7 @@ impl Wrench {
}

for (id, offset) in scroll_offsets {
txn.scroll_node_with_id(*offset, *id, ScrollClamping::NoClamping);
txn.set_scroll_offset(*id, *offset);
}

txn.generate_frame(0, RenderReasons::TESTING);
Expand Down
10 changes: 5 additions & 5 deletions gfx/wr/wrench/src/yaml_frame_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ pub struct YamlFrameReader {

/// A HashMap of offsets which specify what scroll offsets particular
/// scroll layers should be initialized with.
scroll_offsets: HashMap<ExternalScrollId, LayoutPoint>,
scroll_offsets: HashMap<ExternalScrollId, LayoutVector2D>,
next_external_scroll_id: u64,

image_map: HashMap<(PathBuf, Option<i64>), (ImageKey, LayoutSize)>,
Expand Down Expand Up @@ -1829,8 +1829,8 @@ impl YamlFrameReader {
let external_id = ExternalScrollId(self.next_external_scroll_id, dl.pipeline_id);
self.next_external_scroll_id += 1;

if let Some(size) = yaml["scroll-offset"].as_point() {
self.scroll_offsets.insert(external_id, LayoutPoint::new(size.x, size.y));
if let Some(vector) = yaml["scroll-offset"].as_vector() {
self.scroll_offsets.insert(external_id, vector);
}

let clip_to_frame = yaml["clip-to-frame"].as_bool().unwrap_or(false);
Expand Down Expand Up @@ -2161,9 +2161,9 @@ impl YamlFrameReader {
let is_blend_container = yaml["blend-container"].as_bool().unwrap_or(false);

if is_root {
if let Some(size) = yaml["scroll-offset"].as_point() {
if let Some(vector) = yaml["scroll-offset"].as_vector() {
let external_id = ExternalScrollId(0, dl.pipeline_id);
self.scroll_offsets.insert(external_id, LayoutPoint::new(size.x, size.y));
self.scroll_offsets.insert(external_id, vector);
}
}

Expand Down

0 comments on commit 52d73fd

Please sign in to comment.