From 1612260124f7f54e22c28357fb9946aa4ad64ce5 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:11 +0000 Subject: [PATCH 01/21] Bug 1594091 - Move user_data to the glyph instance w field. r=gw First patch in of series that will rearrange the layout of the glyph instance attributes so that it matches brush instances. This will be needed to add a unified shader that can render the most common alpha pass primitives, including text. Differential Revision: https://phabricator.services.mozilla.com/D51879 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/570d5ec05d035d9fb156c8cc336ad66885b21de6 --- webrender/res/ps_text_run.glsl | 8 ++++---- webrender/src/batch.rs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index 40dbdae26f..722078a5e3 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -177,10 +177,10 @@ void main(void) { int prim_header_address = aData.x; int glyph_index = aData.y & 0xffff; int render_task_index = aData.y >> 16; - int resource_address = aData.z; - int raster_space = aData.w >> 16; - int subpx_dir = (aData.w >> 8) & 0xff; - int color_mode = aData.w & 0xff; + int raster_space = aData.z >> 16; + int subpx_dir = (aData.z >> 8) & 0xff; + int color_mode = aData.z & 0xff; + int resource_address = aData.w; PrimitiveHeader ph = fetch_prim_header(prim_header_address); Transform transform = fetch_transform(ph.transform_id); diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 720c829dc5..2a48b20c76 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -980,10 +980,10 @@ impl BatchBuilder { for glyph in glyphs { batch.push(base_instance.build( glyph.index_in_text_run | ((render_task_address.0 as i32) << 16), + (rasterization_space as i32) << 16 + | (subpx_dir as u32 as i32) << 8 + | (color_mode as u32 as i32), glyph.uv_rect_address.as_int(), - (rasterization_space as i32) << 16 | - (subpx_dir as u32 as i32) << 8 | - (color_mode as u32 as i32), )); } } From fc648c6f083af48c03a4f405d555c987865b2cdc Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:19 +0000 Subject: [PATCH 02/21] Bug 1594091 - Move color_mode and subpx_dir to the first half of the glyph instance z field. r=gw Depends on D51879 Differential Revision: https://phabricator.services.mozilla.com/D51880 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/e405048af859fb0b8aca0060ac8c0df5388ee630 --- webrender/res/ps_text_run.glsl | 7 ++++--- webrender/src/batch.rs | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index 722078a5e3..dd5093c302 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -177,11 +177,12 @@ void main(void) { int prim_header_address = aData.x; int glyph_index = aData.y & 0xffff; int render_task_index = aData.y >> 16; - int raster_space = aData.z >> 16; - int subpx_dir = (aData.z >> 8) & 0xff; - int color_mode = aData.z & 0xff; + int raster_space = aData.z & 0xffff; + int subpx_dir = (aData.z >> 24) & 0xff; + int color_mode = (aData.z >> 16) & 0xff; int resource_address = aData.w; + PrimitiveHeader ph = fetch_prim_header(prim_header_address); Transform transform = fetch_transform(ph.transform_id); ClipArea clip_area = fetch_clip_area(ph.user_data.w); diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 2a48b20c76..2b33e2de7c 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -980,9 +980,9 @@ impl BatchBuilder { for glyph in glyphs { batch.push(base_instance.build( glyph.index_in_text_run | ((render_task_address.0 as i32) << 16), - (rasterization_space as i32) << 16 - | (subpx_dir as u32 as i32) << 8 - | (color_mode as u32 as i32), + (subpx_dir as u32 as i32) << 24 + | (color_mode as u32 as i32) << 16 + | (rasterization_space as i32), glyph.uv_rect_address.as_int(), )); } From dc1a52faa3bfdb3572dbe98e991ed75895751765 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:29 +0000 Subject: [PATCH 03/21] Bug 1594091 - Move glyph_index to the second half of the glyph instance z field. r=gw Depends on D51880 Differential Revision: https://phabricator.services.mozilla.com/D51882 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/047d8fd366a4f92f165244e9de598b0890c3dacd --- webrender/res/ps_text_run.glsl | 4 ++-- webrender/src/batch.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index dd5093c302..757a36e2f8 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -175,9 +175,9 @@ VertexInfo write_text_vertex(RectWithSize local_clip_rect, void main(void) { int prim_header_address = aData.x; - int glyph_index = aData.y & 0xffff; int render_task_index = aData.y >> 16; - int raster_space = aData.z & 0xffff; + int raster_space = aData.y & 0xffff; + int glyph_index = aData.z & 0xffff; int subpx_dir = (aData.z >> 24) & 0xff; int color_mode = (aData.z >> 16) & 0xff; int resource_address = aData.w; diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 2b33e2de7c..1bda7e23e1 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -979,10 +979,10 @@ impl BatchBuilder { for glyph in glyphs { batch.push(base_instance.build( - glyph.index_in_text_run | ((render_task_address.0 as i32) << 16), + (rasterization_space as i32) | ((render_task_address.0 as i32) << 16), (subpx_dir as u32 as i32) << 24 | (color_mode as u32 as i32) << 16 - | (rasterization_space as i32), + | glyph.index_in_text_run, glyph.uv_rect_address.as_int(), )); } From b96878d21493630e6c62cb533b1fe6a4d685bb90 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:37 +0000 Subject: [PATCH 04/21] Bug 1594091 - Move clip_task_address to the second half of the glyph instance y field. r=gw Depends on D51882 Differential Revision: https://phabricator.services.mozilla.com/D51883 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/c86523dab3c48d8185787f09338b57efeb3f86b0 --- webrender/res/ps_text_run.glsl | 6 ++++-- webrender/src/batch.rs | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index 757a36e2f8..fe46bd2546 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -176,7 +176,7 @@ VertexInfo write_text_vertex(RectWithSize local_clip_rect, void main(void) { int prim_header_address = aData.x; int render_task_index = aData.y >> 16; - int raster_space = aData.y & 0xffff; + int clip_address = aData.y & 0xffff; int glyph_index = aData.z & 0xffff; int subpx_dir = (aData.z >> 24) & 0xff; int color_mode = (aData.z >> 16) & 0xff; @@ -185,9 +185,11 @@ void main(void) { PrimitiveHeader ph = fetch_prim_header(prim_header_address); Transform transform = fetch_transform(ph.transform_id); - ClipArea clip_area = fetch_clip_area(ph.user_data.w); + ClipArea clip_area = fetch_clip_area(clip_address); PictureTask task = fetch_picture_task(render_task_index); + int raster_space = ph.user_data.w; + TextRun text = fetch_text_run(ph.specific_prim_address); vec2 text_offset = vec2(ph.user_data.xy) / 256.0; diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index 1bda7e23e1..a412d7af29 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -891,7 +891,7 @@ impl BatchBuilder { (run.reference_frame_relative_offset.x * 256.0) as i32, (run.reference_frame_relative_offset.y * 256.0) as i32, (raster_scale * 65535.0).round() as i32, - clip_task_address.unwrap().0 as i32, + (rasterization_space as i32), ], ); let base_instance = GlyphInstance::new( @@ -979,7 +979,8 @@ impl BatchBuilder { for glyph in glyphs { batch.push(base_instance.build( - (rasterization_space as i32) | ((render_task_address.0 as i32) << 16), + ((render_task_address.0 as i32) << 16) + | clip_task_address.unwrap().0 as i32, (subpx_dir as u32 as i32) << 24 | (color_mode as u32 as i32) << 16 | glyph.index_in_text_run, From 8d76cbd647f2ddfc12a5774de514b97542171d94 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:46 +0000 Subject: [PATCH 05/21] Bug 1594091 - Unify text and brush instance attribute decoding. r=gw Depends on D51883 Differential Revision: https://phabricator.services.mozilla.com/D51884 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/f4b58dd6ef384634dc04b1a462e46d6ae246286a --- webrender/res/brush.glsl | 22 +++++++++------------- webrender/res/prim_shared.glsl | 23 +++++++++++++++++++++++ webrender/res/ps_text_run.glsl | 18 ++++++++---------- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/webrender/res/brush.glsl b/webrender/res/brush.glsl index 83c777b8db..514f54400e 100644 --- a/webrender/res/brush.glsl +++ b/webrender/res/brush.glsl @@ -29,25 +29,21 @@ void brush_vs( void main(void) { // Load the brush instance from vertex attributes. - int prim_header_address = aData.x; - int render_task_index = aData.y >> 16; - int clip_address = aData.y & 0xffff; - int segment_index = aData.z & 0xffff; - int edge_flags = (aData.z >> 16) & 0xff; - int brush_flags = (aData.z >> 24) & 0xff; - int segment_user_data = aData.w; - PrimitiveHeader ph = fetch_prim_header(prim_header_address); + Instance instance = decode_instance_attributes(); + int edge_flags = (instance.flags >> 16) & 0xff; + int brush_flags = (instance.flags >> 24) & 0xff; + PrimitiveHeader ph = fetch_prim_header(instance.prim_header_address); // Fetch the segment of this brush primitive we are drawing. vec4 segment_data; RectWithSize segment_rect; - if (segment_index == INVALID_SEGMENT_INDEX) { + if (instance.segment_index == INVALID_SEGMENT_INDEX) { segment_rect = ph.local_rect; segment_data = vec4(0.0); } else { int segment_address = ph.specific_prim_address + VECS_PER_SPECIFIC_BRUSH + - segment_index * VECS_PER_SEGMENT; + instance.segment_index * VECS_PER_SEGMENT; vec4[2] segment_info = fetch_from_gpu_cache_2(segment_address); segment_rect = RectWithSize(segment_info[0].xy, segment_info[0].zw); @@ -58,8 +54,8 @@ void main(void) { VertexInfo vi; // Fetch the dynamic picture that we are drawing on. - PictureTask pic_task = fetch_picture_task(render_task_index); - ClipArea clip_area = fetch_clip_area(clip_address); + PictureTask pic_task = fetch_picture_task(instance.picture_task_address); + ClipArea clip_area = fetch_clip_area(instance.clip_address); Transform transform = fetch_transform(ph.transform_id); @@ -117,7 +113,7 @@ void main(void) { ph.local_rect, segment_rect, ph.user_data, - segment_user_data, + instance.user_data, transform.m, pic_task, brush_flags, diff --git a/webrender/res/prim_shared.glsl b/webrender/res/prim_shared.glsl index cb52464b13..de0ce1f1ee 100644 --- a/webrender/res/prim_shared.glsl +++ b/webrender/res/prim_shared.glsl @@ -50,6 +50,29 @@ in ivec4 aData; #define VECS_PER_PRIM_HEADER_F 2U #define VECS_PER_PRIM_HEADER_I 2U +struct Instance +{ + int prim_header_address; + int picture_task_address; + int clip_address; + int segment_index; + int flags; + int user_data; +}; + +Instance decode_instance_attributes() { + Instance instance; + + instance.prim_header_address = aData.x; + instance.picture_task_address = aData.y >> 16; + instance.clip_address = aData.y & 0xffff; + instance.segment_index = aData.z & 0xffff; + instance.flags = aData.z & 0xffff0000; + instance.user_data = aData.w; + + return instance; +} + struct PrimitiveHeader { RectWithSize local_rect; RectWithSize local_clip_rect; diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index fe46bd2546..49714f792c 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -174,19 +174,17 @@ VertexInfo write_text_vertex(RectWithSize local_clip_rect, } void main(void) { - int prim_header_address = aData.x; - int render_task_index = aData.y >> 16; - int clip_address = aData.y & 0xffff; - int glyph_index = aData.z & 0xffff; - int subpx_dir = (aData.z >> 24) & 0xff; - int color_mode = (aData.z >> 16) & 0xff; - int resource_address = aData.w; + Instance instance = decode_instance_attributes(); + int glyph_index = instance.segment_index; + int subpx_dir = (instance.flags >> 24) & 0xff; + int color_mode = (instance.flags >> 16) & 0xff; + int resource_address = instance.user_data; - PrimitiveHeader ph = fetch_prim_header(prim_header_address); + PrimitiveHeader ph = fetch_prim_header(instance.prim_header_address); Transform transform = fetch_transform(ph.transform_id); - ClipArea clip_area = fetch_clip_area(clip_address); - PictureTask task = fetch_picture_task(render_task_index); + ClipArea clip_area = fetch_clip_area(instance.clip_address); + PictureTask task = fetch_picture_task(instance.picture_task_address); int raster_space = ph.user_data.w; From bc06c579f2ef0b7ddd9cc0f818937db99b856709 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 6 Nov 2019 22:02:54 +0000 Subject: [PATCH 06/21] Bug 1594091 - Rename the instance user_data field into specific_resource_address. r=gw This user_data field is currently only used as an address for some shader-specific resource in the gpu cache. We can always rename it back to something generic if we ever need to use the bits differently in other shaders in the future. Depends on D51884 Differential Revision: https://phabricator.services.mozilla.com/D51885 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/cd13f04835f4d9ebdfccd14b5f4afe6a8a8a2dbe --- webrender/res/brush.glsl | 2 +- webrender/res/brush_blend.glsl | 2 +- webrender/res/brush_image.glsl | 6 +++--- webrender/res/brush_linear_gradient.glsl | 2 +- webrender/res/brush_mix_blend.glsl | 2 +- webrender/res/brush_radial_gradient.glsl | 2 +- webrender/res/brush_solid.glsl | 2 +- webrender/res/brush_yuv_image.glsl | 2 +- webrender/src/batch.rs | 20 ++++++++++---------- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/webrender/res/brush.glsl b/webrender/res/brush.glsl index 514f54400e..b1012b6323 100644 --- a/webrender/res/brush.glsl +++ b/webrender/res/brush.glsl @@ -10,7 +10,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_blend.glsl b/webrender/res/brush_blend.glsl index d53686345a..46e44e95da 100644 --- a/webrender/res/brush_blend.glsl +++ b/webrender/res/brush_blend.glsl @@ -35,7 +35,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_image.glsl b/webrender/res/brush_image.glsl index ced1c17ca4..5a7062657d 100644 --- a/webrender/res/brush_image.glsl +++ b/webrender/res/brush_image.glsl @@ -54,7 +54,7 @@ void brush_vs( RectWithSize prim_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, @@ -70,7 +70,7 @@ void brush_vs( vec2 texture_size = vec2(textureSize(sColor0, 0)); #endif - ImageResource res = fetch_image_resource(segment_user_data); + ImageResource res = fetch_image_resource(specific_resource_address); vec2 uv0 = res.uv_rect.p0; vec2 uv1 = res.uv_rect.p1; @@ -139,7 +139,7 @@ void brush_vs( // Since the screen space UVs specify an arbitrary quad, do // a bilinear interpolation to get the correct UV for this // local position. - f = get_image_quad_uv(segment_user_data, f); + f = get_image_quad_uv(specific_resource_address, f); break; } default: diff --git a/webrender/res/brush_linear_gradient.glsl b/webrender/res/brush_linear_gradient.glsl index cbf6e4b85d..c605e2d8cf 100644 --- a/webrender/res/brush_linear_gradient.glsl +++ b/webrender/res/brush_linear_gradient.glsl @@ -46,7 +46,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_mix_blend.glsl b/webrender/res/brush_mix_blend.glsl index 91acc186e5..9c7dfdcb48 100644 --- a/webrender/res/brush_mix_blend.glsl +++ b/webrender/res/brush_mix_blend.glsl @@ -23,7 +23,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_radial_gradient.glsl b/webrender/res/brush_radial_gradient.glsl index 0659fad75f..a4f0e35eee 100644 --- a/webrender/res/brush_radial_gradient.glsl +++ b/webrender/res/brush_radial_gradient.glsl @@ -46,7 +46,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_solid.glsl b/webrender/res/brush_solid.glsl index c8343843c4..26b7227e00 100644 --- a/webrender/res/brush_solid.glsl +++ b/webrender/res/brush_solid.glsl @@ -29,7 +29,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/res/brush_yuv_image.glsl b/webrender/res/brush_yuv_image.glsl index 646ce50bfb..3856fc46d1 100644 --- a/webrender/res/brush_yuv_image.glsl +++ b/webrender/res/brush_yuv_image.glsl @@ -126,7 +126,7 @@ void brush_vs( RectWithSize local_rect, RectWithSize segment_rect, ivec4 prim_user_data, - int segment_user_data, + int specific_resource_address, mat4 transform, PictureTask pic_task, int brush_flags, diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index a412d7af29..a7aaf052c9 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -448,7 +448,7 @@ impl AlphaBatchContainer { #[derive(Debug, Copy, Clone)] struct SegmentInstanceData { textures: BatchTextures, - user_data: i32, + specific_resource_address: i32, } /// Encapsulates the logic of building batches for items that are blended. @@ -806,7 +806,7 @@ impl BatchBuilder { segment_data.push( SegmentInstanceData { textures: BatchTextures::color(cache_item.texture_id), - user_data: cache_item.uv_rect_handle.as_int(gpu_cache), + specific_resource_address: cache_item.uv_rect_handle.as_int(gpu_cache), } ); } @@ -998,7 +998,7 @@ impl BatchBuilder { let common_data = &ctx.data_stores.line_decoration[data_handle].common; let prim_cache_address = gpu_cache.get_address(&common_data.gpu_cache_handle); - let (batch_kind, textures, prim_user_data, segment_user_data) = match cache_handle { + let (batch_kind, textures, prim_user_data, specific_resource_address) = match cache_handle { Some(cache_handle) => { let rt_cache_entry = ctx .resource_cache @@ -1070,7 +1070,7 @@ impl BatchBuilder { clip_task_address.unwrap(), BrushFlags::PERSPECTIVE_INTERPOLATION, prim_header_index, - segment_user_data, + specific_resource_address, prim_vis_mask, ); } @@ -2163,7 +2163,7 @@ impl BatchBuilder { get_shader_opacity(1.0), 0, ]; - let segment_user_data = cache_item.uv_rect_handle.as_int(gpu_cache); + let specific_resource_address = cache_item.uv_rect_handle.as_int(gpu_cache); prim_header.specific_prim_address = gpu_cache.get_address(&ctx.globals.default_image_handle); let prim_header_index = prim_headers.push( @@ -2188,7 +2188,7 @@ impl BatchBuilder { clip_task_address.unwrap(), BrushFlags::PERSPECTIVE_INTERPOLATION, prim_header_index, - segment_user_data, + specific_resource_address, prim_vis_mask, ); } else if gradient.visible_tiles_range.is_empty() { @@ -2450,7 +2450,7 @@ impl BatchBuilder { clip_task_address, BrushFlags::PERSPECTIVE_INTERPOLATION | segment.brush_flags, prim_header_index, - segment_data.user_data, + segment_data.specific_resource_address, prim_vis_mask, ); } @@ -2550,7 +2550,7 @@ impl BatchBuilder { clip_task_address, BrushFlags::PERSPECTIVE_INTERPOLATION, prim_header_index, - segment_data.user_data, + segment_data.specific_resource_address, prim_vis_mask, ); } @@ -2672,7 +2672,7 @@ impl BrushBatchParameters { batch_kind: BrushBatchKind, textures: BatchTextures, prim_user_data: [i32; 4], - segment_user_data: i32, + specific_resource_address: i32, ) -> Self { BrushBatchParameters { batch_kind, @@ -2680,7 +2680,7 @@ impl BrushBatchParameters { segment_data: SegmentDataKind::Shared( SegmentInstanceData { textures, - user_data: segment_user_data, + specific_resource_address, } ), } From eee1cf30cbaab91be9edc58eea630dcd91fb6d75 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 6 Nov 2019 22:03:03 +0000 Subject: [PATCH 07/21] Bug 1575258 - Make text rasterize, render and snap glyphs consistently. r=lsalzman The glyph pixel space in which we rasterized glyphs differed from how we rendered the rasterized glyphs in the shader. They need to be in agreement because the glyph subpixel offset selected during rasterization depends on it. This patch should make the paths consistent with each other. Additionally, during animations, we now snap the reference frame relative offset ignoring the impact of any animated transforms. This helps with minimizing glyph wiggling during the transition. Differential Revision: https://phabricator.services.mozilla.com/D51305 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/1e12f9b1812772fee75f80a38fd27b5fff8cffbc --- webrender/res/ps_text_run.glsl | 123 ++++++++++++++------------- webrender/src/batch.rs | 12 +-- webrender/src/prim_store/mod.rs | 17 +++- webrender/src/prim_store/text_run.rs | 49 +++++++++-- webrender/src/scene_building.rs | 6 ++ webrender/src/util.rs | 14 +++ wrench/reftests/text/reftest.list | 2 +- 7 files changed, 142 insertions(+), 81 deletions(-) diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index 49714f792c..4df830d04c 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -75,66 +75,40 @@ TextRun fetch_text_run(int address) { VertexInfo write_text_vertex(RectWithSize local_clip_rect, float z, - int raster_space, +#ifdef WR_FEATURE_GLYPH_TRANSFORM + mat2 glyph_transform, +#else + float glyph_scale, +#endif Transform transform, PictureTask task, vec2 text_offset, vec2 glyph_offset, RectWithSize glyph_rect, vec2 snap_bias) { - // The offset to snap the glyph rect to a device pixel - vec2 snap_offset = vec2(0.0); - // Transform from glyph space to local space - mat2 glyph_transform_inv = mat2(1.0); - + // Glyph space refers to the pixel space used by glyph rasterization during frame + // building. If a non-identity transform was used, WR_FEATURE_GLYPH_TRANSFORM will + // be set. Otherwise, regardless of whether the raster space is LOCAL or SCREEN, + // we ignored the transform during glyph rasterization, and need to snap just using + // the device pixel scale and the raster scale. #ifdef WR_FEATURE_GLYPH_TRANSFORM - bool remove_subpx_offset = true; -#else - bool remove_subpx_offset = transform.is_axis_aligned; -#endif - // Compute the snapping offset only if the scroll node transform is axis-aligned. - if (remove_subpx_offset) { - // Be careful to only snap with the transform when in screen raster space. - switch (raster_space) { - case RASTER_SCREEN: { - // Transform from local space to glyph space. - float device_scale = task.device_pixel_scale / transform.m[3].w; - mat2 glyph_transform = mat2(transform.m) * device_scale; - - // Ensure the transformed text offset does not contain a subpixel translation - // such that glyph snapping is stable for equivalent glyph subpixel positions. - vec2 device_text_pos = glyph_transform * text_offset + transform.m[3].xy * device_scale; - snap_offset = floor(device_text_pos + 0.5) - device_text_pos; - - // Snap the glyph offset to a device pixel, using an appropriate bias depending - // on whether subpixel positioning is required. - vec2 device_glyph_offset = glyph_transform * glyph_offset; - snap_offset += floor(device_glyph_offset + snap_bias) - device_glyph_offset; - - // Transform from glyph space back to local space. - glyph_transform_inv = inverse(glyph_transform); - -#ifndef WR_FEATURE_GLYPH_TRANSFORM - // If not using transformed subpixels, the glyph rect is actually in local space. - // So convert the snap offset back to local space. - snap_offset = glyph_transform_inv * snap_offset; -#endif - break; - } - default: { - // Otherwise, when in local raster space, the transform may be animated, so avoid - // snapping with the transform to avoid oscillation. - snap_offset = floor(text_offset + 0.5) - text_offset; - snap_offset += floor(glyph_offset + snap_bias) - glyph_offset; - break; - } - } - } + // Transform from glyph space back to local space. + mat2 glyph_transform_inv = inverse(glyph_transform); + + // Glyph raster pixels include the impact of the transform. This path can only be + // entered for 3d transforms that can be coerced into a 2d transform; they have no + // perspective, and have a 2d inverse. This is a looser condition than axis aligned + // transforms because it also allows 2d rotations. + vec2 raster_glyph_offset = glyph_transform * glyph_offset; + vec2 raster_snap_offset = floor(raster_glyph_offset + snap_bias) - raster_glyph_offset; + vec2 local_snap_offset = glyph_transform_inv * raster_snap_offset; + + // We want to eliminate any subpixel translation in device space to ensure glyph + // snapping is stable for equivalent glyph subpixel positions. Note that we must use + // device pixels, and not glyph raster pixels for this purpose. + vec2 device_text_pos = (transform.m * vec4(text_offset, 0.0, 1.0)).xy * task.device_pixel_scale; + vec2 device_snap_offset = floor(device_text_pos + 0.5) - device_text_pos; - // Actually translate the glyph rect to a device pixel using the snap offset. - glyph_rect.p0 += snap_offset; - -#ifdef WR_FEATURE_GLYPH_TRANSFORM // The glyph rect is in device space, so transform it back to local space. RectWithSize local_rect = transform_rect(glyph_rect, glyph_transform_inv); @@ -148,6 +122,29 @@ VertexInfo write_text_vertex(RectWithSize local_clip_rect, local_pos = glyph_transform_inv * (glyph_rect.p0 + glyph_rect.size * aPosition.xy); } #else + // Glyph raster pixels do not include the impact of the transform. Instead it was + // replaced with an identity transform during glyph rasterization. As such only the + // impact of the raster scale (if in local space) and the device pixel scale (for both + // local and screen space) are included. + // + // This implies one or more of the following conditions: + // - The transform is an identity. In that case, setting WR_FEATURE_GLYPH_TRANSFORM + // should have the same output result as not. We just distingush which path to use + // based on the transform used during glyph rasterization. (Screen space). + // - The transform contains an animation. We will imply local raster space in such + // cases to avoid constantly rerasterizing the glyphs. + // - The transform has perspective or does not have a 2d inverse (Screen or local space). + // - The transform's scale will result in result in very large rasterized glyphs and + // we clamped the size. This will imply local raster space. + vec2 raster_glyph_offset = glyph_offset * glyph_scale; + vec2 raster_snap_offset = floor(raster_glyph_offset + snap_bias) - raster_glyph_offset; + vec2 local_snap_offset = raster_snap_offset / glyph_scale; + + // The transform may be animated, so we don't want to do any snapping here for the + // text offset to avoid glyphs wiggling. The text offset should have been snapped + // already for axis aligned transforms excluding any animations during frame building. + vec2 device_snap_offset = vec2(0.0); + // Select the corner of the glyph rect that we are processing. vec2 local_pos = glyph_rect.p0 + glyph_rect.size * aPosition.xy; #endif @@ -158,15 +155,17 @@ VertexInfo write_text_vertex(RectWithSize local_clip_rect, // Map the clamped local space corner into device space. vec4 world_pos = transform.m * vec4(local_pos, 0.0, 1.0); vec2 device_pos = world_pos.xy * task.device_pixel_scale; + vec4 snapped_world_pos = transform.m * vec4(local_pos + local_snap_offset, 0.0, 1.0); + vec2 snapped_device_pos = snapped_world_pos.xy * task.device_pixel_scale + device_snap_offset * snapped_world_pos.w; // Apply offsets for the render task to get correct screen location. vec2 final_offset = -task.content_origin + task.common_data.task_rect.p0; - gl_Position = uTransform * vec4(device_pos + final_offset * world_pos.w, z * world_pos.w, world_pos.w); + gl_Position = uTransform * vec4(snapped_device_pos + final_offset * world_pos.w, z * world_pos.w, world_pos.w); VertexInfo vi = VertexInfo( local_pos, - snap_offset, + snapped_device_pos - device_pos, world_pos ); @@ -186,8 +185,6 @@ void main(void) { ClipArea clip_area = fetch_clip_area(instance.clip_address); PictureTask task = fetch_picture_task(instance.picture_task_address); - int raster_space = ph.user_data.w; - TextRun text = fetch_text_run(ph.specific_prim_address); vec2 text_offset = vec2(ph.user_data.xy) / 256.0; @@ -211,11 +208,12 @@ void main(void) { float raster_scale = float(ph.user_data.z) / 65535.0; // Scale from glyph space to local space. - float scale = res.scale / (raster_scale * task.device_pixel_scale); + float glyph_scale_inv = res.scale / (raster_scale * task.device_pixel_scale); + float glyph_scale = 1.0 / glyph_scale_inv; // Compute the glyph rect in local space. - RectWithSize glyph_rect = RectWithSize(scale * res.offset + text_offset + glyph.offset, - scale * (res.uv_rect.zw - res.uv_rect.xy)); + RectWithSize glyph_rect = RectWithSize(glyph_scale_inv * res.offset + text_offset + glyph.offset, + glyph_scale_inv * (res.uv_rect.zw - res.uv_rect.xy)); #endif vec2 snap_bias; @@ -245,14 +243,17 @@ void main(void) { VertexInfo vi = write_text_vertex(ph.local_clip_rect, ph.z, - raster_space, +#ifdef WR_FEATURE_GLYPH_TRANSFORM + glyph_transform, +#else + glyph_scale, +#endif transform, task, text_offset, glyph.offset, glyph_rect, snap_bias); - glyph_rect.p0 += vi.snap_offset; #ifdef WR_FEATURE_GLYPH_TRANSFORM vec2 f = (glyph_transform * vi.local_pos - glyph_rect.p0) / glyph_rect.size; diff --git a/webrender/src/batch.rs b/webrender/src/batch.rs index a7aaf052c9..e0e7d63eba 100644 --- a/webrender/src/batch.rs +++ b/webrender/src/batch.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use api::{AlphaType, ClipMode, ExternalImageType, ImageRendering}; -use api::{YuvColorSpace, YuvFormat, ColorDepth, ColorRange, PremultipliedColorF, RasterSpace}; +use api::{YuvColorSpace, YuvFormat, ColorDepth, ColorRange, PremultipliedColorF}; use api::units::*; use crate::clip::{ClipDataStore, ClipNodeFlags, ClipNodeRange, ClipItemKind, ClipStore}; use crate::clip_scroll_tree::{ClipScrollTree, ROOT_SPATIAL_NODE_INDEX, SpatialNodeIndex, CoordinateSystemId}; @@ -879,19 +879,15 @@ impl BatchBuilder { }; let glyph_keys = &ctx.scratch.glyph_keys[run.glyph_keys_range]; - let rasterization_space = match run.raster_space { - RasterSpace::Screen => RasterizationSpace::Screen, - RasterSpace::Local(..) => RasterizationSpace::Local, - }; let raster_scale = run.raster_space.local_scale().unwrap_or(1.0).max(0.001); let prim_header_index = prim_headers.push( &prim_header, z_id, [ - (run.reference_frame_relative_offset.x * 256.0) as i32, - (run.reference_frame_relative_offset.y * 256.0) as i32, + (run.snapped_reference_frame_relative_offset.x * 256.0) as i32, + (run.snapped_reference_frame_relative_offset.y * 256.0) as i32, (raster_scale * 65535.0).round() as i32, - (rasterization_space as i32), + 0, ], ); let base_instance = GlyphInstance::new( diff --git a/webrender/src/prim_store/mod.rs b/webrender/src/prim_store/mod.rs index 6891b70d8b..439aa7bcb4 100644 --- a/webrender/src/prim_store/mod.rs +++ b/webrender/src/prim_store/mod.rs @@ -16,7 +16,7 @@ use crate::clip::{ClipDataStore, ClipNodeFlags, ClipChainId, ClipChainInstance, use crate::debug_colors; use crate::debug_render::DebugItem; use crate::scene_building::{CreateShadow, IsVisible}; -use euclid::{SideOffsets2D, Transform3D, Rect, Scale, Size2D, Point2D}; +use euclid::{SideOffsets2D, Transform3D, Rect, Scale, Size2D, Point2D, Vector2D}; use euclid::approxeq::ApproxEq; use crate::frame_builder::{FrameBuildingContext, FrameBuildingState, PictureContext, PictureState}; use crate::frame_builder::{FrameVisibilityContext, FrameVisibilityState}; @@ -52,7 +52,7 @@ use std::{cmp, fmt, hash, ops, u32, usize, mem}; use std::sync::atomic::{AtomicUsize, Ordering}; use crate::storage; use crate::texture_cache::TEXTURE_REGION_DIMENSIONS; -use crate::util::{MatrixHelpers, MaxRect, Recycler, ScaleOffset, RectHelpers}; +use crate::util::{MatrixHelpers, MaxRect, Recycler, ScaleOffset, RectHelpers, VectorHelpers}; use crate::util::{clamp_to_scale_factor, pack_as_float, project_rect, raster_rect_to_device_pixels}; use crate::internal_types::{LayoutPrimitiveInfo, Filter}; use smallvec::SmallVec; @@ -205,6 +205,17 @@ impl SpaceSnapper { } } + pub fn snap_vector(&self, vector: &Vector2D) -> Vector2D where F: fmt::Debug { + debug_assert!(self.current_target_spatial_node_index != SpatialNodeIndex::INVALID); + match self.snapping_transform { + Some(ref scale_offset) => { + let snapped_device_vector : DeviceVector2D = scale_offset.map_vector(&vector).snap(); + scale_offset.unmap_vector(&snapped_device_vector) + } + None => *vector, + } + } + pub fn snap_size(&self, size: &Size2D) -> Size2D where F: fmt::Debug { debug_assert!(self.current_target_spatial_node_index != SpatialNodeIndex::INVALID); match self.snapping_transform { @@ -2911,11 +2922,13 @@ impl PrimitiveStore { &prim_data.glyphs, &transform.to_transform().with_destination::<_>(), surface, + prim_spatial_node_index, raster_space, pic_context.subpixel_mode, frame_state.resource_cache, frame_state.gpu_cache, frame_state.render_tasks, + frame_context.clip_scroll_tree, scratch, ); diff --git a/webrender/src/prim_store/text_run.rs b/webrender/src/prim_store/text_run.rs index cda289bb92..0a4b69e88c 100644 --- a/webrender/src/prim_store/text_run.rs +++ b/webrender/src/prim_store/text_run.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use api::{ColorF, GlyphInstance, RasterSpace, Shadow}; -use api::units::{DevicePixelScale, LayoutToWorldTransform, LayoutVector2D}; +use api::units::{LayoutToWorldTransform, LayoutVector2D}; use crate::scene_building::{CreateShadow, IsVisible}; use crate::frame_builder::FrameBuildingState; use crate::glyph_rasterizer::{FontInstance, FontTransform, GlyphKey, FONT_SIZE_LIMIT}; @@ -17,7 +17,8 @@ use crate::render_task_graph::RenderTaskGraph; use crate::renderer::{MAX_VERTEX_TEXTURE_WIDTH}; use crate::resource_cache::{ResourceCache}; use crate::util::{MatrixHelpers}; -use crate::prim_store::{InternablePrimitive, PrimitiveInstanceKind}; +use crate::prim_store::{InternablePrimitive, PrimitiveInstanceKind, SpaceSnapper}; +use crate::clip_scroll_tree::{ClipScrollTree, SpatialNodeIndex}; use std::ops; use std::sync::Arc; use crate::storage; @@ -174,6 +175,7 @@ impl InternablePrimitive for TextRun { used_font: key.font.clone(), glyph_keys_range: storage::Range::empty(), reference_frame_relative_offset, + snapped_reference_frame_relative_offset: reference_frame_relative_offset, shadow: key.shadow, raster_space: RasterSpace::Screen, }); @@ -212,6 +214,7 @@ pub struct TextRunPrimitive { pub used_font: FontInstance, pub glyph_keys_range: storage::Range, pub reference_frame_relative_offset: LayoutVector2D, + pub snapped_reference_frame_relative_offset: LayoutVector2D, pub shadow: bool, pub raster_space: RasterSpace, } @@ -220,10 +223,12 @@ impl TextRunPrimitive { pub fn update_font_instance( &mut self, specified_font: &FontInstance, - device_pixel_scale: DevicePixelScale, + surface: &SurfaceInfo, + spatial_node_index: SpatialNodeIndex, transform: &LayoutToWorldTransform, subpixel_mode: SubpixelMode, raster_space: RasterSpace, + clip_scroll_tree: &ClipScrollTree, ) -> bool { // If local raster space is specified, include that in the scale // of the glyphs that get rasterized. @@ -235,7 +240,7 @@ impl TextRunPrimitive { let raster_scale = raster_space.local_scale().unwrap_or(1.0).max(0.001); // Get the current font size in device pixels - let mut device_font_size = specified_font.size.scale_by(device_pixel_scale.0 * raster_scale); + let mut device_font_size = specified_font.size.scale_by(surface.device_pixel_scale.0 * raster_scale); // Determine if rasterizing glyphs in local or screen space. let transform_glyphs = if raster_space != RasterSpace::Screen { @@ -266,6 +271,30 @@ impl TextRunPrimitive { // Record the raster space the text needs to be snapped in. self.raster_space = raster_space; + // TODO(aosmond): Snapping really ought to happen during scene building + // as much as possible. This will allow clips to be already adjusted + // based on the snapping requirements of the primitive. This may affect + // complex clips that create a different task, and when we rasterize + // glyphs without the transform (because the shader doesn't have the + // snap offsets to adjust its clip). These rects are fairly conservative + // to begin with and do not appear to be causing significant issues at + // this time. + self.snapped_reference_frame_relative_offset = if !font_transform.is_identity() { + // Don't touch the reference frame relative offset. We'll let the + // shader do the snapping in device pixels. + self.reference_frame_relative_offset + } else { + // There may be an animation, so snap the reference frame relative + // offset such that it excludes the impact, if any. + let snap_to_device = SpaceSnapper::new_with_target( + surface.raster_spatial_node_index, + spatial_node_index, + surface.device_pixel_scale, + clip_scroll_tree, + ); + snap_to_device.snap_vector(&self.reference_frame_relative_offset) + }; + // If the transform or device size is different, then the caller of // this method needs to know to rebuild the glyphs. let cache_dirty = @@ -299,21 +328,23 @@ impl TextRunPrimitive { glyphs: &[GlyphInstance], transform: &LayoutToWorldTransform, surface: &SurfaceInfo, + spatial_node_index: SpatialNodeIndex, raster_space: RasterSpace, subpixel_mode: SubpixelMode, resource_cache: &mut ResourceCache, gpu_cache: &mut GpuCache, render_tasks: &mut RenderTaskGraph, + clip_scroll_tree: &ClipScrollTree, scratch: &mut PrimitiveScratchBuffer, ) { - let device_pixel_scale = surface.device_pixel_scale; - let cache_dirty = self.update_font_instance( specified_font, - device_pixel_scale, + surface, + spatial_node_index, transform, subpixel_mode, raster_space, + clip_scroll_tree, ); if self.glyph_keys_range.is_empty() || cache_dirty { @@ -323,7 +354,7 @@ impl TextRunPrimitive { glyphs.iter().map(|src| { let src_point = src.point + prim_offset; let world_offset = self.used_font.transform.transform(&src_point); - let device_offset = device_pixel_scale.transform_point(world_offset); + let device_offset = surface.device_pixel_scale.transform_point(world_offset); GlyphKey::new(src.index, device_offset, subpx_dir) })); } @@ -351,5 +382,5 @@ fn test_struct_sizes() { assert_eq!(mem::size_of::(), 56, "TextRun size changed"); assert_eq!(mem::size_of::(), 72, "TextRunTemplate size changed"); assert_eq!(mem::size_of::(), 64, "TextRunKey size changed"); - assert_eq!(mem::size_of::(), 72, "TextRunPrimitive size changed"); + assert_eq!(mem::size_of::(), 80, "TextRunPrimitive size changed"); } diff --git a/webrender/src/scene_building.rs b/webrender/src/scene_building.rs index 5bf216f672..e7cc18f46b 100644 --- a/webrender/src/scene_building.rs +++ b/webrender/src/scene_building.rs @@ -1114,6 +1114,12 @@ impl<'a> SceneBuilder<'a> { ); } DisplayItem::Text(ref info) => { + // TODO(aosmond): Snapping text primitives does not make much sense, given the + // primitive bounds and clip are supposed to be conservative, not definitive. + // E.g. they should be able to grow and not impact the output. However there + // are subtle interactions between the primitive origin and the glyph offset + // which appear to be significant (presumably due to some sort of accumulated + // error throughout the layers). We should fix this at some point. let (layout, _, clip_and_scroll) = self.process_common_properties_with_bounds( &info.common, &info.bounds, diff --git a/webrender/src/util.rs b/webrender/src/util.rs index de67abf2cc..3f5bf0a870 100644 --- a/webrender/src/util.rs +++ b/webrender/src/util.rs @@ -246,6 +246,20 @@ impl ScaleOffset { ) } + pub fn map_vector(&self, vector: &Vector2D) -> Vector2D { + Vector2D::new( + vector.x * self.scale.x + self.offset.x, + vector.y * self.scale.y + self.offset.y, + ) + } + + pub fn unmap_vector(&self, vector: &Vector2D) -> Vector2D { + Vector2D::new( + (vector.x - self.offset.x) / self.scale.x, + (vector.y - self.offset.y) / self.scale.y, + ) + } + pub fn to_transform(&self) -> Transform3D { Transform3D::row_major( self.scale.x, diff --git a/wrench/reftests/text/reftest.list b/wrench/reftests/text/reftest.list index 51bf26b662..2597070110 100644 --- a/wrench/reftests/text/reftest.list +++ b/wrench/reftests/text/reftest.list @@ -65,7 +65,7 @@ fuzzy(1,113) platform(linux) == raster-space.yaml raster-space.png skip_on(android) skip_on(mac,>=10.14) != allow-subpixel.yaml allow-subpixel-ref.yaml # Android: we don't enable sub-px aa on this platform. skip_on(android,device) == bg-color.yaml bg-color-ref.yaml # Fails on Pixel2 != large-glyphs.yaml blank.yaml -== snap-text-offset.yaml snap-text-offset-ref.yaml +skip_on(android,device) == snap-text-offset.yaml snap-text-offset-ref.yaml fuzzy(5,4435) == shadow-border.yaml shadow-solid-ref.yaml fuzzy(5,4435) == shadow-image.yaml shadow-solid-ref.yaml options(disable-aa) == snap-clip.yaml snap-clip-ref.yaml From 52cfe18bfc0086846fd274fccfd5b1185a2a4552 Mon Sep 17 00:00:00 2001 From: sotaro Date: Wed, 6 Nov 2019 22:03:11 +0000 Subject: [PATCH 08/21] Bug 1593929 - Make invalidate_rendered_frame triggers force redraw r=nical Invalidate_rendered_frame expects full rendering in next WR rendering. Then when invalidate_rendered_frame is requested, we need to request force redraw. Otherwise, SwapChain might skip present call during partial present. Differential Revision: https://phabricator.services.mozilla.com/D51784 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/8189d25586fb6dd2d8e4f5bafbc43a1e31f4988e --- webrender/src/internal_types.rs | 1 + webrender/src/render_backend.rs | 8 ++++++++ webrender/src/renderer.rs | 3 +++ 3 files changed, 12 insertions(+) diff --git a/webrender/src/internal_types.rs b/webrender/src/internal_types.rs index cb8c55f6b4..6bc7b7006b 100644 --- a/webrender/src/internal_types.rs +++ b/webrender/src/internal_types.rs @@ -535,6 +535,7 @@ pub enum ResultMsg { BackendProfileCounters, ), AppendNotificationRequests(Vec), + ForceRedraw, } #[derive(Clone, Debug)] diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index 35b1e4b2f7..0d6faf3405 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -23,6 +23,7 @@ use api::CaptureBits; #[cfg(feature = "replay")] use api::CapturedDocument; use crate::clip_scroll_tree::SpatialNodeIndex; +use crate::composite::CompositorKind; #[cfg(feature = "debugger")] use crate::debug_server; use crate::frame_builder::{FrameBuilder, FrameBuilderConfig}; @@ -1494,6 +1495,13 @@ impl RenderBackend { // external image with NativeTexture or when platform requested to composite frame. if invalidate_rendered_frame { doc.rendered_frame_is_valid = false; + if let CompositorKind::Draw { max_partial_present_rects } = self.frame_config.compositor_kind { + // When partial present is enabled, we need to force redraw. + if max_partial_present_rects > 0 { + let msg = ResultMsg::ForceRedraw; + self.result_tx.send(msg).unwrap(); + } + } } let mut frame_build_time = None; diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 797f581d11..53f2bcd05d 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2568,6 +2568,9 @@ impl Renderer { } self.notifications.append(&mut notifications); } + ResultMsg::ForceRedraw => { + self.force_redraw = true; + } ResultMsg::RefreshShader(path) => { self.pending_shader_updates.push(path); } From 7258a1471456bc529c6fce62bca50c44546bb33c Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 7 Nov 2019 10:00:23 +0000 Subject: [PATCH 09/21] Bug 1594305 - Only allocate compositor surfaces for tiles that are not occluded r=nical Tiles that are occluded are generally never seen, or only seen occasionally. To reduce the number of compositor surfaces: * Defer native surface allocation until after occlusion culling occurs. * If a tile has a native surface, then becomes occluded, drop the surface. With this scheme, the number of unused native surfaces will always be 0 on a page that doesn't have scrolling. For a page that has a scrollable region, there will be a small number of unused tiles retained. The unused tiles are those that are (a) not occluded (b) not currently visible (c) are in the display port. We retain these for a small amount of time in case they get scrolled back on screen. This makes the allocation patterns for native surfaces match the way that picture cache surfaces are allocated for simple compositing mode. Differential Revision: https://phabricator.services.mozilla.com/D51973 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/70442369cd48e37b05d2982f4109622cf8209bc6 --- webrender/src/composite.rs | 16 ++--- webrender/src/picture.rs | 120 +++++++++++++++++++++++-------------- 2 files changed, 85 insertions(+), 51 deletions(-) diff --git a/webrender/src/composite.rs b/webrender/src/composite.rs index 7f53e27155..ef7bcf4d6a 100644 --- a/webrender/src/composite.rs +++ b/webrender/src/composite.rs @@ -5,14 +5,18 @@ use api::ColorF; use api::units::{DeviceRect, DeviceIntSize, DeviceIntRect, DeviceIntPoint, WorldRect, DevicePixelScale}; use crate::gpu_types::{ZBufferId, ZBufferIdGenerator}; -use crate::picture::{ResolvedSurfaceTexture, SurfaceTextureDescriptor}; +use crate::picture::{ResolvedSurfaceTexture}; use std::{ops, u64}; +use std::sync::atomic::{AtomicU64, Ordering}; /* Types and definitions related to compositing picture cache tiles and/or OS compositor integration. */ +// Counter for generating unique native surface ids +static NEXT_NATIVE_SURFACE_ID: AtomicU64 = AtomicU64::new(0); + /// Describes details of an operation to apply to a native surface #[derive(Debug, Clone)] #[cfg_attr(feature = "capture", derive(Serialize))] @@ -192,10 +196,11 @@ impl CompositeState { /// specified id and dimensions. pub fn create_surface( &mut self, - id: NativeSurfaceId, size: DeviceIntSize, is_opaque: bool, - ) -> SurfaceTextureDescriptor { + ) -> NativeSurfaceId { + let id = NativeSurfaceId(NEXT_NATIVE_SURFACE_ID.fetch_add(1, Ordering::Relaxed)); + self.native_surface_updates.push( NativeSurfaceOperation { id, @@ -206,10 +211,7 @@ impl CompositeState { } ); - SurfaceTextureDescriptor::NativeSurface { - id, - size, - } + id } /// Queue up destruction of an existing native OS surface. This is used when diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index cda8518cec..1d18afbfec 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -440,7 +440,7 @@ pub enum SurfaceTextureDescriptor { /// surface identified by arbitrary id. NativeSurface { /// The arbitrary id of this surface. - id: NativeSurfaceId, + id: Option, /// Size in device pixels of the native surface. size: DeviceIntSize, }, @@ -484,7 +484,7 @@ impl SurfaceTextureDescriptor { } SurfaceTextureDescriptor::NativeSurface { id, size } => { ResolvedSurfaceTexture::NativeSurface { - id: *id, + id: id.expect("bug: native surface not allocated"), size: *size, } } @@ -871,7 +871,7 @@ impl Tile { // the tile was previously a color, or not set, then just set // up a new texture cache handle. match self.surface.take() { - Some(TileSurface::Texture { descriptor, visibility_mask }) => { + Some(TileSurface::Texture { mut descriptor, visibility_mask }) => { // If opacity changed, and this is a native OS compositor surface, // it needs to be recreated. // TODO(gw): This is a limitation of the DirectComposite APIs. It might @@ -879,18 +879,17 @@ impl Tile { // a property on a surface, if we ever see pages where this // is changing frequently. if opacity_changed { - if let SurfaceTextureDescriptor::NativeSurface { id, size } = descriptor { + if let SurfaceTextureDescriptor::NativeSurface { ref mut id, .. } = descriptor { // Reset the dirty rect and tile validity in this case, to // force the new tile to be completely redrawn. self.dirty_rect = self.rect; self.is_valid = false; - state.composite_state.destroy_surface(id); - state.composite_state.create_surface( - id, - size, - self.is_opaque, - ); + // If this tile has a currently allocated native surface, destroy it. It + // will be re-allocated next time it's determined to be visible. + if let Some(id) = id.take() { + state.composite_state.destroy_surface(id); + } } } @@ -914,13 +913,13 @@ impl Tile { } } CompositorKind::Native { .. } => { - // For a new native OS surface, we need to queue up creation - // of a native surface to be passed to the compositor interface. - state.composite_state.create_surface( - NativeSurfaceId(self.id.0 as u64), - ctx.current_tile_size, - self.is_opaque, - ) + // Create a native surface surface descriptor, but don't allocate + // a surface yet. The surface is allocated *after* occlusion + // culling occurs, so that only visible tiles allocate GPU memory. + SurfaceTextureDescriptor::NativeSurface { + id: None, + size: ctx.current_tile_size, + } } }; @@ -3328,11 +3327,23 @@ impl PicturePrimitive { } }; + let surface = tile.surface.as_mut().expect("no tile surface set!"); + // 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 // it will fail this test, and get rasterized by the render task setup // code below. if frame_state.composite_state.is_tile_occluded(tile_cache.slice, tile_draw_rect) { + // If this tile has an allocated native surface, free it, since it's completely + // occluded. We will need to re-allocate this surface if it becomes visible, + // but that's likely to be rare (e.g. when there is no content display list + // for a frame or two during a tab switch). + if let TileSurface::Texture { descriptor: SurfaceTextureDescriptor::NativeSurface { id, .. }, .. } = surface { + if let Some(id) = id.take() { + frame_state.composite_state.destroy_surface(id); + } + } + tile.is_visible = false; continue; } @@ -3347,8 +3358,6 @@ impl PicturePrimitive { frame_state.resource_cache.set_image_active(*image_key); } - let surface = tile.surface.as_mut().expect("no tile surface set!"); - if frame_context.debug_flags.contains(DebugFlags::PICTURE_CACHING_DBG) { tile.root.draw_debug_rects( &map_pic_to_world, @@ -3373,23 +3382,34 @@ impl PicturePrimitive { } } - if let TileSurface::Texture { descriptor: SurfaceTextureDescriptor::TextureCache { ref handle, .. }, .. } = surface { - // Invalidate if the backing texture was evicted. - if frame_state.resource_cache.texture_cache.is_allocated(handle) { - // Request the backing texture so it won't get evicted this frame. - // We specifically want to mark the tile texture as used, even - // if it's detected not visible below and skipped. This is because - // we maintain the set of tiles we care about based on visibility - // during pre_update. If a tile still exists after that, we are - // assuming that it's either visible or we want to retain it for - // a while in case it gets scrolled back onto screen soon. - // TODO(gw): Consider switching to manual eviction policy? - frame_state.resource_cache.texture_cache.request(handle, frame_state.gpu_cache); - } else { - // If the texture was evicted on a previous frame, we need to assume - // that the entire tile rect is dirty. - tile.is_valid = false; - tile.dirty_rect = tile.rect; + if let TileSurface::Texture { descriptor, .. } = surface { + match descriptor { + SurfaceTextureDescriptor::TextureCache { ref handle, .. } => { + // Invalidate if the backing texture was evicted. + if frame_state.resource_cache.texture_cache.is_allocated(handle) { + // Request the backing texture so it won't get evicted this frame. + // We specifically want to mark the tile texture as used, even + // if it's detected not visible below and skipped. This is because + // we maintain the set of tiles we care about based on visibility + // during pre_update. If a tile still exists after that, we are + // assuming that it's either visible or we want to retain it for + // a while in case it gets scrolled back onto screen soon. + // TODO(gw): Consider switching to manual eviction policy? + frame_state.resource_cache.texture_cache.request(handle, frame_state.gpu_cache); + } else { + // If the texture was evicted on a previous frame, we need to assume + // that the entire tile rect is dirty. + tile.is_valid = false; + tile.dirty_rect = tile.rect; + } + } + SurfaceTextureDescriptor::NativeSurface { id, .. } => { + if id.is_none() { + // There is no current surface allocation, so ensure the entire tile is invalidated + tile.is_valid = false; + tile.dirty_rect = tile.rect; + } + } } } @@ -3402,13 +3422,23 @@ impl PicturePrimitive { // Ensure that this texture is allocated. if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = surface { - if let SurfaceTextureDescriptor::TextureCache { ref mut handle } = descriptor { - if !frame_state.resource_cache.texture_cache.is_allocated(handle) { - frame_state.resource_cache.texture_cache.update_picture_cache( - tile_cache.current_tile_size, - handle, - frame_state.gpu_cache, - ); + match descriptor { + SurfaceTextureDescriptor::TextureCache { ref mut handle } => { + if !frame_state.resource_cache.texture_cache.is_allocated(handle) { + frame_state.resource_cache.texture_cache.update_picture_cache( + tile_cache.current_tile_size, + handle, + frame_state.gpu_cache, + ); + } + } + SurfaceTextureDescriptor::NativeSurface { id, size } => { + if id.is_none() { + *id = Some(frame_state.composite_state.create_surface( + *size, + tile.is_opaque, + )); + } } } @@ -4871,7 +4901,9 @@ impl CompositeState { // possible for display port tiles to be created that never // come on screen, and thus never get a native surface allocated. if let Some(TileSurface::Texture { descriptor: SurfaceTextureDescriptor::NativeSurface { id, .. }, .. }) = tile.surface { - self.destroy_surface(id); + if let Some(id) = id { + self.destroy_surface(id); + } } } } From ac56990a2d03ee8353582f05591f30d600efdc51 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 7 Nov 2019 10:00:32 +0000 Subject: [PATCH 10/21] Bug 1594524 - Move the call to gpu_profile.end_frame() up so that we don't measure debug overlays. r=gw Differential Revision: https://phabricator.services.mozilla.com/D52090 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/455d161a3fa92006c59f6411e3e01ccabb3c7074 --- webrender/src/renderer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 53f2bcd05d..67c4c1256c 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -3117,6 +3117,9 @@ impl Renderer { self.unlock_external_images(); self.active_documents = active_documents; + + let _gm = self.gpu_profile.start_marker("end frame"); + self.gpu_profile.end_frame(); }); if let Some(device_size) = device_size { @@ -3228,8 +3231,6 @@ impl Renderer { self.gpu_cache_upload_time = 0; profile_timers.cpu_time.profile(|| { - let _gm = self.gpu_profile.start_marker("end frame"); - self.gpu_profile.end_frame(); if let Some(debug_renderer) = self.debug.try_get_mut() { let small_screen = self.debug_flags.contains(DebugFlags::SMALL_SCREEN); let scale = if small_screen { 1.6 } else { 1.0 }; From 809dd129687476a97757ab22bcc94e6e38adb8fe Mon Sep 17 00:00:00 2001 From: Jamie Nicol Date: Thu, 7 Nov 2019 10:00:41 +0000 Subject: [PATCH 11/21] Bug 1589666 - Disable picture caching whilst pinch-zooming. r=gw Whilst pinch zooming, every picture cache tile gets completely invalidated every frame. It is therefore a waste of memory bandwidth to render in to picture cache tiles then composite those to the screen. This change dynamically disables picture caching for frames in which we are pinch zooming. The exception is if we are using a native compositor, in which case picture caching will remain enabled, because it relies on picture caching to work, and does not waste memory bandwidth. Differential Revision: https://phabricator.services.mozilla.com/D52017 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/dd0e1c0d6acc4b7cb8b8b251aa3da7984f8b496f --- webrender/src/frame_builder.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/webrender/src/frame_builder.rs b/webrender/src/frame_builder.rs index 20a79e3d60..0186362533 100644 --- a/webrender/src/frame_builder.rs +++ b/webrender/src/frame_builder.rs @@ -503,7 +503,7 @@ impl FrameBuilder { // Determine if we will draw this frame with picture caching enabled. This depends on: // (1) If globally enabled when WR was initialized // (2) If current debug flags allow picture caching - // (3) [In future] Whether we are currently pinch zooming + // (3) Whether we are currently pinch zooming // (4) If any picture cache spatial nodes are not in the root coordinate system let picture_caching_is_enabled = scene.config.global_enable_picture_caching && @@ -512,7 +512,8 @@ impl FrameBuilder { let spatial_node = &scene .clip_scroll_tree .spatial_nodes[spatial_node_index.0 as usize]; - spatial_node.coordinate_system_id != CoordinateSystemId::root() + spatial_node.coordinate_system_id != CoordinateSystemId::root() || + spatial_node.is_ancestor_or_self_zooming }); let mut composite_state = CompositeState::new( From 2ab35cf100c3990522b94110eaf7fb87b128e46e Mon Sep 17 00:00:00 2001 From: Dorel Luca Date: Thu, 7 Nov 2019 10:00:50 +0000 Subject: [PATCH 12/21] Backed out changeset 455d161a3fa9 (bug 1594524) for causing Wrench failure in adb wait-for-device shell cat /sys/class/power_supply/battery/input_suspend 2>/dev/null. CLOSED TREE [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/b53c2535ec7da21a0046cc362263b741613c387d --- webrender/src/renderer.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 67c4c1256c..53f2bcd05d 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -3117,9 +3117,6 @@ impl Renderer { self.unlock_external_images(); self.active_documents = active_documents; - - let _gm = self.gpu_profile.start_marker("end frame"); - self.gpu_profile.end_frame(); }); if let Some(device_size) = device_size { @@ -3231,6 +3228,8 @@ impl Renderer { self.gpu_cache_upload_time = 0; profile_timers.cpu_time.profile(|| { + let _gm = self.gpu_profile.start_marker("end frame"); + self.gpu_profile.end_frame(); if let Some(debug_renderer) = self.debug.try_get_mut() { let small_screen = self.debug_flags.contains(DebugFlags::SMALL_SCREEN); let scale = if small_screen { 1.6 } else { 1.0 }; From 4285877f3b117a254db4e69401f6ff1199bf08b9 Mon Sep 17 00:00:00 2001 From: Bert Peers Date: Thu, 7 Nov 2019 10:01:10 +0000 Subject: [PATCH 13/21] Bug 1571972 - Re-export RendererError as a public type r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D51897 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/f2a5d56c886dd7f31aaebfcf6016539aa1d5d42f --- webrender/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrender/src/lib.rs b/webrender/src/lib.rs index 2c0dead46b..f7c27f6837 100644 --- a/webrender/src/lib.rs +++ b/webrender/src/lib.rs @@ -211,7 +211,7 @@ pub use crate::profiler::{ProfilerHooks, set_profiler_hooks}; pub use crate::renderer::{ AsyncPropertySampler, CpuProfile, DebugFlags, OutputImageHandler, RendererKind, ExternalImage, ExternalImageHandler, ExternalImageSource, GpuProfile, GraphicsApi, GraphicsApiInfo, - PipelineInfo, Renderer, RendererOptions, RenderResults, RendererStats, SceneBuilderHooks, + PipelineInfo, Renderer, RendererError, RendererOptions, RenderResults, RendererStats, SceneBuilderHooks, ThreadListener, ShaderPrecacheFlags, MAX_VERTEX_TEXTURE_WIDTH, }; pub use crate::screen_capture::{AsyncScreenshotHandle, RecordedFrameHandle}; From 0163f1ba1e782bbd47707bb13223925e11df1589 Mon Sep 17 00:00:00 2001 From: Pete Moore Date: Thu, 7 Nov 2019 10:01:19 +0000 Subject: [PATCH 14/21] Bug 1575648 - Migrate from taskcluster.net to tools.community-tc.services.mozilla.com r=jrmuizel [import_pr] From https://github.com/servo/webrender/pull/3793 Differential Revision: https://phabricator.services.mozilla.com/D52103 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/1e710184eb57d8decb388aee8efdac082d9b48a1 --- .taskcluster.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.taskcluster.yml b/.taskcluster.yml index ed9c2c6ff0..56601d905e 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -1,9 +1,9 @@ # This file specifies the configuration needed to test WebRender using the -# Taskcluster infrastructure. Most of this should be relatively self-explanatory; -# this file was originally generated by using the Taskcluster-GitHub integration -# quickstart tool at https://tools.taskcluster.net/quickstart/ and then expanded -# as needed. +# Taskcluster infrastructure. Most of this should be relatively +# self-explanatory; this file was originally generated by using the +# Taskcluster-GitHub integration quickstart tool which no longer exists. # +# See https://community-tc.services.mozilla.com/docs version: 1 policy: pullRequests: public @@ -56,8 +56,8 @@ tasks: description: Runs release-mode WebRender CI stuff on a Linux TC worker owner: noreply@mozilla.com source: ${repo_url} - provisionerId: aws-provisioner-v1 - workerType: github-worker + provisionerId: proj-webrender + workerType: ci-linux deadline: {$fromNow: '1 day'} payload: maxRunTime: 7200 @@ -77,14 +77,14 @@ tasks: servo-tidy && ci-scripts/linux-release-tests.sh routes: - - "index.garbage.webrender.ci.${login}.${branch}.linux-release" + - "index.project.webrender.ci.${login}.${branch}.linux-release" - metadata: name: Linux debug tests description: Runs debug-mode WebRender CI stuff on a Linux TC worker owner: noreply@mozilla.com source: ${repo_url} - provisionerId: aws-provisioner-v1 - workerType: github-worker + provisionerId: proj-webrender + workerType: ci-linux deadline: {$fromNow: '1 day'} payload: maxRunTime: 7200 @@ -104,12 +104,12 @@ tasks: servo-tidy && ci-scripts/linux-debug-tests.sh routes: - - "index.garbage.webrender.ci.${login}.${branch}.linux-debug" + - "index.project.webrender.ci.${login}.${branch}.linux-debug" # For the OS X jobs we use a pool of machines that we are managing, because # Mozilla releng doesn't have any spare OS X machines for us at this time. # Talk to :kats or :jrmuizel if you need more details about this. The machines - # are hooked up to taskcluster using taskcluster-worker; they use a worker-type - # of webrender-ci-osx. They are set up with all the dependencies needed + # are hooked up to taskcluster using taskcluster-worker; they use a workerpool + # of webrender/ci-macos. They are set up with all the dependencies needed # to build and test webrender, including Rust (currently 1.30), servo-tidy, # mako, zlib, etc. Note that unlike the docker-worker used for Linux, these # machines WILL persist state from one run to the next, so any cleanup needs @@ -119,8 +119,8 @@ tasks: description: Runs release-mode WebRender CI stuff on a OS X TC worker owner: noreply@mozilla.com source: ${repo_url} - provisionerId: 'localprovisioner' - workerType: 'webrender-ci-osx' + provisionerId: 'proj-webrender' + workerType: 'ci-macos' deadline: {$fromNow: '1 day'} payload: maxRunTime: 3600 @@ -142,14 +142,14 @@ tasks: export MAKE="$HOME/make" ci-scripts/macos-release-tests.sh routes: - - "index.garbage.webrender.ci.${login}.${branch}.osx-release" + - "index.project.webrender.ci.${login}.${branch}.osx-release" - metadata: name: OS X debug tests description: Runs debug-mode WebRender CI stuff on a OS X TC worker owner: noreply@mozilla.com source: ${repo_url} - provisionerId: 'localprovisioner' - workerType: 'webrender-ci-osx' + provisionerId: 'proj-webrender' + workerType: 'ci-macos' deadline: {$fromNow: '1 day'} payload: maxRunTime: 3600 @@ -171,4 +171,4 @@ tasks: export MAKE="$HOME/make" ci-scripts/macos-debug-tests.sh routes: - - "index.garbage.webrender.ci.${login}.${branch}.osx-debug" + - "index.project.webrender.ci.${login}.${branch}.osx-debug" From b998c396e3369ed02590fa56df7d510660d87503 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Thu, 7 Nov 2019 10:01:28 +0000 Subject: [PATCH 15/21] Bug 1594567 - Fix occluder clip rects for off-screen picture caches. r=kvark Differential Revision: https://phabricator.services.mozilla.com/D52108 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/ecccc6021b088d35a1b7ee9c09090295605b3e21 --- webrender/src/picture.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index 1d18afbfec..bd103bd3ac 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -1480,11 +1480,12 @@ impl TileCacheInstance { false, ); - if let Some(clip_chain_instance) = clip_chain_instance { - // TODO(gw): Maybe in future we can early out if the clip rect is None here, - // signalling that the entire picture was clipped out? - self.local_clip_rect = clip_chain_instance.pic_clip_rect; - } + // Ensure that if the entire picture cache is clipped out, the local + // clip rect is zero. This makes sure we don't register any occluders + // that are actually off-screen. + self.local_clip_rect = clip_chain_instance.map_or(PictureRect::zero(), |clip_chain_instance| { + clip_chain_instance.pic_clip_rect + }); } // If there are pending retained state, retrieve it. From 67f4e5337414a492d6b343ed9b6d007bceb69f8c Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Fri, 8 Nov 2019 05:37:17 +0000 Subject: [PATCH 16/21] Bug 1585713 - don't prune WR glyphs that were recently used. r=jnicol Differential Revision: https://phabricator.services.mozilla.com/D51340 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/e517398d7ab9a17f721e5940037fbed56717557d --- webrender/src/glyph_cache.rs | 38 +++++++++++++++++++++++++++------- webrender/src/texture_cache.rs | 8 +++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/webrender/src/glyph_cache.rs b/webrender/src/glyph_cache.rs index 5180e22047..e3629b1d23 100644 --- a/webrender/src/glyph_cache.rs +++ b/webrender/src/glyph_cache.rs @@ -49,7 +49,16 @@ impl GlyphCacheEntry { if let GlyphCacheEntry::Cached(ref glyph) = *self { texture_cache.mark_unused(&glyph.texture_cache_handle); } - }} + } + + fn is_recently_used(&self, texture_cache: &mut TextureCache) -> bool { + if let GlyphCacheEntry::Cached(ref glyph) = *self { + texture_cache.is_recently_used(&glyph.texture_cache_handle, 1) + } else { + false + } + } +} #[allow(dead_code)] #[cfg_attr(feature = "capture", derive(Serialize))] @@ -78,23 +87,31 @@ impl GlyphKeyCache { &self.user_data.eviction_notice } - fn clear_glyphs(&mut self, texture_cache: &mut TextureCache) { + fn is_recently_used(&self, current_frame: FrameId) -> bool { + self.user_data.last_frame_used + 1 >= current_frame + } + + fn clear_glyphs(&mut self, texture_cache: &mut TextureCache) -> usize { + let pruned = self.user_data.bytes_used; for (_, entry) in self.iter() { entry.mark_unused(texture_cache); } self.clear(); self.user_data.bytes_used = 0; + pruned } fn prune_glyphs( &mut self, + skip_recent: bool, excess_bytes_used: usize, texture_cache: &mut TextureCache, render_task_cache: &RenderTaskCache, ) -> usize { let mut pruned = 0; self.retain(|_, entry| { - if pruned <= excess_bytes_used { + if pruned <= excess_bytes_used && + (!skip_recent || !entry.is_recently_used(texture_cache)) { match entry.get_allocated_size(texture_cache, render_task_cache) { Some(size) => { pruned += size; @@ -243,15 +260,20 @@ impl GlyphCache { if self.bytes_used < self.max_bytes_used { break; } + let recent = cache.is_recently_used(self.current_frame); let excess = self.bytes_used - self.max_bytes_used; - if excess >= cache.user_data.bytes_used { + if !recent && excess >= cache.user_data.bytes_used { // If the excess is greater than the cache's size, just clear the whole thing. - cache.clear_glyphs(texture_cache); - self.bytes_used -= cache.user_data.bytes_used; + self.bytes_used -= cache.clear_glyphs(texture_cache); } else { - // Otherwise, just clear as little of the cache as needs to remove the excess + // Otherwise, just clear as little of the cache as needed to remove the excess // and avoid rematerialization costs. - self.bytes_used -= cache.prune_glyphs(excess, texture_cache, render_task_cache); + self.bytes_used -= cache.prune_glyphs( + recent, + excess, + texture_cache, + render_task_cache, + ); } } } diff --git a/webrender/src/texture_cache.rs b/webrender/src/texture_cache.rs index f55fd93c86..0cdda74a07 100644 --- a/webrender/src/texture_cache.rs +++ b/webrender/src/texture_cache.rs @@ -985,6 +985,14 @@ impl TextureCache { self.entries.get_opt(handle).is_some() } + // Check if a given texture handle was last used as recently + // as the specified number of previous frames. + pub fn is_recently_used(&self, handle: &TextureCacheHandle, margin: usize) -> bool { + self.entries.get_opt(handle).map_or(false, |entry| { + entry.last_access.frame_id() + margin >= self.now.frame_id() + }) + } + // Return the allocated size of the texture handle's associated data, // or otherwise indicate the handle is invalid. pub fn get_allocated_size(&self, handle: &TextureCacheHandle) -> Option { From bc865e4d9f026b16693df2b531694a8b70710b00 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Fri, 8 Nov 2019 05:37:26 +0000 Subject: [PATCH 17/21] Bug 1585713 - disable subpixel positioning for oversized WR fonts. r=jnicol Differential Revision: https://phabricator.services.mozilla.com/D51746 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/3479e57e405a9b3b97acc25f76ab4797b0994e84 --- webrender/src/glyph_rasterizer/mod.rs | 17 ------------- webrender/src/platform/macos/font.rs | 17 +++++++------ webrender/src/platform/unix/font.rs | 9 ++++--- webrender/src/platform/windows/font.rs | 11 ++++----- webrender/src/prim_store/text_run.rs | 33 +++++++++++++++++--------- 5 files changed, 39 insertions(+), 48 deletions(-) diff --git a/webrender/src/glyph_rasterizer/mod.rs b/webrender/src/glyph_rasterizer/mod.rs index 393ee81e23..4b0dca4d4e 100644 --- a/webrender/src/glyph_rasterizer/mod.rs +++ b/webrender/src/glyph_rasterizer/mod.rs @@ -534,23 +534,6 @@ impl FontInstance { 0 } } - - pub fn oversized_scale_factor(&self, x_scale: f64, y_scale: f64) -> f64 { - // If the scaled size is over the limit, then it will need to - // be scaled up from the size limit to the scaled size. - // However, this should only occur when the font isn't using any - // features that would tie it to device space, like transforms or - // subpixel AA. - let max_size = self.size.to_f64_px() * x_scale.max(y_scale); - if max_size > FONT_SIZE_LIMIT && - self.transform.is_identity() && - self.render_mode != FontRenderMode::Subpixel - { - max_size / FONT_SIZE_LIMIT - } else { - 1.0 - } - } } #[repr(u32)] diff --git a/webrender/src/platform/macos/font.rs b/webrender/src/platform/macos/font.rs index f91fc84c3a..25b4a64684 100644 --- a/webrender/src/platform/macos/font.rs +++ b/webrender/src/platform/macos/font.rs @@ -491,8 +491,7 @@ impl FontContext { pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult { let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0)); - let scale = font.oversized_scale_factor(x_scale, y_scale); - let size = font.size.scale_by((y_scale / scale) as f32); + let size = font.size.scale_by(y_scale as f32); let ct_font = self.get_ct_font(font.font_key, size, &font.variations).ok_or(GlyphRasterError::LoadFailed)?; let glyph_type = if is_bitmap_font(&ct_font) { GlyphType::Bitmap @@ -538,13 +537,13 @@ impl FontContext { (x_scale, y_scale / x_scale) }; - let extra_strikes = font.get_extra_strikes(strike_scale / scale); + let extra_strikes = font.get_extra_strikes(strike_scale); let metrics = get_glyph_metrics( &ct_font, transform.as_ref(), glyph, - x_offset / scale, - y_offset / scale, + x_offset, + y_offset, extra_strikes as f64 * pixel_step, ); if metrics.rasterized_width == 0 || metrics.rasterized_height == 0 { @@ -634,8 +633,8 @@ impl FontContext { // CG Origin is bottom left, WR is top left. Need -y offset let mut draw_origin = CGPoint { - x: -metrics.rasterized_left as f64 + x_offset / scale, - y: metrics.rasterized_descent as f64 - y_offset / scale, + x: -metrics.rasterized_left as f64 + x_offset, + y: metrics.rasterized_descent as f64 - y_offset, }; if let Some(transform) = transform { @@ -719,8 +718,8 @@ impl FontContext { width: metrics.rasterized_width, height: metrics.rasterized_height, scale: match glyph_type { - GlyphType::Bitmap => (scale / y_scale) as f32, - GlyphType::Vector => scale as f32, + GlyphType::Bitmap => y_scale.recip() as f32, + GlyphType::Vector => 1.0, }, format: match glyph_type { GlyphType::Bitmap => GlyphFormat::ColorBitmap, diff --git a/webrender/src/platform/unix/font.rs b/webrender/src/platform/unix/font.rs index 256eee1203..fad1ac0b3b 100644 --- a/webrender/src/platform/unix/font.rs +++ b/webrender/src/platform/unix/font.rs @@ -420,14 +420,13 @@ impl FontContext { load_flags |= FT_LOAD_IGNORE_GLOBAL_ADVANCE_WIDTH; let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0)); - let scale = font.oversized_scale_factor(x_scale, y_scale); let req_size = font.size.to_f64_px(); let face_flags = unsafe { (*face).face_flags }; let mut result = if (face_flags & (FT_FACE_FLAG_FIXED_SIZES as FT_Long)) != 0 && (face_flags & (FT_FACE_FLAG_SCALABLE as FT_Long)) == 0 && (load_flags & FT_LOAD_NO_BITMAP) == 0 { unsafe { FT_Set_Transform(face, ptr::null_mut(), ptr::null_mut()) }; - self.choose_bitmap_size(face, req_size * y_scale / scale) + self.choose_bitmap_size(face, req_size * y_scale) } else { let mut shape = font.transform.invert_scale(x_scale, y_scale); if font.flags.contains(FontInstanceFlags::FLIP_X) { @@ -452,8 +451,8 @@ impl FontContext { FT_Set_Transform(face, &mut ft_shape, ptr::null_mut()); FT_Set_Char_Size( face, - (req_size * x_scale / scale * 64.0 + 0.5) as FT_F26Dot6, - (req_size * y_scale / scale * 64.0 + 0.5) as FT_F26Dot6, + (req_size * x_scale * 64.0 + 0.5) as FT_F26Dot6, + (req_size * y_scale * 64.0 + 0.5) as FT_F26Dot6, 0, 0, ) @@ -505,7 +504,7 @@ impl FontContext { let y_size = unsafe { (*(*(*slot).face).size).metrics.y_ppem }; Some((slot, req_size as f32 / y_size as f32)) } - FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((slot, scale as f32)), + FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((slot, 1.0)), _ => { error!("Unsupported format"); debug!("format={:?}", format); diff --git a/webrender/src/platform/windows/font.rs b/webrender/src/platform/windows/font.rs index fc2478fba4..ae082cbf67 100644 --- a/webrender/src/platform/windows/font.rs +++ b/webrender/src/platform/windows/font.rs @@ -505,9 +505,8 @@ impl FontContext { } pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult { - let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0)); - let scale = font.oversized_scale_factor(x_scale, y_scale); - let size = (font.size.to_f64_px() * y_scale / scale) as f32; + let (_, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0)); + let size = (font.size.to_f64_px() * y_scale) as f32; let bitmaps = is_bitmap_font(font); let (mut shape, (x_offset, y_offset)) = if bitmaps { (FontTransform::identity(), (0.0, 0.0)) @@ -532,8 +531,8 @@ impl FontContext { m12: shape.skew_y, m21: shape.skew_x, m22: shape.scale_y, - dx: (x_offset / scale) as f32, - dy: (y_offset / scale) as f32, + dx: x_offset as f32, + dy: y_offset as f32, }) } else { None @@ -577,7 +576,7 @@ impl FontContext { top: -bounds.top as f32, width, height, - scale: (if bitmaps { scale / y_scale } else { scale }) as f32, + scale: (if bitmaps { y_scale.recip() } else { 1.0 }) as f32, format, bytes: bgra_pixels, }) diff --git a/webrender/src/prim_store/text_run.rs b/webrender/src/prim_store/text_run.rs index 0a4b69e88c..0b288a3469 100644 --- a/webrender/src/prim_store/text_run.rs +++ b/webrender/src/prim_store/text_run.rs @@ -242,24 +242,26 @@ impl TextRunPrimitive { // Get the current font size in device pixels let mut device_font_size = specified_font.size.scale_by(surface.device_pixel_scale.0 * raster_scale); - // Determine if rasterizing glyphs in local or screen space. - let transform_glyphs = if raster_space != RasterSpace::Screen { - // Ensure the font is supposed to be rasterized in screen-space. - false - } else if transform.has_perspective_component() || !transform.has_2d_inverse() { - // Only support transforms that can be coerced to simple 2D transforms. - false + // Check there is a valid transform that doesn't exceed the font size limit. + // Ensure the font is supposed to be rasterized in screen-space. + // Only support transforms that can be coerced to simple 2D transforms. + let (transform_glyphs, oversized) = if raster_space != RasterSpace::Screen || + transform.has_perspective_component() || !transform.has_2d_inverse() { + (false, device_font_size.to_f64_px() > FONT_SIZE_LIMIT) } else if transform.exceeds_2d_scale(FONT_SIZE_LIMIT / device_font_size.to_f64_px()) { + (false, true) + } else { + (true, false) + }; + + if oversized { // Font sizes larger than the limit need to be scaled, thus can't use subpixels. // In this case we adjust the font size and raster space to ensure // we rasterize at the limit, to minimize the amount of scaling. let max_scale = (FONT_SIZE_LIMIT / device_font_size.to_f64_px()) as f32; raster_space = RasterSpace::Local(max_scale * raster_scale); device_font_size = device_font_size.scale_by(max_scale); - false - } else { - true - }; + } // Get the font transform matrix (skew / scale) from the complete transform. let font_transform = if transform_glyphs { @@ -316,6 +318,15 @@ impl TextRunPrimitive { !transform_glyphs { self.used_font.disable_subpixel_aa(); + + // Disable subpixel positioning for oversized glyphs to avoid + // thrashing the glyph cache with many subpixel variations of + // big glyph textures. A possible subpixel positioning error + // is small relative to the maximum font size and thus should + // not be very noticeable. + if oversized { + self.used_font.disable_subpixel_position(); + } } cache_dirty From 363617d9b6e818f6d19a85cc1234fd8329323e6c Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Fri, 8 Nov 2019 05:37:35 +0000 Subject: [PATCH 18/21] Bug 1585713 - reduce WR font size limit from 512 to 320. r=jnicol Differential Revision: https://phabricator.services.mozilla.com/D51747 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/613cdbb8111686c8be0b9a6e90b2bd92899226f0 --- webrender/src/glyph_rasterizer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webrender/src/glyph_rasterizer/mod.rs b/webrender/src/glyph_rasterizer/mod.rs index 4b0dca4d4e..f7ffff1b5e 100644 --- a/webrender/src/glyph_rasterizer/mod.rs +++ b/webrender/src/glyph_rasterizer/mod.rs @@ -374,7 +374,7 @@ impl<'a> From<&'a LayoutToWorldTransform> for FontTransform { // Some platforms (i.e. Windows) may have trouble rasterizing glyphs above this size. // Ensure glyph sizes are reasonably limited to avoid that scenario. -pub const FONT_SIZE_LIMIT: f64 = 512.0; +pub const FONT_SIZE_LIMIT: f64 = 320.0; /// A mutable font instance description. /// From 3313bb33e5b224cb533d6a79c7a938e8514ecebe Mon Sep 17 00:00:00 2001 From: Jeff Muizelaar Date: Fri, 8 Nov 2019 05:37:44 +0000 Subject: [PATCH 19/21] Bug 1594747. Update bitflags in WebRender to 1.2. r=kvark This will help with some warnings about try! Differential Revision: https://phabricator.services.mozilla.com/D52176 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/cc5ba185000817048946121942490e3409d554c5 --- Cargo.lock | 34 +++++++++++++++++----------------- webrender/Cargo.toml | 2 +- webrender_api/Cargo.toml | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4200bddaa3..53ae1a4aba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18,7 +18,7 @@ name = "andrew" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "line_drawing 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "rusttype 0.7.7 (registry+https://github.com/rust-lang/crates.io-index)", "walkdir 2.2.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -124,7 +124,7 @@ dependencies = [ [[package]] name = "bitflags" -version = "1.0.3" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] @@ -199,7 +199,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "strsim 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "textwrap 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -212,7 +212,7 @@ name = "cloudabi" version = "0.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -228,7 +228,7 @@ name = "cocoa" version = "0.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "block 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "core-graphics 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -272,7 +272,7 @@ name = "core-graphics" version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "foreign-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)", @@ -283,7 +283,7 @@ name = "core-graphics" version = "0.17.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "foreign-types 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)", @@ -561,7 +561,7 @@ name = "fuchsia-zircon" version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "fuchsia-zircon-sys 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -950,7 +950,7 @@ name = "nix" version = "0.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "cc 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1114,7 +1114,7 @@ name = "png" version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "crc32fast 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "deflate 0.7.18 (registry+https://github.com/rust-lang/crates.io-index)", "inflate 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1511,7 +1511,7 @@ version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "andrew 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "dlib 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "memmap 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1744,7 +1744,7 @@ name = "wayland-client" version = "0.21.13" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "downcast-rs 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.58 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.14.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1767,7 +1767,7 @@ name = "wayland-protocols" version = "0.21.13" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "wayland-client 0.21.13 (registry+https://github.com/rust-lang/crates.io-index)", "wayland-commons 0.21.13 (registry+https://github.com/rust-lang/crates.io-index)", "wayland-scanner 0.21.13 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1799,7 +1799,7 @@ version = "0.60.0" dependencies = [ "base64 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)", "bincode 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.7 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1856,7 +1856,7 @@ name = "webrender_api" version = "0.60.0" dependencies = [ "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.7 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "core-graphics 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1932,7 +1932,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "android_glue 0.2.3 (git+https://github.com/rust-windowing/android-rs-glue.git?rev=e3ac6edea5814e1faca0c31ea8fac6877cb929ea)", "backtrace 0.3.12 (registry+https://github.com/rust-lang/crates.io-index)", - "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", + "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "cocoa 0.18.4 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "core-graphics 0.17.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2066,7 +2066,7 @@ dependencies = [ "checksum base64 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "621fc7ecb8008f86d7fb9b95356cd692ce9514b80a86d85b397f32a22da7b9e2" "checksum binary-space-partition 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "88ceb0d16c4fd0e42876e298d7d3ce3780dd9ebdcbe4199816a32c77e08597ff" "checksum bincode 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bda13183df33055cbb84b847becce220d392df502ebe7a4a78d7021771ed94d0" -"checksum bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "d0c54bb8f454c567f21197eefcdbf5679d0bd99f2ddbe52e84c77061952e6789" +"checksum bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" "checksum block 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "0d8c1fef690941d3e7788d328517591fecc684c084084702d6ff1641e993699a" "checksum block-buffer 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "49665c62e0e700857531fa5d3763e91b539ff1abeebd56808d378b495870d60d" "checksum block-padding 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "4fc4358306e344bf9775d0197fd00d2603e5afb0771bb353538630f022068ea3" diff --git a/webrender/Cargo.toml b/webrender/Cargo.toml index 10c8669587..d46ed2d4a1 100644 --- a/webrender/Cargo.toml +++ b/webrender/Cargo.toml @@ -26,7 +26,7 @@ webrender_build = { version = "0.0.1", path = "../webrender_build" } [dependencies] base64 = { optional = true, version = "0.10" } bincode = "1.0" -bitflags = "1.0" +bitflags = "1.2" byteorder = "1.0" cfg-if = "0.1.2" cstr = "0.1.2" diff --git a/webrender_api/Cargo.toml b/webrender_api/Cargo.toml index c9acfbbc41..bce81b3210 100644 --- a/webrender_api/Cargo.toml +++ b/webrender_api/Cargo.toml @@ -16,7 +16,7 @@ display_list_stats = [] [dependencies] app_units = "0.7" -bitflags = "1.0" +bitflags = "1.2" byteorder = "1.2.1" derive_more = "0.13" ipc-channel = {version = "0.12.0", optional = true} From 81159b720a507d5a6f7b78c62ad07bf0b0b16bfd Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 8 Nov 2019 17:42:41 +0000 Subject: [PATCH 20/21] Bug 1594500 - Document the memory layout of brush vertex shaders. r=gw Differential Revision: https://phabricator.services.mozilla.com/D52138 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/b7d3f7bb8f7c4a43392673bfb26e1efcfe438b55 --- webrender/res/brush.glsl | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/webrender/res/brush.glsl b/webrender/res/brush.glsl index b1012b6323..a338afb308 100644 --- a/webrender/res/brush.glsl +++ b/webrender/res/brush.glsl @@ -2,6 +2,51 @@ * 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/. */ +/// # Brush vertex shaders memory layout +/// +/// The overall memory layout is the same for all brush shaders. +/// +/// The vertex shader receives a minimal amount of data from vertex attributes (packed into a single +/// ivec4 per instance) and the rest is fetched from various uniform samplers using offsets decoded +/// from the vertex attributes. +/// +/// The diagram below shows the the various pieces of data fectched in the vertex shader: +/// +///```ascii +/// (sPrimitiveHeadersI) +/// (VBO) +-----------------------+ +/// +----------------------------+ +----------------------------> | Int header | +/// | Instance vertex attributes | | (sPrimitiveHeadersF) | | +/// | | | +---------------------+ | z | +/// | x: prim_header_address +-------+---> | Float header | | specific_address +-----+ +/// | y: picture_task_address +---------+ | | | transform_address +---+ | +/// | clip_address +-----+ | | local_rect | | user_data | | | +/// | z: flags | | | | local_clip_rect | +-----------------------+ | | +/// | segment_index | | | +---------------------+ | | +/// | w: resource_address +--+ | | | | +/// +----------------------------+ | | | (sGpuCache) | | +/// | | | (sGpuCache) +------------+ | | +/// | | | +---------------+ | Transform | <--------+ | +/// (sGpuCache) | | +-> | Picture task | +------------+ | +/// +-------------+ | | | | | +/// | Resource | <---+ | | ... | | +/// | | | +---------------+ +--------------------------------+ +/// | | | | +/// +-------------+ | (sGpuCache) v (sGpuCache) +/// | +---------------+ +--------------+---------------+-+-+ +/// +-----> | Clip area | | Brush data | Segment data | | | +/// | | | | | | | +/// | ... | | ... | ... | | | ... +/// +---------------+ +--------------+---------------+-+-+ +///``` +/// +/// - Segment data address is obtained by combining the address stored in the int header and the +/// segment index decoded from the vertex attributes. +/// - Resource data is optional, some brush types (such as images) store some extra data there while +/// other brush types don't use it. +/// + + #ifdef WR_VERTEX_SHADER void brush_vs( From 7971c991b9367fd16cd74186f0327e001b19e0ee Mon Sep 17 00:00:00 2001 From: Peter Moore Date: Fri, 8 Nov 2019 21:43:57 +0000 Subject: [PATCH 21/21] Bug 1575648 - don't set RUSTFLAGS='--deny warnings' (temporary fix) r=nical This change is a temporary fix to not set RUSTFLAGS='--deny warnings' in order to unblock https://bugzil.la/1575648 (which is time-critical). The longer term solution for this is https://bugzil.la/1564873. Differential Revision: https://phabricator.services.mozilla.com/D52360 [wrupdater] From https://hg.mozilla.org/mozilla-central/rev/c61c05f3707767aaf77f7cffa741c86ef44ace2d --- .taskcluster.yml | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/.taskcluster.yml b/.taskcluster.yml index 56601d905e..3a874006e1 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -64,7 +64,12 @@ tasks: image: 'staktrace/webrender-test:debian-v3' env: RUST_BACKTRACE: 'full' - RUSTFLAGS: '--deny warnings' + ############################################################# + ### Temporarily disable until https://bugzil.la/1564873 lands + ### in order to unblock https://bugzil.la/1575648 (which is + ### time-critical) + # RUSTFLAGS: '--deny warnings' + ############################################################# command: - /bin/bash - '--login' @@ -91,7 +96,12 @@ tasks: image: 'staktrace/webrender-test:debian-v3' env: RUST_BACKTRACE: 'full' - RUSTFLAGS: '--deny warnings' + ############################################################# + ### Temporarily disable until https://bugzil.la/1564873 lands + ### in order to unblock https://bugzil.la/1575648 (which is + ### time-critical) + # RUSTFLAGS: '--deny warnings' + ############################################################# command: - /bin/bash - '--login' @@ -135,7 +145,12 @@ tasks: source $HOME/servotidy-venv/bin/activate servo-tidy export RUST_BACKTRACE=full - export RUSTFLAGS='--deny warnings' + ############################################################# + ### Temporarily disable until https://bugzil.la/1564873 lands + ### in order to unblock https://bugzil.la/1575648 (which is + ### time-critical) + # export RUSTFLAGS='--deny warnings' + ############################################################# export PKG_CONFIG_PATH="/usr/local/opt/zlib/lib/pkgconfig:$PKG_CONFIG_PATH" echo 'exec make -j1 "$@"' > $HOME/make # See #2638 chmod +x $HOME/make @@ -164,7 +179,12 @@ tasks: source $HOME/servotidy-venv/bin/activate servo-tidy export RUST_BACKTRACE=full - export RUSTFLAGS='--deny warnings' + ############################################################# + ### Temporarily disable until https://bugzil.la/1564873 lands + ### in order to unblock https://bugzil.la/1575648 (which is + ### time-critical) + # export RUSTFLAGS='--deny warnings' + ############################################################# export PKG_CONFIG_PATH="/usr/local/opt/zlib/lib/pkgconfig:$PKG_CONFIG_PATH" echo 'exec make -j1 "$@"' > $HOME/make # See #2638 chmod +x $HOME/make