From 080f46fcc226c7a108aaa68fee55587232fae28b Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 18 Feb 2016 12:24:05 +1000 Subject: [PATCH 1/2] Change the box shadow structure to store the local raster origin. This is vital because it is used as a hash key for raster ops. Before this change, two box shadows with the same parameters but different positions on screen would not hit the texture cache. --- src/internal_types.rs | 18 +++++++----------- src/renderer.rs | 10 +++++----- src/texture_cache.rs | 10 ++++------ 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/internal_types.rs b/src/internal_types.rs index 9e49b5fc58..7c8e0f2361 100644 --- a/src/internal_types.rs +++ b/src/internal_types.rs @@ -355,7 +355,7 @@ pub enum TextureUpdateDetails { /// All four corners, the tessellation index, and whether inverted, respectively. BorderRadius(DevicePixel, DevicePixel, DevicePixel, DevicePixel, Option, bool, BorderType), /// Blur radius, border radius, box rect, raster origin, and whether inverted, respectively. - BoxShadow(Au, Au, Rect, Point2D, bool, BorderType), + BoxShadow(Au, Au, Size2D, Point2D, bool, BorderType), } #[derive(Clone, Copy, Debug)] @@ -872,9 +872,8 @@ pub struct BoxShadowRasterOp { pub blur_radius: Au, pub border_radius: Au, // This is a tuple to work around the lack of `Eq` on `Rect`. - pub box_rect_origin: (Au, Au), pub box_rect_size: (Au, Au), - pub raster_origin: (Au, Au), + pub local_raster_origin: (Au, Au), pub raster_size: (Au, Au), pub part: BoxShadowPart, pub inverted: bool, @@ -913,15 +912,14 @@ impl BoxShadowRasterOp { border_radius, BoxShadowPart::Corner, box_rect); + Some(BoxShadowRasterOp { blur_radius: Au::from_f32_px(blur_radius), border_radius: Au::from_f32_px(border_radius), - box_rect_origin: (Au::from_f32_px(box_rect.origin.x), - Au::from_f32_px(box_rect.origin.y)), + local_raster_origin: (Au::from_f32_px(box_rect.origin.x - raster_rect.origin.x), + Au::from_f32_px(box_rect.origin.y - raster_rect.origin.y)), box_rect_size: (Au::from_f32_px(box_rect.size.width), Au::from_f32_px(box_rect.size.height)), - raster_origin: (Au::from_f32_px(raster_rect.origin.x), - Au::from_f32_px(raster_rect.origin.y)), raster_size: (Au::from_f32_px(raster_rect.size.width), Au::from_f32_px(raster_rect.size.height)), part: BoxShadowPart::Corner, @@ -942,12 +940,10 @@ impl BoxShadowRasterOp { Some(BoxShadowRasterOp { blur_radius: Au::from_f32_px(blur_radius), border_radius: Au::from_f32_px(border_radius), - box_rect_origin: (Au::from_f32_px(box_rect.origin.x), - Au::from_f32_px(box_rect.origin.y)), + local_raster_origin: (Au::from_f32_px(box_rect.origin.x - raster_rect.origin.x), + Au::from_f32_px(box_rect.origin.y - raster_rect.origin.y)), box_rect_size: (Au::from_f32_px(box_rect.size.width), Au::from_f32_px(box_rect.size.height)), - raster_origin: (Au::from_f32_px(raster_rect.origin.x), - Au::from_f32_px(raster_rect.origin.y)), raster_size: (Au::from_f32_px(raster_rect.size.width), Au::from_f32_px(raster_rect.size.height)), part: BoxShadowPart::Edge, diff --git a/src/renderer.rs b/src/renderer.rs index 42656d8e8e..96af640f15 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -739,7 +739,7 @@ impl Renderer { } TextureUpdateDetails::BoxShadow(blur_radius, border_radius, - box_rect, + box_rect_size, raster_origin, inverted, border_type) => { @@ -749,10 +749,10 @@ impl Renderer { &Rect::new(Point2D::new(x, y), Size2D::new(width, height)), &Rect::new( - Point2D::new((box_rect.origin.x - raster_origin.x) * device_pixel_ratio, - (box_rect.origin.y - raster_origin.y) * device_pixel_ratio), - Size2D::new(box_rect.size.width * device_pixel_ratio, - box_rect.size.height * device_pixel_ratio)), + Point2D::new(raster_origin.x * device_pixel_ratio, + raster_origin.y * device_pixel_ratio), + Size2D::new(box_rect_size.width * device_pixel_ratio, + box_rect_size.height * device_pixel_ratio)), blur_radius.to_f32_px() * device_pixel_ratio, border_radius.to_f32_px() * device_pixel_ratio, inverted, diff --git a/src/texture_cache.rs b/src/texture_cache.rs index 684296df34..eda6fef4ee 100644 --- a/src/texture_cache.rs +++ b/src/texture_cache.rs @@ -929,12 +929,10 @@ impl TextureCache { TextureUpdateDetails::BoxShadow( op.blur_radius, op.border_radius, - Rect::new(Point2D::new(op.box_rect_origin.0.to_f32_px(), - op.box_rect_origin.1.to_f32_px()), - Size2D::new(op.box_rect_size.0.to_f32_px(), - op.box_rect_size.1.to_f32_px())), - Point2D::new(op.raster_origin.0.to_f32_px(), - op.raster_origin.1.to_f32_px()), + Size2D::new(op.box_rect_size.0.to_f32_px(), + op.box_rect_size.1.to_f32_px()), + Point2D::new(op.local_raster_origin.0.to_f32_px(), + op.local_raster_origin.1.to_f32_px()), op.inverted, BorderType::SinglePixel)), } From 2e1a1ef681360b223186e29aaf2cda42804bd17b Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 18 Feb 2016 13:06:39 +1000 Subject: [PATCH 2/2] Store box shadow raster ops in device pixels, as we do for border raster ops. The renderer converts them to device pixels later anyway, so this shouldn't affect accuracy. This just means that fractional radii get the same hash key and hit the texture cache more often. --- src/batch_builder.rs | 6 ++-- src/internal_types.rs | 80 +++++++++++++++++++++++++++---------------- src/renderer.rs | 32 ++++++++--------- src/resource_cache.rs | 3 +- src/resource_list.rs | 6 ++-- src/texture_cache.rs | 24 +++++-------- 6 files changed, 84 insertions(+), 67 deletions(-) diff --git a/src/batch_builder.rs b/src/batch_builder.rs index c62f2852e0..d7be9d0771 100644 --- a/src/batch_builder.rs +++ b/src/batch_builder.rs @@ -1713,7 +1713,8 @@ impl<'a> BatchBuilder<'a> { let color_image = match BoxShadowRasterOp::create_corner(blur_radius, border_radius, box_rect, - inverted) { + inverted, + self.device_pixel_ratio) { Some(raster_item) => { let raster_item = RasterItem::BoxShadow(raster_item); resource_cache.get_raster(&raster_item, frame_id) @@ -1755,7 +1756,8 @@ impl<'a> BatchBuilder<'a> { let color_image = match BoxShadowRasterOp::create_edge(blur_radius, border_radius, box_rect, - inverted) { + inverted, + self.device_pixel_ratio) { Some(raster_item) => { let raster_item = RasterItem::BoxShadow(raster_item); resource_cache.get_raster(&raster_item, frame_id) diff --git a/src/internal_types.rs b/src/internal_types.rs index 7c8e0f2361..a9bbba5d45 100644 --- a/src/internal_types.rs +++ b/src/internal_types.rs @@ -12,7 +12,7 @@ use num::Zero; use profiler::BackendProfileCounters; use std::collections::HashMap; use std::hash::BuildHasherDefault; -use std::ops::Add; +use std::ops::{Add, Sub}; use std::path::PathBuf; use std::sync::Arc; use texture_cache::BorderType; @@ -65,6 +65,14 @@ impl Add for DevicePixel { } } +impl Sub for DevicePixel { + type Output = DevicePixel; + + fn sub(self, other: DevicePixel) -> DevicePixel { + DevicePixel(self.0 - other.0) + } +} + impl Zero for DevicePixel { fn zero() -> DevicePixel { DevicePixel(0) @@ -354,8 +362,8 @@ pub enum TextureUpdateDetails { Blur(Vec, Size2D, Au, TextureImage, TextureImage, BorderType), /// All four corners, the tessellation index, and whether inverted, respectively. BorderRadius(DevicePixel, DevicePixel, DevicePixel, DevicePixel, Option, bool, BorderType), - /// Blur radius, border radius, box rect, raster origin, and whether inverted, respectively. - BoxShadow(Au, Au, Size2D, Point2D, bool, BorderType), + /// Blur radius, border radius, box size, raster origin, and whether inverted, respectively. + BoxShadow(DevicePixel, DevicePixel, Size2D, Point2D, bool, BorderType), } #[derive(Clone, Copy, Debug)] @@ -869,12 +877,12 @@ impl BorderRadiusRasterOp { #[derive(Clone, Debug, Hash, Eq, PartialEq)] pub struct BoxShadowRasterOp { - pub blur_radius: Au, - pub border_radius: Au, + pub blur_radius: DevicePixel, + pub border_radius: DevicePixel, // This is a tuple to work around the lack of `Eq` on `Rect`. - pub box_rect_size: (Au, Au), - pub local_raster_origin: (Au, Au), - pub raster_size: (Au, Au), + pub box_rect_size: (DevicePixel, DevicePixel), + pub local_raster_origin: (DevicePixel, DevicePixel), + pub raster_size: (DevicePixel, DevicePixel), pub part: BoxShadowPart, pub inverted: bool, } @@ -905,7 +913,8 @@ impl BoxShadowRasterOp { pub fn create_corner(blur_radius: f32, border_radius: f32, box_rect: &Rect, - inverted: bool) + inverted: bool, + device_pixel_ratio: f32) -> Option { if blur_radius > 0.0 || border_radius > 0.0 { let raster_rect = BoxShadowRasterOp::raster_rect(blur_radius, @@ -913,15 +922,18 @@ impl BoxShadowRasterOp { BoxShadowPart::Corner, box_rect); + let blur_radius = DevicePixel::new(blur_radius, device_pixel_ratio); + let border_radius = DevicePixel::new(border_radius, device_pixel_ratio); + Some(BoxShadowRasterOp { - blur_radius: Au::from_f32_px(blur_radius), - border_radius: Au::from_f32_px(border_radius), - local_raster_origin: (Au::from_f32_px(box_rect.origin.x - raster_rect.origin.x), - Au::from_f32_px(box_rect.origin.y - raster_rect.origin.y)), - box_rect_size: (Au::from_f32_px(box_rect.size.width), - Au::from_f32_px(box_rect.size.height)), - raster_size: (Au::from_f32_px(raster_rect.size.width), - Au::from_f32_px(raster_rect.size.height)), + blur_radius: blur_radius, + border_radius: border_radius, + local_raster_origin: (DevicePixel::new(box_rect.origin.x - raster_rect.origin.x, device_pixel_ratio), + DevicePixel::new(box_rect.origin.y - raster_rect.origin.y, device_pixel_ratio)), + box_rect_size: (DevicePixel::new(box_rect.size.width, device_pixel_ratio), + DevicePixel::new(box_rect.size.height, device_pixel_ratio)), + raster_size: (DevicePixel::new(raster_rect.size.width, device_pixel_ratio), + DevicePixel::new(raster_rect.size.height, device_pixel_ratio)), part: BoxShadowPart::Corner, inverted: inverted, }) @@ -930,22 +942,30 @@ impl BoxShadowRasterOp { } } - pub fn create_edge(blur_radius: f32, border_radius: f32, box_rect: &Rect, inverted: bool) + pub fn create_edge(blur_radius: f32, + border_radius: f32, + box_rect: &Rect, + inverted: bool, + device_pixel_ratio: f32) -> Option { - let raster_rect = BoxShadowRasterOp::raster_rect(blur_radius, - border_radius, - BoxShadowPart::Edge, - box_rect); if blur_radius > 0.0 { + let raster_rect = BoxShadowRasterOp::raster_rect(blur_radius, + border_radius, + BoxShadowPart::Edge, + box_rect); + + let blur_radius = DevicePixel::new(blur_radius, device_pixel_ratio); + let border_radius = DevicePixel::new(border_radius, device_pixel_ratio); + Some(BoxShadowRasterOp { - blur_radius: Au::from_f32_px(blur_radius), - border_radius: Au::from_f32_px(border_radius), - local_raster_origin: (Au::from_f32_px(box_rect.origin.x - raster_rect.origin.x), - Au::from_f32_px(box_rect.origin.y - raster_rect.origin.y)), - box_rect_size: (Au::from_f32_px(box_rect.size.width), - Au::from_f32_px(box_rect.size.height)), - raster_size: (Au::from_f32_px(raster_rect.size.width), - Au::from_f32_px(raster_rect.size.height)), + blur_radius: blur_radius, + border_radius: border_radius, + local_raster_origin: (DevicePixel::new(box_rect.origin.x - raster_rect.origin.x, device_pixel_ratio), + DevicePixel::new(box_rect.origin.y - raster_rect.origin.y, device_pixel_ratio)), + box_rect_size: (DevicePixel::new(box_rect.size.width, device_pixel_ratio), + DevicePixel::new(box_rect.size.height, device_pixel_ratio)), + raster_size: (DevicePixel::new(raster_rect.size.width, device_pixel_ratio), + DevicePixel::new(raster_rect.size.height, device_pixel_ratio)), part: BoxShadowPart::Edge, inverted: inverted, }) diff --git a/src/renderer.rs b/src/renderer.rs index 96af640f15..0d8360f7cd 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -11,7 +11,7 @@ use fnv::FnvHasher; use gleam::gl; use internal_types::{RendererFrame, ResultMsg, TextureUpdateOp, BatchUpdateOp, BatchUpdateList}; use internal_types::{TextureUpdateDetails, TextureUpdateList, PackedVertex, RenderTargetMode}; -use internal_types::{ORTHO_NEAR_PLANE, ORTHO_FAR_PLANE}; +use internal_types::{ORTHO_NEAR_PLANE, ORTHO_FAR_PLANE, DevicePixel}; use internal_types::{PackedVertexForTextureCacheUpdate, CompositionOp, ChildLayerIndex}; use internal_types::{AxisDirection, LowLevelFilterOp, DrawCommand, DrawLayer, ANGLE_FLOAT_TO_FIXED}; use ipc_channel::ipc; @@ -743,18 +743,15 @@ impl Renderer { raster_origin, inverted, border_type) => { - let device_pixel_ratio = self.device_pixel_ratio; self.update_texture_cache_for_box_shadow( update.id, &Rect::new(Point2D::new(x, y), Size2D::new(width, height)), &Rect::new( - Point2D::new(raster_origin.x * device_pixel_ratio, - raster_origin.y * device_pixel_ratio), - Size2D::new(box_rect_size.width * device_pixel_ratio, - box_rect_size.height * device_pixel_ratio)), - blur_radius.to_f32_px() * device_pixel_ratio, - border_radius.to_f32_px() * device_pixel_ratio, + Point2D::new(raster_origin.x, raster_origin.y), + Size2D::new(box_rect_size.width, box_rect_size.height)), + blur_radius, + border_radius, inverted, border_type) } @@ -770,14 +767,16 @@ impl Renderer { fn update_texture_cache_for_box_shadow(&mut self, update_id: TextureId, texture_rect: &Rect, - box_rect: &Rect, - blur_radius: f32, - border_radius: f32, + box_rect: &Rect, + blur_radius: DevicePixel, + border_radius: DevicePixel, inverted: bool, border_type: BorderType) { debug_assert!(border_type == BorderType::SinglePixel); let box_shadow_program_id = self.box_shadow_program_id; + let blur_radius = blur_radius.as_f32(); + let color = if inverted { ColorF::new(1.0, 1.0, 1.0, 0.0) } else { @@ -794,11 +793,12 @@ impl Renderer { &texture_rect, border_type, |texture_rect| { - let box_rect_top_left = Point2D::new(box_rect.origin.x + texture_rect.origin.x, - box_rect.origin.y + texture_rect.origin.y); - let box_rect_bottom_right = Point2D::new(box_rect_top_left.x + box_rect.size.width, - box_rect_top_left.y + box_rect.size.height); - let border_radii = Point2D::new(border_radius, border_radius); + let box_rect_top_left = Point2D::new(box_rect.origin.x.as_f32() + texture_rect.origin.x, + box_rect.origin.y.as_f32() + texture_rect.origin.y); + let box_rect_bottom_right = Point2D::new(box_rect_top_left.x + box_rect.size.width.as_f32(), + box_rect_top_left.y + box_rect.size.height.as_f32()); + let border_radii = Point2D::new(border_radius.as_f32(), + border_radius.as_f32()); [ PackedVertexForTextureCacheUpdate::new(&texture_rect.origin, diff --git a/src/resource_cache.rs b/src/resource_cache.rs index 7717a78ad1..56c4f9d773 100644 --- a/src/resource_cache.rs +++ b/src/resource_cache.rs @@ -232,8 +232,7 @@ impl ResourceCache { if !self.cached_rasters.contains_key(raster_item) { let image_id = self.texture_cache.new_item_id(); self.texture_cache.insert_raster_op(image_id, - raster_item, - self.device_pixel_ratio); + raster_item); self.cached_rasters.insert(raster_item.clone(), image_id, frame_id); } self.cached_rasters.mark_as_needed(raster_item, frame_id); diff --git a/src/resource_list.rs b/src/resource_list.rs index 5a978d06d3..d50308ec22 100644 --- a/src/resource_list.rs +++ b/src/resource_list.rs @@ -94,7 +94,8 @@ impl ResourceList { if let Some(raster_item) = BoxShadowRasterOp::create_corner(blur_radius, border_radius, box_rect, - inverted) { + inverted, + self.device_pixel_ratio) { self.required_rasters.insert(RasterItem::BoxShadow(raster_item)); } } @@ -107,7 +108,8 @@ impl ResourceList { if let Some(raster_item) = BoxShadowRasterOp::create_edge(blur_radius, border_radius, box_rect, - inverted) { + inverted, + self.device_pixel_ratio) { self.required_rasters.insert(RasterItem::BoxShadow(raster_item)); } } diff --git a/src/texture_cache.rs b/src/texture_cache.rs index eda6fef4ee..9584d9beca 100644 --- a/src/texture_cache.rs +++ b/src/texture_cache.rs @@ -844,8 +844,7 @@ impl TextureCache { pub fn insert_raster_op(&mut self, image_id: TextureCacheItemId, - item: &RasterItem, - device_pixel_ratio: f32) { + item: &RasterItem) { let update_op = match item { &RasterItem::BorderRadius(ref op) => { let rect = @@ -901,16 +900,11 @@ impl TextureCache { } } &RasterItem::BoxShadow(ref op) => { - let device_raster_size = - Size2D::new((op.raster_size.0.to_nearest_px() as f32 * - device_pixel_ratio) as u32, - (op.raster_size.1.to_nearest_px() as f32 * - device_pixel_ratio) as u32); let allocation = self.allocate(image_id, 0, 0, - device_raster_size.width, - device_raster_size.height, + op.raster_size.0.as_u32(), + op.raster_size.1.as_u32(), ImageFormat::RGBA8, TextureCacheItemKind::Standard, BorderType::SinglePixel, @@ -924,15 +918,15 @@ impl TextureCache { op: TextureUpdateOp::Update( allocation.item.requested_rect.origin.x, allocation.item.requested_rect.origin.y, - device_raster_size.width, - device_raster_size.height, + op.raster_size.0.as_u32(), + op.raster_size.1.as_u32(), TextureUpdateDetails::BoxShadow( op.blur_radius, op.border_radius, - Size2D::new(op.box_rect_size.0.to_f32_px(), - op.box_rect_size.1.to_f32_px()), - Point2D::new(op.local_raster_origin.0.to_f32_px(), - op.local_raster_origin.1.to_f32_px()), + Size2D::new(op.box_rect_size.0, + op.box_rect_size.1), + Point2D::new(op.local_raster_origin.0, + op.local_raster_origin.1), op.inverted, BorderType::SinglePixel)), }