Bug 1894106 - Supply tile rect to composite shader in device space. r=gfx-reviewers,gw

Currently the composite shader takes as input the tile rect in local
space, a local-to-device, transform, and the clip rect in device
space. The shader will then transform the local rect in to device
space and then clip it.

On the CPU side, that clip rect was calculated by transforming the
same tile rect in to device space using the same transform. (And
intersecting with additional rects too). However, performing the same
floating point calculations on the CPU and GPU can produce slightly
different results. Discrepancies between the device rect as calculated
by the CPU and GPU were causing the tiles to occasionally be clipped
fractionally smaller than intended, which was resulting in visible
seams whilst zooming a page.

To fix this, we supply the tile rect in device space to the shader,
meaning the transformation is only ever calculated on the CPU. As the
transform could contain a negative scale to indicate a flip, we
replace it with a "flip" input for each axis.

Differential Revision: https://phabricator.services.mozilla.com/D211989
This commit is contained in:
Jamie Nicol 2024-05-29 20:19:44 +00:00
parent 001af07af3
commit a9a71d0ad7
4 changed files with 37 additions and 50 deletions

View File

@ -45,11 +45,11 @@ uniform mediump vec2 uTextureSize;
// CPU side data is in CompositeInstance (gpu_types.rs) and is // CPU side data is in CompositeInstance (gpu_types.rs) and is
// converted to GPU data using desc::COMPOSITE (renderer.rs) by // converted to GPU data using desc::COMPOSITE (renderer.rs) by
// filling vaos.composite_vao with VertexArrayKind::Composite. // filling vaos.composite_vao with VertexArrayKind::Composite.
PER_INSTANCE attribute vec4 aLocalRect; PER_INSTANCE attribute vec4 aDeviceRect;
PER_INSTANCE attribute vec4 aDeviceClipRect; PER_INSTANCE attribute vec4 aDeviceClipRect;
PER_INSTANCE attribute vec4 aColor; PER_INSTANCE attribute vec4 aColor;
PER_INSTANCE attribute vec4 aParams; PER_INSTANCE attribute vec4 aParams;
PER_INSTANCE attribute vec4 aTransform; PER_INSTANCE attribute vec2 aFlip;
#ifdef WR_FEATURE_YUV #ifdef WR_FEATURE_YUV
// YUV treats these as a UV clip rect (clamp) // YUV treats these as a UV clip rect (clamp)
@ -60,10 +60,6 @@ PER_INSTANCE attribute vec4 aUvRect2;
PER_INSTANCE attribute vec4 aUvRect0; PER_INSTANCE attribute vec4 aUvRect0;
#endif #endif
vec2 apply_transform(vec2 p, vec4 transform) {
return p * transform.xy + transform.zw;
}
#ifdef WR_FEATURE_YUV #ifdef WR_FEATURE_YUV
YuvPrimitive fetch_yuv_primitive() { YuvPrimitive fetch_yuv_primitive() {
// From ExternalSurfaceDependency::Yuv: // From ExternalSurfaceDependency::Yuv:
@ -75,16 +71,17 @@ YuvPrimitive fetch_yuv_primitive() {
#endif #endif
void main(void) { void main(void) {
// Get world position // Flip device rect if required
vec2 world_p0 = apply_transform(aLocalRect.xy, aTransform); vec4 device_rect = mix(aDeviceRect.xyzw, aDeviceRect.zwxy, aFlip.xyxy);
vec2 world_p1 = apply_transform(aLocalRect.zw, aTransform);
vec2 world_pos = mix(world_p0, world_p1, aPosition.xy); // Get world position
vec2 world_pos = mix(device_rect.xy, device_rect.zw, aPosition.xy);
// Clip the position to the world space clip rect // Clip the position to the world space clip rect
vec2 clipped_world_pos = clamp(world_pos, aDeviceClipRect.xy, aDeviceClipRect.zw); vec2 clipped_world_pos = clamp(world_pos, aDeviceClipRect.xy, aDeviceClipRect.zw);
// Derive the normalized UV from the clipped vertex position // Derive the normalized UV from the clipped vertex position
vec2 uv = (clipped_world_pos - world_p0) / (world_p1 - world_p0); vec2 uv = (clipped_world_pos - device_rect.xy) / (device_rect.zw - device_rect.xy);
#ifdef WR_FEATURE_YUV #ifdef WR_FEATURE_YUV
YuvPrimitive prim = fetch_yuv_primitive(); YuvPrimitive prim = fetch_yuv_primitive();

View File

@ -252,17 +252,6 @@ pub struct CompositorTransform {
pub ty: f32, pub ty: f32,
} }
impl CompositorTransform {
pub fn identity() -> Self {
CompositorTransform {
sx: 1.0,
sy: 1.0,
tx: 0.0,
ty: 0.0,
}
}
}
impl From<ScaleOffset> for CompositorTransform { impl From<ScaleOffset> for CompositorTransform {
fn from(scale_offset: ScaleOffset) -> Self { fn from(scale_offset: ScaleOffset) -> Self {
CompositorTransform { CompositorTransform {
@ -280,8 +269,8 @@ impl From<ScaleOffset> for CompositorTransform {
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[repr(C)] #[repr(C)]
pub struct CompositeInstance { pub struct CompositeInstance {
// Picture space destination rectangle of surface // Device space destination rectangle of surface
rect: PictureRect, rect: DeviceRect,
// Device space destination clip rect for this surface // Device space destination clip rect for this surface
clip_rect: DeviceRect, clip_rect: DeviceRect,
// Color for solid color tiles, white otherwise // Color for solid color tiles, white otherwise
@ -297,16 +286,16 @@ pub struct CompositeInstance {
// UV rectangles (pixel space) for color / yuv texture planes // UV rectangles (pixel space) for color / yuv texture planes
uv_rects: [TexelRect; 3], uv_rects: [TexelRect; 3],
// A 2d scale + offset transform for the rect // Whether to flip the x and y axis respectively, where 0.0 is no-flip and 1.0 is flip.
transform: CompositorTransform, flip: (f32, f32),
} }
impl CompositeInstance { impl CompositeInstance {
pub fn new( pub fn new(
rect: PictureRect, rect: DeviceRect,
clip_rect: DeviceRect, clip_rect: DeviceRect,
color: PremultipliedColorF, color: PremultipliedColorF,
transform: CompositorTransform, flip: (bool, bool),
) -> Self { ) -> Self {
let uv = TexelRect::new(0.0, 0.0, 1.0, 1.0); let uv = TexelRect::new(0.0, 0.0, 1.0, 1.0);
CompositeInstance { CompositeInstance {
@ -318,16 +307,16 @@ impl CompositeInstance {
yuv_format: 0.0, yuv_format: 0.0,
yuv_channel_bit_depth: 0.0, yuv_channel_bit_depth: 0.0,
uv_rects: [uv, uv, uv], uv_rects: [uv, uv, uv],
transform, flip: (flip.0.into(), flip.1.into()),
} }
} }
pub fn new_rgb( pub fn new_rgb(
rect: PictureRect, rect: DeviceRect,
clip_rect: DeviceRect, clip_rect: DeviceRect,
color: PremultipliedColorF, color: PremultipliedColorF,
uv_rect: TexelRect, uv_rect: TexelRect,
transform: CompositorTransform, flip: (bool, bool),
) -> Self { ) -> Self {
CompositeInstance { CompositeInstance {
rect, rect,
@ -338,18 +327,18 @@ impl CompositeInstance {
yuv_format: 0.0, yuv_format: 0.0,
yuv_channel_bit_depth: 0.0, yuv_channel_bit_depth: 0.0,
uv_rects: [uv_rect, uv_rect, uv_rect], uv_rects: [uv_rect, uv_rect, uv_rect],
transform, flip: (flip.0.into(), flip.1.into()),
} }
} }
pub fn new_yuv( pub fn new_yuv(
rect: PictureRect, rect: DeviceRect,
clip_rect: DeviceRect, clip_rect: DeviceRect,
yuv_color_space: YuvRangedColorSpace, yuv_color_space: YuvRangedColorSpace,
yuv_format: YuvFormat, yuv_format: YuvFormat,
yuv_channel_bit_depth: u32, yuv_channel_bit_depth: u32,
uv_rects: [TexelRect; 3], uv_rects: [TexelRect; 3],
transform: CompositorTransform, flip: (bool, bool),
) -> Self { ) -> Self {
CompositeInstance { CompositeInstance {
rect, rect,
@ -360,7 +349,7 @@ impl CompositeInstance {
yuv_format: pack_as_float(yuv_format as u32), yuv_format: pack_as_float(yuv_format as u32),
yuv_channel_bit_depth: pack_as_float(yuv_channel_bit_depth), yuv_channel_bit_depth: pack_as_float(yuv_channel_bit_depth),
uv_rects, uv_rects,
transform, flip: (flip.0.into(), flip.1.into()),
} }
} }

View File

@ -70,7 +70,7 @@ use glyph_rasterizer::GlyphFormat;
use crate::gpu_cache::{GpuCacheUpdate, GpuCacheUpdateList}; use crate::gpu_cache::{GpuCacheUpdate, GpuCacheUpdateList};
use crate::gpu_cache::{GpuCacheDebugChunk, GpuCacheDebugCmd}; use crate::gpu_cache::{GpuCacheDebugChunk, GpuCacheDebugCmd};
use crate::gpu_types::{ScalingInstance, SvgFilterInstance, SVGFEFilterInstance, CopyInstance, PrimitiveInstanceData}; use crate::gpu_types::{ScalingInstance, SvgFilterInstance, SVGFEFilterInstance, CopyInstance, PrimitiveInstanceData};
use crate::gpu_types::{BlurInstance, ClearInstance, CompositeInstance, CompositorTransform}; use crate::gpu_types::{BlurInstance, ClearInstance, CompositeInstance};
use crate::internal_types::{TextureSource, TextureCacheCategory, FrameId}; use crate::internal_types::{TextureSource, TextureCacheCategory, FrameId};
#[cfg(any(feature = "capture", feature = "replay"))] #[cfg(any(feature = "capture", feature = "replay"))]
use crate::internal_types::DebugOutput; use crate::internal_types::DebugOutput;
@ -3043,7 +3043,7 @@ impl Renderer {
]; ];
let instance = CompositeInstance::new_yuv( let instance = CompositeInstance::new_yuv(
surface_rect.cast_unit().to_f32(), surface_rect.to_f32(),
surface_rect.to_f32(), surface_rect.to_f32(),
// z-id is not relevant when updating a native compositor surface. // z-id is not relevant when updating a native compositor surface.
// TODO(gw): Support compositor surfaces without z-buffer, for memory / perf win here. // TODO(gw): Support compositor surfaces without z-buffer, for memory / perf win here.
@ -3051,7 +3051,7 @@ impl Renderer {
format, format,
channel_bit_depth, channel_bit_depth,
uv_rects, uv_rects,
CompositorTransform::identity(), (false, false),
); );
( textures, instance ) ( textures, instance )
@ -3074,11 +3074,11 @@ impl Renderer {
let textures = BatchTextures::composite_rgb(plane.texture); let textures = BatchTextures::composite_rgb(plane.texture);
let uv_rect = self.texture_resolver.get_uv_rect(&textures.input.colors[0], plane.uv_rect); let uv_rect = self.texture_resolver.get_uv_rect(&textures.input.colors[0], plane.uv_rect);
let instance = CompositeInstance::new_rgb( let instance = CompositeInstance::new_rgb(
surface_rect.cast_unit().to_f32(), surface_rect.to_f32(),
surface_rect.to_f32(), surface_rect.to_f32(),
PremultipliedColorF::WHITE, PremultipliedColorF::WHITE,
uv_rect, uv_rect,
CompositorTransform::identity(), (false, false),
); );
( textures, instance ) ( textures, instance )
@ -3137,8 +3137,9 @@ impl Renderer {
let tile = &composite_state.tiles[item.key]; let tile = &composite_state.tiles[item.key];
let clip_rect = item.rectangle; let clip_rect = item.rectangle;
let tile_rect = tile.local_rect; let transform = composite_state.get_device_transform(tile.transform_index);
let transform = composite_state.get_device_transform(tile.transform_index).into(); let tile_rect = transform.map_rect(&tile.local_rect);
let flip = (transform.scale.x < 0.0, transform.scale.y < 0.0);
// Work out the draw params based on the tile surface // Work out the draw params based on the tile surface
let (instance, textures, shader_params) = match tile.surface { let (instance, textures, shader_params) = match tile.surface {
@ -3149,7 +3150,7 @@ impl Renderer {
tile_rect, tile_rect,
clip_rect, clip_rect,
color.premultiplied(), color.premultiplied(),
transform, flip,
); );
let features = instance.get_rgb_features(); let features = instance.get_rgb_features();
( (
@ -3163,7 +3164,7 @@ impl Renderer {
tile_rect, tile_rect,
clip_rect, clip_rect,
PremultipliedColorF::WHITE, PremultipliedColorF::WHITE,
transform, flip,
); );
let features = instance.get_rgb_features(); let features = instance.get_rgb_features();
( (
@ -3207,7 +3208,7 @@ impl Renderer {
format, format,
channel_bit_depth, channel_bit_depth,
uv_rects, uv_rects,
transform, flip,
), ),
textures, textures,
( (
@ -3225,7 +3226,7 @@ impl Renderer {
clip_rect, clip_rect,
PremultipliedColorF::WHITE, PremultipliedColorF::WHITE,
uv_rect, uv_rect,
transform, flip,
); );
let features = instance.get_rgb_features(); let features = instance.get_rgb_features();
( (
@ -3248,7 +3249,7 @@ impl Renderer {
tile_rect, tile_rect,
clip_rect, clip_rect,
PremultipliedColorF::BLACK, PremultipliedColorF::BLACK,
transform, flip,
); );
let features = instance.get_rgb_features(); let features = instance.get_rgb_features();
( (

View File

@ -725,7 +725,7 @@ pub mod desc {
}], }],
instance_attributes: &[ instance_attributes: &[
VertexAttribute { VertexAttribute {
name: "aLocalRect", name: "aDeviceRect",
count: 4, count: 4,
kind: VertexAttributeKind::F32, kind: VertexAttributeKind::F32,
}, },
@ -760,8 +760,8 @@ pub mod desc {
kind: VertexAttributeKind::F32, kind: VertexAttributeKind::F32,
}, },
VertexAttribute { VertexAttribute {
name: "aTransform", name: "aFlip",
count: 4, count: 2,
kind: VertexAttributeKind::F32, kind: VertexAttributeKind::F32,
}, },
], ],