Skip to content

Commit

Permalink
Bug 1642308 - Fix some snapping related picture cache tile rect calcu…
Browse files Browse the repository at this point in the history 
…lations. r=gw

Our fract offset for the tiles should be a simple mapping of the snapped
device position back to picture space. If no snapping is required, then
the position will be precisely the origin. When using this value to
decide if the position has changed for glyph subpixel offset purposes,
we must consider it in device space, since picture to device space can
be effectively arbitrary.

We update the stored fract offset at which a tile was rendered whenever
we invalidate the whole tile, not just when we detect the fract offset
has changed by a notable amount. This should reduce spurious
invalidations since the tile was actually rendered at a different offset
that we had recorded prior to this patch.

Also, when evaluating the tile's valid rect, we cannot use the local
valid rect. The device valid rect we use is the local mapped from
picture space, but also snapped. Thus it makes far more sense to compare
that which we used for drawing purposes which has the bonus of avoiding
floating point errors.

Differential Revision: https://phabricator.services.mozilla.com/D91156
  • Loading branch information
@aosmond
aosmond committed Oct 29, 2020
1 parent 1cca051 commit d11c215
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 31 deletions.
2 changes: 1 addition & 1 deletion gfx/wr/webrender/src/composite.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ impl CompositeState {
let tile = CompositeTile {
surface,
rect: device_rect,
valid_rect: tile.device_valid_rect.translate(-device_rect.origin.to_vector()),
valid_rect: tile.current_descriptor.device_valid_rect.translate(-device_rect.origin.to_vector()),
dirty_rect: tile.device_dirty_rect.translate(-device_rect.origin.to_vector()),
clip_rect: device_clip_rect,
z_id: tile.z_id,
Expand Down
87 changes: 57 additions & 30 deletions gfx/wr/webrender/src/picture.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ struct TilePreUpdateContext {
/// The fractional position of the picture cache, which may
/// require invalidation of all tiles.
fract_offset: PictureVector2D,
device_fract_offset: DeviceVector2D,

/// The optional background color of the picture cache instance
background_color: Option<ColorF>,
Expand Down Expand Up @@ -796,8 +797,8 @@ pub enum PrimitiveCompareResultDetail {
pub enum InvalidationReason {
/// The fractional offset changed
FractionalOffset {
old: PictureVector2D,
new: PictureVector2D,
old: DeviceVector2D,
new: DeviceVector2D,
},
/// The background color changed
BackgroundColor {
Expand Down Expand Up @@ -837,7 +838,7 @@ pub enum InvalidationReason {
pub struct TileSerializer {
pub rect: PictureRect,
pub current_descriptor: TileDescriptor,
pub fract_offset: PictureVector2D,
pub device_fract_offset: DeviceVector2D,
pub id: TileId,
pub root: TileNode,
pub background_color: Option<ColorF>,
Expand Down Expand Up @@ -870,8 +871,6 @@ pub struct Tile {
/// TODO(gw): We have multiple dirty rects available due to the quadtree above. In future,
/// expose these as multiple dirty rects, which will help in some cases.
pub device_dirty_rect: DeviceRect,
/// Device space rect that contains valid pixels region of this tile.
pub device_valid_rect: DeviceRect,
/// Uniquely describes the content of this tile, in a way that can be
/// (reasonably) efficiently hashed and compared.
pub current_descriptor: TileDescriptor,
Expand All @@ -889,7 +888,7 @@ pub struct Tile {
/// The current fractional offset of the cache transform root. If this changes,
/// all tiles need to be invalidated and redrawn, since snapping differences are
/// likely to occur.
fract_offset: PictureVector2D,
device_fract_offset: DeviceVector2D,
/// The tile id is stable between display lists and / or frames,
/// if the tile is retained. Useful for debugging tile evictions.
pub id: TileId,
Expand Down Expand Up @@ -927,15 +926,14 @@ impl Tile {
local_tile_rect: PictureRect::zero(),
local_tile_box: PictureBox2D::zero(),
world_tile_rect: WorldRect::zero(),
device_valid_rect: DeviceRect::zero(),
local_dirty_rect: PictureRect::zero(),
device_dirty_rect: DeviceRect::zero(),
surface: None,
current_descriptor: TileDescriptor::new(),
prev_descriptor: TileDescriptor::new(),
is_valid: false,
is_visible: false,
fract_offset: PictureVector2D::zero(),
device_fract_offset: DeviceVector2D::zero(),
id,
is_opaque: false,
root: TileNode::new_leaf(Vec::new()),
Expand All @@ -953,7 +951,7 @@ impl Tile {
fn print(&self, pt: &mut dyn PrintTreePrinter) {
pt.new_level(format!("Tile {:?}", self.id));
pt.add_item(format!("local_tile_rect: {:?}", self.local_tile_rect));
pt.add_item(format!("fract_offset: {:?}", self.fract_offset));
pt.add_item(format!("device_fract_offset: {:?}", self.device_fract_offset));
pt.add_item(format!("background_color: {:?}", self.background_color));
pt.add_item(format!("invalidation_reason: {:?}", self.invalidation_reason));
self.current_descriptor.print(pt);
Expand Down Expand Up @@ -1022,7 +1020,7 @@ impl Tile {
}
// TODO(gw): We can avoid invalidating the whole tile in some cases here,
// but it should be a fairly rare invalidation case.
if self.current_descriptor.local_valid_rect != self.prev_descriptor.local_valid_rect {
if self.current_descriptor.device_valid_rect != self.prev_descriptor.device_valid_rect {
self.invalidate(None, InvalidationReason::ValidRectChanged);
state.composite_state.dirty_rects_are_valid = false;
}
Expand Down Expand Up @@ -1090,15 +1088,16 @@ impl Tile {
return;
}

// Determine if the fractional offset of the transform is different this frame
// from the currently cached tile set.
let fract_changed = (self.fract_offset.x - ctx.fract_offset.x).abs() > 0.01 ||
(self.fract_offset.y - ctx.fract_offset.y).abs() > 0.01;
// We may need to rerender if glyph subpixel positions have changed. Note
// that we update the tile fract offset itself after we have completed
// invalidation. This allows for other whole tile invalidation cases to
// update the fract offset appropriately.
let fract_delta = self.device_fract_offset - ctx.device_fract_offset;
let fract_changed = fract_delta.x.abs() > 0.01 || fract_delta.y.abs() > 0.01;
if fract_changed {
self.invalidate(None, InvalidationReason::FractionalOffset {
old: self.fract_offset,
new: ctx.fract_offset });
self.fract_offset = ctx.fract_offset;
old: self.device_fract_offset,
new: ctx.device_fract_offset });
}

if ctx.background_color != self.background_color {
Expand Down Expand Up @@ -1284,7 +1283,7 @@ impl Tile {
// always aligned to a device pixel. To handle this, round out to get all
// required pixels, and intersect with the tile device rect.
let device_rect = (self.world_tile_rect * ctx.global_device_pixel_scale).round();
self.device_valid_rect = (world_valid_rect * ctx.global_device_pixel_scale)
self.current_descriptor.device_valid_rect = (world_valid_rect * ctx.global_device_pixel_scale)
.round_out()
.intersection(&device_rect)
.unwrap_or_else(DeviceRect::zero);
Expand All @@ -1308,6 +1307,20 @@ impl Tile {
return false;
}

let world_valid_rect = ctx.pic_to_world_mapper
.map(&self.current_descriptor.local_valid_rect)
.expect("bug: map local valid rect");

// The device rect is guaranteed to be aligned on a device pixel - the round
// is just to deal with float accuracy. However, the valid rect is not
// always aligned to a device pixel. To handle this, round out to get all
// required pixels, and intersect with the tile device rect.
let device_rect = (self.world_tile_rect * ctx.global_device_pixel_scale).round();
self.current_descriptor.device_valid_rect = (world_valid_rect * ctx.global_device_pixel_scale)
.round_out()
.intersection(&device_rect)
.unwrap_or_else(DeviceRect::zero);

// Check if this tile can be considered opaque. Opacity state must be updated only
// after all early out checks have been performed. Otherwise, we might miss updating
// the native surface next time this tile becomes visible.
Expand Down Expand Up @@ -1630,6 +1643,9 @@ pub struct TileDescriptor {
/// Picture space rect that contains valid pixels region of this tile.
local_valid_rect: PictureRect,

/// Device space rect that contains valid pixels region of this tile.
pub device_valid_rect: DeviceRect,

/// List of the effects of color that we care about
/// tracking for this tile.
color_bindings: Vec<ColorBinding>,
Expand All @@ -1644,6 +1660,7 @@ impl TileDescriptor {
images: Vec::new(),
transforms: Vec::new(),
local_valid_rect: PictureRect::zero(),
device_valid_rect: DeviceRect::zero(),
color_bindings: Vec::new(),
}
}
Expand Down Expand Up @@ -2308,6 +2325,8 @@ pub struct TileCacheInstance {
frames_until_size_eval: usize,
/// The current fractional offset of the cached picture
fract_offset: PictureVector2D,
/// The current device fractional offset of the cached picture
device_fract_offset: DeviceVector2D,
/// For DirectComposition, virtual surfaces don't support negative coordinates. However,
/// picture cache tile coordinates can be negative. To handle this, we apply an offset
/// to each tile in DirectComposition. We want to change this as little as possible,
Expand Down Expand Up @@ -2383,6 +2402,7 @@ impl TileCacheInstance {
current_tile_size: DeviceIntSize::zero(),
frames_until_size_eval: 0,
fract_offset: PictureVector2D::zero(),
device_fract_offset: DeviceVector2D::zero(),
// Default to centering the virtual offset in the middle of the DC virtual surface
virtual_offset: DeviceIntPoint::new(
params.virtual_surface_size / 2,
Expand Down Expand Up @@ -2632,24 +2652,21 @@ impl TileCacheInstance {
let device_origin = world_origin * frame_context.global_device_pixel_scale;
let desired_device_origin = device_origin.round();
self.device_position = desired_device_origin;
self.device_fract_offset = desired_device_origin - device_origin;

// Unmap from device space to world space rect
let ref_world_rect = WorldRect::new(
desired_device_origin / frame_context.global_device_pixel_scale,
WorldSize::new(1.0, 1.0),
);

// Unmap from world space to picture space
let ref_point = pic_to_world_mapper
// Unmap from world space to picture space; this should be the fractional offset
// required in picture space to align in device space
self.fract_offset = pic_to_world_mapper
.unmap(&ref_world_rect)
.expect("bug: unable to unmap ref world rect")
.origin;

// Extract the fractional offset required in picture space to align in device space
self.fract_offset = PictureVector2D::new(
ref_point.x.fract(),
ref_point.y.fract(),
);
.origin
.to_vector();

// Do a hacky diff of opacity binding values from the last frame. This is
// used later on during tile invalidation tests.
Expand Down Expand Up @@ -2825,6 +2842,7 @@ impl TileCacheInstance {
let ctx = TilePreUpdateContext {
pic_to_world_mapper,
fract_offset: self.fract_offset,
device_fract_offset: self.device_fract_offset,
background_color: self.background_color,
global_screen_world_rect: frame_context.global_screen_world_rect,
tile_size: self.tile_size,
Expand Down Expand Up @@ -5177,7 +5195,9 @@ impl PicturePrimitive {

if tile.is_visible {
// Get the world space rect that this tile will actually occupy on screem
let device_draw_rect = device_clip_rect.intersection(&tile.device_valid_rect);
let device_draw_rect = device_clip_rect.intersection(
&tile.current_descriptor.device_valid_rect,
);

// If that draw rect is occluded by some set of tiles in front of it,
// then mark it as not visible and skip drawing. When it's not occluded
Expand Down Expand Up @@ -5409,7 +5429,8 @@ impl PicturePrimitive {
.round()
.to_i32();

let valid_rect = tile.device_valid_rect
let valid_rect = tile.current_descriptor
.device_valid_rect
.translate(-device_rect.origin.to_vector())
.round()
.to_i32();
Expand Down Expand Up @@ -5460,6 +5481,12 @@ impl PicturePrimitive {
);
}

// If the entire tile valid region is dirty, we can update the fract offset
// at which the tile was rendered.
if tile.device_dirty_rect.contains_rect(&tile.current_descriptor.device_valid_rect) {
tile.device_fract_offset = tile_cache.device_fract_offset;
}

// Now that the tile is valid, reset the dirty rect.
tile.local_dirty_rect = PictureRect::zero();
tile.is_valid = true;
Expand Down Expand Up @@ -5597,7 +5624,7 @@ impl PicturePrimitive {
tile_cache_tiny.tiles.insert(*key, TileSerializer {
rect: tile.local_tile_rect,
current_descriptor: tile.current_descriptor.clone(),
fract_offset: tile.fract_offset,
device_fract_offset: tile.device_fract_offset,
id: tile.id,
root: tile.root.clone(),
background_color: tile.background_color,
Expand Down

0 comments on commit d11c215

Please sign in to comment.