From 9dd1e066ce512a15ebe68f1c69ead425ce570d40 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Thu, 9 Nov 2017 12:02:01 +0100 Subject: [PATCH] Fix some issues with scrollbars Scrollbars would scroll horizontally instead of sticking to the edge of the frame and were also not visible at all on iframes. This change fixes those issues by adding them to the containing reference frame. As part of this change, iframe background colors are also attached to the reference frame and only painted over the frame rect. --- webrender/src/border.rs | 9 +++-- webrender/src/frame.rs | 42 +++++++++++++---------- webrender/src/frame_builder.rs | 63 +++++++++++++--------------------- webrender/src/tiling.rs | 8 +---- 4 files changed, 53 insertions(+), 69 deletions(-) diff --git a/webrender/src/border.rs b/webrender/src/border.rs index e3fd2e4786..4f2c2674da 100644 --- a/webrender/src/border.rs +++ b/webrender/src/border.rs @@ -10,7 +10,6 @@ use ellipse::Ellipse; use frame_builder::FrameBuilder; use gpu_cache::GpuDataRequest; use prim_store::{BorderPrimitiveCpu, RectangleContent, PrimitiveContainer, TexelRect}; -use tiling::PrimitiveFlags; use util::{lerp, pack_as_float}; #[repr(u8)] @@ -388,7 +387,7 @@ impl FrameBuilder { clip_and_scroll, &info, RectangleContent::Fill(border.top.color), - PrimitiveFlags::None, + None, ); } if left_edge == BorderEdgeKind::Solid { @@ -401,7 +400,7 @@ impl FrameBuilder { clip_and_scroll, &info, RectangleContent::Fill(border.left.color), - PrimitiveFlags::None, + None, ); } if right_edge == BorderEdgeKind::Solid { @@ -414,7 +413,7 @@ impl FrameBuilder { clip_and_scroll, &info, RectangleContent::Fill(border.right.color), - PrimitiveFlags::None, + None, ); } if bottom_edge == BorderEdgeKind::Solid { @@ -427,7 +426,7 @@ impl FrameBuilder { clip_and_scroll, &info, RectangleContent::Fill(border.bottom.color), - PrimitiveFlags::None, + None, ); } } else { diff --git a/webrender/src/frame.rs b/webrender/src/frame.rs index a6c721c863..affe252d2b 100644 --- a/webrender/src/frame.rs +++ b/webrender/src/frame.rs @@ -15,14 +15,14 @@ use clip::ClipRegion; use clip_scroll_node::StickyFrameInfo; use clip_scroll_tree::{ClipScrollTree, ScrollStates}; use euclid::rect; -use frame_builder::{FrameBuilder, FrameBuilderConfig}; +use frame_builder::{FrameBuilder, FrameBuilderConfig, ScrollbarInfo}; use gpu_cache::GpuCache; use internal_types::{FastHashMap, FastHashSet, RendererFrame}; use prim_store::RectangleContent; use profiler::{GpuCacheProfileCounters, TextureCacheProfileCounters}; use resource_cache::{FontInstanceMap,ResourceCache, TiledImageMap}; use scene::{Scene, StackingContextHelpers, ScenePipeline}; -use tiling::{CompositeOps, Frame, PrimitiveFlags}; +use tiling::{CompositeOps, Frame}; use util::ComplexClipRegionHelpers; #[derive(Copy, Clone, PartialEq, PartialOrd, Debug, Eq, Ord)] @@ -84,7 +84,9 @@ impl<'a> FlattenContext<'a> { &mut self, traversal: &mut BuiltDisplayListIter<'a>, pipeline_id: PipelineId, - content_size: &LayoutSize, + frame_size: &LayoutSize, + root_reference_frame_id: ClipId, + root_scroll_frame_id: ClipId, ) { self.builder.push_stacking_context( &LayerVector2D::zero(), @@ -103,17 +105,16 @@ impl<'a> FlattenContext<'a> { // For the root pipeline, there's no need to add a full screen rectangle // here, as it's handled by the framebuffer clear. - let clip_id = ClipId::root_scroll_node(pipeline_id); if self.scene.root_pipeline_id != Some(pipeline_id) { if let Some(pipeline) = self.scene.pipelines.get(&pipeline_id) { if let Some(bg_color) = pipeline.background_color { - let root_bounds = LayerRect::new(LayerPoint::zero(), *content_size); + let root_bounds = LayerRect::new(LayerPoint::zero(), *frame_size); let info = LayerPrimitiveInfo::new(root_bounds); self.builder.add_solid_rectangle( - ClipAndScrollInfo::simple(clip_id), + ClipAndScrollInfo::simple(root_reference_frame_id), &info, RectangleContent::Fill(bg_color), - PrimitiveFlags::None, + None, ); } } @@ -124,13 +125,12 @@ impl<'a> FlattenContext<'a> { if self.builder.config.enable_scrollbars { let scrollbar_rect = LayerRect::new(LayerPoint::zero(), LayerSize::new(10.0, 70.0)); - let info = LayerPrimitiveInfo::new(scrollbar_rect); - + let container_rect = LayerRect::new(LayerPoint::zero(), *frame_size); self.builder.add_solid_rectangle( - ClipAndScrollInfo::simple(clip_id), - &info, + ClipAndScrollInfo::simple(root_reference_frame_id), + &LayerPrimitiveInfo::new(scrollbar_rect), RectangleContent::Fill(DEFAULT_SCROLLBAR_COLOR), - PrimitiveFlags::Scrollbar(self.clip_scroll_tree.topmost_scrolling_node_id(), 4.0), + Some(ScrollbarInfo(root_scroll_frame_id, container_rect)), ); } @@ -371,7 +371,9 @@ impl<'a> FlattenContext<'a> { self.flatten_root( &mut pipeline.display_list.iter(), pipeline_id, - &pipeline.content_size, + &iframe_rect.size, + iframe_reference_frame_id, + ClipId::root_scroll_node(pipeline_id), ); self.builder.pop_reference_frame(); @@ -459,7 +461,7 @@ impl<'a> FlattenContext<'a> { clip_and_scroll, &prim_info, RectangleContent::Fill(info.color), - PrimitiveFlags::None, + None, ); } } @@ -468,7 +470,7 @@ impl<'a> FlattenContext<'a> { clip_and_scroll, &prim_info, RectangleContent::Clear, - PrimitiveFlags::None, + None, ); } SpecificDisplayItem::Line(ref info) => { @@ -689,7 +691,7 @@ impl<'a> FlattenContext<'a> { *clip_and_scroll, &prim_info, content, - PrimitiveFlags::None, + None, ); has_opaque = true; } @@ -710,7 +712,7 @@ impl<'a> FlattenContext<'a> { *clip_and_scroll, &prim_info, content, - PrimitiveFlags::None, + None, ); } true @@ -1142,10 +1144,14 @@ impl FrameContext { roller.clip_scroll_tree, ); + let reference_frame_id = roller.clip_scroll_tree.root_reference_frame_id; + let scroll_frame_id = roller.clip_scroll_tree.topmost_scrolling_node_id; roller.flatten_root( &mut root_pipeline.display_list.iter(), root_pipeline_id, - &root_pipeline.content_size, + &root_pipeline.viewport_size, + reference_frame_id, + scroll_frame_id, ); self.pipeline_epoch_map.extend(roller.pipeline_epochs.drain(..)); diff --git a/webrender/src/frame_builder.rs b/webrender/src/frame_builder.rs index 471ef4d3e5..ff1711445e 100644 --- a/webrender/src/frame_builder.rs +++ b/webrender/src/frame_builder.rs @@ -34,13 +34,15 @@ use render_task::RenderTaskTree; use resource_cache::ResourceCache; use scene::ScenePipeline; use std::{mem, usize, f32, i32}; -use tiling::{CompositeOps, Frame}; -use tiling::{ContextIsolation, RenderTargetKind, StackingContextIndex}; -use tiling::{PrimitiveFlags, PrimitiveRunCmd, RenderPass}; -use tiling::{RenderTargetContext, ScrollbarPrimitive, StackingContext}; +use tiling::{CompositeOps, ContextIsolation, Frame, PrimitiveRunCmd, RenderPass}; +use tiling::{RenderTargetContext, RenderTargetKind, ScrollbarPrimitive, StackingContext}; +use tiling::StackingContextIndex; use util::{self, pack_as_float, RectHelpers, recycle_vec}; use box_shadow::BLUR_SAMPLE_SCALE; +#[derive(Debug)] +pub struct ScrollbarInfo(pub ClipId, pub LayerRect); + /// Construct a polygon from stacking context boundaries. /// `anchor` here is an index that's going to be preserved in all the /// splits of the polygon. @@ -565,7 +567,7 @@ impl FrameBuilder { clip_and_scroll: ClipAndScrollInfo, info: &LayerPrimitiveInfo, content: RectangleContent, - flags: PrimitiveFlags, + scrollbar_info: Option, ) { if let RectangleContent::Fill(ColorF{a, ..}) = content { if a == 0.0 { @@ -587,15 +589,12 @@ impl FrameBuilder { PrimitiveContainer::Rectangle(prim), ); - match flags { - PrimitiveFlags::None => {} - PrimitiveFlags::Scrollbar(clip_id, border_radius) => { - self.scrollbar_prims.push(ScrollbarPrimitive { - prim_index, - clip_id, - border_radius, - }); - } + if let Some(ScrollbarInfo(clip_id, frame_rect)) = scrollbar_info { + self.scrollbar_prims.push(ScrollbarPrimitive { + prim_index, + clip_id, + frame_rect, + }); } } @@ -1515,45 +1514,31 @@ impl FrameBuilder { } fn update_scroll_bars(&mut self, clip_scroll_tree: &ClipScrollTree, gpu_cache: &mut GpuCache) { - let distance_from_edge = 8.0; + static SCROLLBAR_PADDING: f32 = 8.0; for scrollbar_prim in &self.scrollbar_prims { let metadata = &mut self.prim_store.cpu_metadata[scrollbar_prim.prim_index.0]; - let clip_scroll_node = &clip_scroll_tree.nodes[&scrollbar_prim.clip_id]; + let scroll_frame = &clip_scroll_tree.nodes[&scrollbar_prim.clip_id]; // Invalidate what's in the cache so it will get rebuilt. gpu_cache.invalidate(&metadata.gpu_location); - let scrollable_distance = clip_scroll_node.scrollable_size().height; - + let scrollable_distance = scroll_frame.scrollable_size().height; if scrollable_distance <= 0.0 { metadata.local_clip_rect.size = LayerSize::zero(); continue; } + let amount_scrolled = -scroll_frame.scroll_offset().y / scrollable_distance; - let scroll_offset = clip_scroll_node.scroll_offset(); - let f = -scroll_offset.y / scrollable_distance; - - let min_y = clip_scroll_node.local_viewport_rect.origin.y - scroll_offset.y + - distance_from_edge; - - let max_y = clip_scroll_node.local_viewport_rect.origin.y + - clip_scroll_node.local_viewport_rect.size.height - - scroll_offset.y - metadata.local_rect.size.height - - distance_from_edge; + let frame_rect = scrollbar_prim.frame_rect; + let min_y = frame_rect.origin.y + SCROLLBAR_PADDING; + let max_y = frame_rect.origin.y + frame_rect.size.height - + (SCROLLBAR_PADDING + metadata.local_rect.size.height); - metadata.local_rect.origin.x = clip_scroll_node.local_viewport_rect.origin.x + - clip_scroll_node.local_viewport_rect.size.width - - metadata.local_rect.size.width - - distance_from_edge; - - metadata.local_rect.origin.y = util::lerp(min_y, max_y, f); + metadata.local_rect.origin.x = frame_rect.origin.x + frame_rect.size.width - + (metadata.local_rect.size.width + SCROLLBAR_PADDING); + metadata.local_rect.origin.y = util::lerp(min_y, max_y, amount_scrolled); metadata.local_clip_rect = metadata.local_rect; - - // TODO(gw): The code to set / update border clips on scroll bars - // has been broken for a long time, so I've removed it - // for now. We can re-add that code once the clips - // data is moved over to the GPU cache! } } diff --git a/webrender/src/tiling.rs b/webrender/src/tiling.rs index 8bcf5e054b..a229a51559 100644 --- a/webrender/src/tiling.rs +++ b/webrender/src/tiling.rs @@ -112,7 +112,7 @@ impl AlphaBatchHelpers for PrimitiveStore { pub struct ScrollbarPrimitive { pub clip_id: ClipId, pub prim_index: PrimitiveIndex, - pub border_radius: f32, + pub frame_rect: LayerRect, } #[derive(Debug)] @@ -122,12 +122,6 @@ pub enum PrimitiveRunCmd { PrimitiveRun(PrimitiveRun), } -#[derive(Debug, Copy, Clone)] -pub enum PrimitiveFlags { - None, - Scrollbar(ClipId, f32), -} - #[derive(Debug, Copy, Clone)] pub struct RenderTargetIndex(pub usize);