From dd19cddebe6a3352dcd2d6e5cf91ac40fefcfe91 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Wed, 12 Sep 2018 13:05:01 -0400 Subject: [PATCH 1/2] clip text against the local clip rect after snapping rather than before --- webrender/res/brush_image.glsl | 2 +- webrender/res/clip_shared.glsl | 3 +- webrender/res/prim_shared.glsl | 3 +- webrender/res/ps_text_run.glsl | 108 +++++++++++---------- webrender/res/snap.glsl | 11 +-- wrench/reftests/text/clipped-transform.png | Bin 1790 -> 1788 bytes 6 files changed, 63 insertions(+), 64 deletions(-) diff --git a/webrender/res/brush_image.glsl b/webrender/res/brush_image.glsl index 1ad36a238c..6fd0dc074a 100644 --- a/webrender/res/brush_image.glsl +++ b/webrender/res/brush_image.glsl @@ -48,7 +48,7 @@ vec2 transform_point_snapped( RectWithSize local_rect, mat4 transform ) { - vec2 snap_offset = compute_snap_offset(local_pos, transform, local_rect, vec2(0.5)); + vec2 snap_offset = compute_snap_offset(local_pos, transform, local_rect); vec4 world_pos = transform * vec4(local_pos, 0.0, 1.0); vec2 device_pos = world_pos.xy / world_pos.w * uDevicePixelRatio; diff --git a/webrender/res/clip_shared.glsl b/webrender/res/clip_shared.glsl index 5580b34d93..fbce194af6 100644 --- a/webrender/res/clip_shared.glsl +++ b/webrender/res/clip_shared.glsl @@ -74,8 +74,7 @@ ClipVertexInfo write_clip_tile_vertex(RectWithSize local_clip_rect, clip_transform.m, local_clip_rect, RectWithSize(snap_positions.xy, snap_positions.zw - snap_positions.xy), - snap_positions, - vec2(0.5) + snap_positions ); device_pos -= snap_offsets; diff --git a/webrender/res/prim_shared.glsl b/webrender/res/prim_shared.glsl index f0f6a4b92f..42ce495d35 100644 --- a/webrender/res/prim_shared.glsl +++ b/webrender/res/prim_shared.glsl @@ -114,8 +114,7 @@ VertexInfo write_vertex(RectWithSize instance_rect, vec2 snap_offset = compute_snap_offset( clamped_local_pos, transform.m, - snap_rect, - vec2(0.5) + snap_rect ); // Transform the current vertex to world space. diff --git a/webrender/res/ps_text_run.glsl b/webrender/res/ps_text_run.glsl index cb74ec1315..c523a91021 100644 --- a/webrender/res/ps_text_run.glsl +++ b/webrender/res/ps_text_run.glsl @@ -61,64 +61,86 @@ TextRun fetch_text_run(int address) { return TextRun(data[0], data[1], data[2].xy); } -VertexInfo write_text_vertex(vec2 clamped_local_pos, - RectWithSize local_clip_rect, +VertexInfo write_text_vertex(RectWithSize local_clip_rect, float z, Transform transform, PictureTask task, vec2 text_offset, - RectWithSize snap_rect, + vec2 glyph_offset, + RectWithSize glyph_rect, vec2 snap_bias) { - // Transform the current vertex to world space. - vec4 world_pos = transform.m * vec4(clamped_local_pos, 0.0, 1.0); - - // Convert the world positions to device pixel space. - float device_scale = uDevicePixelRatio / world_pos.w; - vec2 device_pos = world_pos.xy * device_scale; + // The offset to snap the glyph rect to a device pixel vec2 snap_offset = vec2(0.0); + mat2 local_transform; -#if defined(WR_FEATURE_GLYPH_TRANSFORM) +#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) { + // Transform from local space to device space. + float device_scale = uDevicePixelRatio / transform.m[3].w; + mat2 device_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 world_text_offset = mat2(transform.m) * text_offset; - vec2 device_text_pos = (transform.m[3].xy + world_text_offset) * device_scale; - snap_offset += floor(device_text_pos + 0.5) - device_text_pos; + vec2 device_text_pos = device_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 = device_transform * glyph_offset; + snap_offset += floor(device_glyph_offset + snap_bias) - device_glyph_offset; + + // Transform from device space back to local space. + local_transform = inverse(device_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 = local_transform * snap_offset; +#endif + } + + // Actually translate the glyph rect to a device pixel using the snap offset. + glyph_rect.p0 += snap_offset; #ifdef WR_FEATURE_GLYPH_TRANSFORM - // For transformed subpixels, we just need to align the glyph origin to a device pixel. - // The transformed text offset has already been snapped, so remove it from the glyph - // origin when snapping the glyph. - vec2 rough_offset = snap_rect.p0 - world_text_offset * device_scale; - snap_offset += floor(rough_offset + snap_bias) - rough_offset; + // The glyph rect is in device space, so transform it back to local space. + RectWithSize local_rect = transform_rect(glyph_rect, local_transform); + + // Select the corner of the glyph's local space rect that we are processing. + vec2 local_pos = local_rect.p0 + local_rect.size * aPosition.xy; + + // If the glyph's local rect would fit inside the local clip rect, then select a corner from + // the device space glyph rect to reduce overdraw of clipped pixels in the fragment shader. + // Otherwise, fall back to clamping the glyph's local rect to the local clip rect. + if (rect_inside_rect(local_rect, local_clip_rect)) { + local_pos = local_transform * (glyph_rect.p0 + glyph_rect.size * aPosition.xy); + } #else - // The transformed text offset has already been snapped, so remove it from the transform - // when snapping the glyph. - mat4 snap_transform = transform.m; - snap_transform[3].xy = -world_text_offset; - snap_offset += compute_snap_offset( - clamped_local_pos, - snap_transform, - snap_rect, - snap_bias - ); + // Select the corner of the glyph rect that we are processing. + vec2 local_pos = glyph_rect.p0 + glyph_rect.size * aPosition.xy; #endif - } + + // Clamp to the local clip rect. + local_pos = clamp_rect(local_pos, 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 / world_pos.w * uDevicePixelRatio; // Apply offsets for the render task to get correct screen location. - vec2 final_pos = device_pos + snap_offset - + vec2 final_pos = device_pos - task.content_origin + task.common_data.task_rect.p0; gl_Position = uTransform * vec4(final_pos, z, 1.0); VertexInfo vi = VertexInfo( - clamped_local_pos, + local_pos, snap_offset, world_pos ); @@ -156,19 +178,6 @@ void main(void) { RectWithSize glyph_rect = RectWithSize(res.offset + glyph_transform * (text.offset + glyph.offset), res.uv_rect.zw - res.uv_rect.xy); - // Transform the glyph rect back to local space. - mat2 inv = inverse(glyph_transform); - RectWithSize local_rect = transform_rect(glyph_rect, inv); - - // Select the corner of the glyph's local space rect that we are processing. - vec2 local_pos = local_rect.p0 + local_rect.size * aPosition.xy; - - // If the glyph's local rect would fit inside the local clip rect, then select a corner from - // the device space glyph rect to reduce overdraw of clipped pixels in the fragment shader. - // Otherwise, fall back to clamping the glyph's local rect to the local clip rect. - local_pos = rect_inside_rect(local_rect, ph.local_clip_rect) ? - inv * (glyph_rect.p0 + glyph_rect.size * aPosition.xy) : - clamp_rect(local_pos, ph.local_clip_rect); #else // Scale from glyph space to local space. float scale = res.scale / uDevicePixelRatio; @@ -176,12 +185,6 @@ void main(void) { // 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)); - - // Select the corner of the glyph rect that we are processing. - vec2 local_pos = glyph_rect.p0 + glyph_rect.size * aPosition.xy; - - // Clamp to the local clip rect. - local_pos = clamp_rect(local_pos, ph.local_clip_rect); #endif vec2 snap_bias; @@ -209,14 +212,15 @@ void main(void) { break; } - VertexInfo vi = write_text_vertex(local_pos, - ph.local_clip_rect, + VertexInfo vi = write_text_vertex(ph.local_clip_rect, ph.z, 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/res/snap.glsl b/webrender/res/snap.glsl index 9b8214fd01..6090c99511 100644 --- a/webrender/res/snap.glsl +++ b/webrender/res/snap.glsl @@ -27,11 +27,10 @@ vec2 compute_snap_offset_impl( mat4 transform, RectWithSize snap_rect, RectWithSize reference_rect, - vec4 snap_positions, - vec2 snap_bias) { + vec4 snap_positions) { /// World offsets applied to the corners of the snap rectangle. - vec4 snap_offsets = floor(snap_positions + snap_bias.xyxy) - snap_positions; + vec4 snap_offsets = floor(snap_positions + 0.5) - snap_positions; /// Compute the position of this vertex inside the snap rectangle. vec2 normalized_snap_pos = (reference_pos - reference_rect.p0) / reference_rect.size; @@ -44,8 +43,7 @@ vec2 compute_snap_offset_impl( // given local position on the transform and a snap rectangle. vec2 compute_snap_offset(vec2 local_pos, mat4 transform, - RectWithSize snap_rect, - vec2 snap_bias) { + RectWithSize snap_rect) { vec4 snap_positions = compute_snap_positions( transform, snap_rect @@ -56,8 +54,7 @@ vec2 compute_snap_offset(vec2 local_pos, transform, snap_rect, snap_rect, - snap_positions, - snap_bias + snap_positions ); return snap_offsets; diff --git a/wrench/reftests/text/clipped-transform.png b/wrench/reftests/text/clipped-transform.png index 9dc5cc4849f0c76a8a6554e6124f95e90fb574a5..062f238e6faed38afc8137054792f38777c4f35e 100644 GIT binary patch literal 1788 zcmb7_`y&&G1Hjc+-lDl=M#7Y0p7SV{Fl_Uf$i*2(M;;M}$zy1^%&R{#SAjV+IT`xCz3A3mSYpYS0%JK0G~C`t$k2}vUnaF_43{En@d$am*+ z_{Ty*B2*;Y^3EgjW^t^u+uHeeNe8Nz^iW2r0dQ@dS`n=<1x#{{{^f=ushavT6;Ttx z!|uMA293-C$pGuZItq&m9oUhls9pR6^H5NZ!Bkh_NuR?d| zTlCq+td?zeFTxHI4nS{g9}6N2ge{S>|EE5I3Y}wGvi2_ML~wI}`PTO@a(DWv44@g` zGk#YZQL}txF^NDS)mM&Fejs<)MpI!Lfy(S@yig@gR9_uR2BPsYS2`Zpf4psFROLae zbLRlMa!9U+dar_WqCh~(P<9AWX+NFOS?L9adXKHl)QHJzsH_E~U;;Ht@Fp|}dU_B2;zn{!+XhjeaOo$27_6QR||@ zuO^8mGx=u$ZNGlY1db#H+;}bT=D8Q79LJez^B;9!tKq9uRy2yxO|b-(KIUCYQrtV1 zF^MJf+^gj|0*6^@LS} zg7@12=e8GdryQZ-TU@%-?|U)Vv^!cy9TIxdvk)>4fOU$qc=p``S>sDZECc)B2=r_s zvEvUxcg!o5$MUt;`db;jx2O@*!(`JPP~65F?DNz(r%w%6@WjsYb zF!$@Y2g_^ydM~Z8LQAqH9}$^Q!uWIXe#4XAGgElzD zLZD=lFI(Bssb1Jr54Q)r$*bz^z&=a0y_QE{-!w}>*&8#`=$>O79Ix^P)m;(g<2$l) zTd5J%c%S!8E-uQs-q|33K>joe3|plQ3s(W;`S z0U3O~p^f=*@z$RiT)!id!Q3FY)?Tb@coEF1U`luna0@Q%9PM5V^L$Clk(|R9n^||h6qCO(JaNhW8aVu zMw&UPcO=MItb-pvhI;cQrw$ov+?Q^raInh3o5i=1Q-p8mAqY}5*4nO={XpE(#u8(4 z!D{)5A}3?o0dgk9Rxe>jKUg~iYxlb5K?dOT_#8yn&(}C!d$2J=Py7-pTUYM zG@_P<)j5H!9J9K^sd+vUF-o>aO-iIk8$J|0@#(@w)g96P1Q&I@>On2~heJs@F{q#E zkE`b_!lat6x7BrMm|gr7Z`vCz;UzVjlAV9)UoR#wD`wUc(_-FSVn|(yIeO@rv^~)#p}&@(>mxDA{01j|~gLdApZzA~;5I@UNI=4~x$1wUZJ6 zj!|sI%E`au0gxyu6P?PJ$u)3Y^jz*y(6X$#(VX#aBE&QpA6?M3kGM4KDi|7?*s8zQ z*`!v;HT1#qMg5CPU68G#Qdx;lo_>I(g_PF4&H4mjrBg4whM|`hdN6Ozm^PCygcdH& z+JsVZfV%0(tA`qi^1v}>izAg&G2zg@Urt|#fQl`Kyo_z;eea1oLYX?3@hid0##MO& z|9=Fein$eIEa2SjOm~GFYSf%z(n?_MBs?_d}M4e4it{>h{@9%fd$LIU=dzd^IJejgUAHv806-l5MMWIj|;4UC=j`b;$ zhomVjT!4xP!?|JlQzC-@vE;3+VIzf&`)ZT9$Y6L`${l-Azgq|@lHZT3j@9ygvTQNC z6Mu4+nNf|8aZ0N4Hut19OL}oo>E2Q%A?JkQw@V%VzRO&SJBe8~sJj|V9+$h?#%fR? z32cH`opCQL_`^7HFoUuG*?*RM=@vdMw%x4Bz8J;|{cB=8v*yDoyX;iZ z?(+4rGSAGT-7Be9F6B*sy#)!GYGJoW2ZBkK8Oi8#!Fyc~V2e4}TpNohD>VuL>FgSj z$(y=2@Fw+|v#HU0bdM-HQO~1!*9XFx@5k2epXTMZbqk|0gtsrf8}7MNFLcoq#+@Rp9TAopVCotD!C7g~YT^q)uP+|BL;UI{TTWoRq&s@f4dUT8wb8?#|txb z?~E!WAts06p=|r)MQ<-u)5z9mtGhkfp-BhF<3KZq3@IuIrwd zA^P|Rlsk9hx3#9Rpb&LCli@D`zj_bqd{rlY`tykO(L2Bw+CBWbQ^&9?@Hph7W7Ufm zKDY5jOPl_El`YKAR{av%Byds+UzX#~b_f zcj%ysd~mXSt=Y#ZyN2kyJ@(?|BW>)whZ;0wwouwa;-neQlCC!2cHTEQ<9UChH?tQF z@WP@j&8OMzz7jy*0ehfRg9b7VT$VwZrMlYXLoS_YaDWMZrffg?TVOGt zy%0y+X_7inmsK>%I@Uv7qECi>NMpIa7)7uBb#UM^@F2MT`q!U2i2>f$AtD_M;^qN! zrZ}*GZA-8`DR`dnJ^(0*upx9L1$L4*X{)JE3~v&N7r7JrZ(=S5r_JE)jEC(jhgB3pacAz9qN)1vCUM=nB zu7xMUV?S{BagJUHrrd#f81$-k1STC{s?nuauWiAQr&jXw-VD9 Date: Thu, 13 Sep 2018 01:49:03 -0400 Subject: [PATCH 2/2] add reftest to verify that glyphs are snapped before clipping --- wrench/reftests/text/reftest.list | 1 + wrench/reftests/text/snap-clip-ref.yaml | 7 +++++++ wrench/reftests/text/snap-clip.yaml | 8 ++++++++ 3 files changed, 16 insertions(+) create mode 100644 wrench/reftests/text/snap-clip-ref.yaml create mode 100644 wrench/reftests/text/snap-clip.yaml diff --git a/wrench/reftests/text/reftest.list b/wrench/reftests/text/reftest.list index a730275107..3f81003616 100644 --- a/wrench/reftests/text/reftest.list +++ b/wrench/reftests/text/reftest.list @@ -65,3 +65,4 @@ fuzzy(1,71) platform(linux) == raster-space.yaml raster-space.png == snap-text-offset.yaml snap-text-offset-ref.yaml == shadow-border.yaml shadow-solid-ref.yaml == shadow-image.yaml shadow-solid-ref.yaml +options(disable-aa) == snap-clip.yaml snap-clip-ref.yaml diff --git a/wrench/reftests/text/snap-clip-ref.yaml b/wrench/reftests/text/snap-clip-ref.yaml new file mode 100644 index 0000000000..676b7c80c5 --- /dev/null +++ b/wrench/reftests/text/snap-clip-ref.yaml @@ -0,0 +1,7 @@ +root: + items: + - bounds: [0, 0, 35, 35] + glyphs: [50] + offsets: [10, 30] + size: 20 + font: "FreeSans.ttf" diff --git a/wrench/reftests/text/snap-clip.yaml b/wrench/reftests/text/snap-clip.yaml new file mode 100644 index 0000000000..6ee30aa09b --- /dev/null +++ b/wrench/reftests/text/snap-clip.yaml @@ -0,0 +1,8 @@ +root: + items: + - bounds: [0, 0, 35, 35] + glyphs: [50] + offsets: [10.3, 30] + clip-rect: [0, 0, 29.7, 35] + size: 20 + font: "FreeSans.ttf"