Attachment 357048 Details for Bug 192554 – Patch

[patch] Patch

bug-192554-20181211163442.patch (text/plain), 14.37 KB, created by Karl Leplat on 2018-12-11 07:34:47 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Karl Leplat

Created: 2018-12-11 07:34:47 PST

Size: 14.37 KB

patch

obsolete

>Subversion Revision: 239026
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 202893917e53d9aaa0af273fc573667ad8a8ea56..bd1042186dc4a7526f47201feaaf6bd7e1942308 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,87 @@
>+2018-12-10  Karl Leplat  <karl.leplat_ext@softathome.com>
>+
>+        [GTK][WPE] Rendering artifact on subpixel positions
>+        https://bugs.webkit.org/show_bug.cgi?id=192554
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In compositing mode, rendering artifacts have been detected on the repaint of elements on subpixel position.
>+        We suspect a problem on rounding computing sequences, this bug is reproducible on WebKit GTK and WPE.
>+        This bug is not reproducible with compositing mode disabled.
>+
>+        AncestorGraphicsLayer               1st frame                                    DirtyRect              2nd frame
>+        --------------- 24px           --------------- 24px                                                 --------------
>+        - 24.359375px -                 -     24px   -                                                      - no painted
>+        --------------- 48.359375px    --------------- 48px  (Force compositing mode) ------------- 48px    -------------- 48px
>+        - 24.359375px -                -      25px   -  --> Need display this rect    -    24px   -    -->  -    25px    -
>+        --------------- 72.7187        --------------- 73px                           -------------         -------------- BUG: one pixels line no repainted
>+        - 24.359375px -                -      24px   -                                                      - no painted -
>+        ---------------                ---------------                                                      --------------
>+
>+        Source files for logs:
>+        <WebCore/platform/graphics/LayoutRect.h::snapRectToDevicePixels()>
>+        <WebCore/rendering/RenderLayerBacking.cpp::setContentsNeedDisplayInRect()>
>+        <WebCore/platform/graphics/nicosia/NicosiaPaintingEngineBasic.cpp::paint()>
>+        <WebCore/platform/graphics/cairo/CairoOperations.cpp::fillRectWithColor()>
>+
>+        Dump logs with the previous HTML test:
>+
>+        ***************************** 1st frame processing ************************************
>+        ************** Update tiles in backingStore for GraphicsLayer ******************
>+        setContentsNeedDisplayInRect LayoutRect before: x(48.000000) y(24.000000) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(48.000000) y(24.000000) width(59.000000) height(24.000000)
>+        outlineBoundsForRepaint container before: x(0.000000) y(0.000000) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(0.000000) y(0.000000) w(59.000000) h(24.000000)
>+
>+        setContentsNeedDisplayInRect LayoutRect before: x(48.000000) y(48.359375) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(48.000000) y(48.000000) width(59.000000) height(25.000000)
>+        outlineBoundsForRepaint container before: x(0.000000) y(0.000000) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(0.000000) y(0.000000) w(59.000000) h(24) ??????
>+
>+        setContentsNeedDisplayInRect LayoutRecti before: x(48.000000) y(72.718750) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(48.000000) y(73.000000) width(59.000000) height(24.000000)
>+        outlineBoundsForRepaint container before: x(0.000000) y(0.000000) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result after: x(0.000000) y(0.000000) w(59.000000) h(24.000000)
>+        ********************************************************************************
>+        ************ Paint Web contents ******************************************
>+        ********* paint() --> fillRectWithColor() **************
>+        cairo paint fill (Color::transparent) x(0.000000) y(0.000000) width(59.000000) height(25.000000)
>+        cairo paint fill contents x(0.000000) y(0.000000) width(58.000000) height(24.000000)
>+
>+        ********* paint() --> fillRectWithColor() **************
>+        new targetRect with size : width(59px) height(25px)
>+        cairo paint fill (Color::transparent) x(0.000000) y(0.000000) width(59.000000) height(25.000000)
>+        ------------> TO BE NOTED: this rect that should be updated in the second frame is filled with 25px of height <------------
>+        cairo paint fill contents x(0.000000) y(0.000000) width(58.000000) height(25.000000)
>+
>+        ********* paint() --> fillRectWithColor() **************
>+        cairo paint fill (Color::transparent) x(0.000000) y(0.000000) width(59.000000) height(26.000000)
>+        cairo paint fill contents x(0.000000) y(1.000000) width(58.000000) height(24.000000)
>+        ************************************* end of frame ********************************************
>+
>+        ********************* 2nd frame processing in compositing mode *******************
>+        ************** Update tile in backingStore for GraphicsLayer ******************
>+        setContentsNeedDisplayInRect LayoutRect x(0.000000) y(0.000000) w(59.000000) h(24.359375)
>+        snapRectToDevicePixels result: x(0.000000) y(0.000000) width(59.000000) height(24.000000) ?????
>+
>+        The layout position (0.0 , 0.0) is not the same with the previous frame(48.0 , 48.359375)
>+        Results with the two origins and a layout height of 24.359375px :
>+        for position(0.0 , 0.0) we get 0.0 + 24.359375 = int(24)px of height
>+        for position(48.0 , 48.359375) we get 48.359375 ==> 0.359375 + 24.359375 = 24,71875 = int(25)px of height
>+        We can see that for the same dirtyRect we get differents height.
>+        The calculation is done before moving by subpixels offset, perhaps we should take this for the rounding ?
>+        *******************************************************************************
>+        ************ Paint Web contents ******************************************
>+        ********* paint() --> fillRectWithColor() **************
>+        cairo paint fill (Color::transparent) x(0.000000) y(0.000000) width(59.000000) height(24.000000) ?????
>+        -------> TO BE NOTED: this rect is now filled with only 24px of height why ??
>+        cairo paint fill contents x(0.000000) y(0.000000) width(58.000000) height(25.000000)
>+        ********************* end of frame  ******************************************************
>+
>+        * compositing/repaint-element-on-subpixel-position-expected.png: Added.
>+        * compositing/repaint-element-on-subpixel-position-expected.txt: Added.
>+        * compositing/repaint-element-on-subpixel-position.html: Added.
>+
> 2018-12-09  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239010.
>diff --git a/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.png b/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.png
>new file mode 100644
>index 0000000000000000000000000000000000000000..2da9d848a7351f73bc0fb16170e5e59dc8e1e14e
>GIT binary patch
>literal 6852
>zcmeHMYg7~0zTaRKdwkFyd*lEDx{9FHiV)r*P>Tg49AXQVry?2xDiSr}6(DG7;dl|0
>zAYu$8Dr#y4f++GPAQcECM4&){kcdDqgd`G3(mc82y6ZewdyaRV`{^#$e3)5#X7--h
>z^Ly?8Tsjowzu|WlzXJelKpxot6#%QY0$?1r));E}!$*Ixhju&hUxeXL#K)dEjY~QM
>zoMRj@CtRI8JX~BoTpZC(F;20LdmUZTu4s(&@mM$4+kgI?1KqUdc!2+Yu>5wr)j}@<
>zz+yjg|GtpaQpv=T;A?TMvW4i6<sW>z`D)zd2k1Wq{&@2yLYw--;je>K4Gc%wvEe?}
>zbUfYZ>u_XvjNK)_{0}|~`N&xwFU!W?L7xrzh4FLuY;fjTMbdalit2qqqqOPvInls9
>z01nZ`Ni+c3D1OI`z(<?H0C@i1X7HKg#ns?Y;9r{>qgC(g@+@tbQW&@}GFvJ@sqLz>
>z|8j}AKHys)9;YK;Z<@csF2ABef{RbK_0>xj$~nB|lCDV6DfBHjScgcK)?RbMSIfpq
>zgqnkVnpYbIfP&hB@pjE*P>zYyDElW-R6{XbOpeDS?3l1WFW%R=28Y>Og1!H+@Dipo
>z_dI5=<#^hgdcFMlP=<!P-gLS{)Qq@!PaA$TeW8Zge`=;FQq5q<uRIqhT2=vU4%9W_
>zp<6J0e5$Hy2_Lr<LRznwZHhz=43^nu>m_R2%EM2S$EO%(a(=2^4(YB8y@x$buPljB
>zhNoC#@Ox+Jt2cwQA6S`%Wf9|+)qH7G>BRDj{jkdD&L&h%^K}({-~j0|wdeIi2e|7E
>z+mzvGWvLOM4xKAm8$DFwMtXw#gf)MF7{#$K!JXL-M;#TMGy+{=U6Nxhch%3MB7;bZ
>ze4U6lsLp#Vn(L_xQ0q>$)>9XQ$rNPyLUSwtb5=ubnzNp;4y1gv7_pe|C&b9x@lc0u
>z0cygwe6-YKWU~=4*l!sM_%`29dv0Og+aKk8KH1iW6MKjJT{=_Cf(9J_azjP*2IY2^
>zTY!YV$jy*33Im;^J9a{+c!j1Fv`;6nrkg`mym6v--~HU^UUTR~Xf8jywubsVeU!(V
>z$Zg1g$qM3b)giJJ$t1ReP&;FLvbLs$!YNa(Ln(xe=vX<P(1?pea102yV+ic1pkBYV
>zZUmYl$%Ia&SW-;uLK=BHO+j;M=e+<cWoQ#NBcpqD1WTAqeKH`}^yPbPAL6Pyh<k8b
>zts1kF{+NGPHZAskAH4iyy05g|7`)zMybj#D(Pj*G*i{4YU+?b*7moj{rr~p0d2e!?
>z(&`^A-+t77x3IW2b0en!{XG*wd_tzE<f*%k<gZ`CVBGZxhp^;?crKfhp7tjAY0AJz
>z5qXlnYZYj|{C@^aN9I))#`^~ASe8LH!)sPG5sg9Hmm}xkVORxb4xi2&DyB*=QxB4E
>z8!}RMAqkB|=xQc{c6X>}I5AzP_?&Oa{qYcKsf_&{S@%ToRj;?W>`|$CAZc`xy?)jv
>z&jbF|a*)vlx39Za57PvrS*>+&vpAKHbAO<?WI<ZAn=zw*;7qbDr#orpdZ{#mr}t<X
>zM`A{E1#JvZ>F*~WYUF;T<e=?jE7p~V>gn@UmFQVyyXK0klV@1<MsPL*&)Ya0a5I`+
>zJ7XIz=%)1{%9!qys;Z2pYg~pfPwe4S+(34=;6@5p6UTbWa@FxwFTXXqFg5l4n&I3*
>zvUw1RTQyI{Iy<=)nciQVI@KE9GqUcaEsopp^cNZ8<jmqjwLmt#6uH`dJ=kL7ulRjT
>zc|S4AtBKpcDZkW;<&T5(VDdA85%0{*W0_o<$~T8klZ`=F-#@@Q4`u2?x+L5;%;R&G
>zwXVyK<T`)TOd@Doz)i0b#|5ayv!4$kR8IxOtD?!!$Wu6bn&D&E$0F~-^ebam>?_Tf
>zAApx}Rs_@G9>Qi>EY3c~Mkwbr!>JefO5M;n(usLPR#F6g6J?@n%9w-K5QRxBW(|r&
>zePs$~t%`R+(Hu)lQpIQ&DwY?SN_7bU+EY=Dcu^9q<EWdr_q1^(8V5&3h-9pNOz*XT
>z!D`s3ZD3!-zuq_;<4kHTVrXr_g%k4|8cZOl<JSH=g#QW;AkfdN|5u=Y)>7dOy^tmd
>zBggx#*{Dkd7pA_SO0Fi>7foIabcH2(h9=JR5$tZ7TOL$h;t{SsYA-cs&rl#mze8TQ
>z4IOtTTYDukGtRlt46u$>>a(8MD;N>+`pD7dc`VxjbBDYRX6{kU<jl@V1d2_N-Z%aS
>zqCDG&&e(ypqma#6#p7w%JBBi2fc>FhU4;+x@p1+>Wlx_pEB+37*kNkIKKy8}Hu$71
>z(hn88G*p7{QQ$M~h()gea44VJ^I_``4p`fhvmvC>LS6k$)SlU~gUQq_$qQYP;Sn4g
>zJJTiz_Z9Dq`-VhDEOFbzD{?~!-^u}g2E254|KXxy5|7C#ntr@1U1*ul=1gacprNqW
>z<%-WOSUg<Srv}SSzU2C=m1R1;R(HmGIc*!m`Xp#`ObT)M(A7fH_587R3!=Y!v@pZD
>z<Pv7H)Jm5Hq0L6nCy%(kJVVI=fQt)RB@WHbM9Mi|X+^qMgTDFbJL;ZwiSyI_!a@3x
>zeA`jPk|YY^z`0-Jz}y0+is6+>5EBWFFwfN2!S8{B{EaOpr(+R$s+xwPCR3am!?)so
>zhD|rjMr%FZg#kl@Wv6#RDiPU|_M{MUx0wtAd<ka^JinY8xEH|CZc#QaENnB2-%c4%
>z8#tK7ZI=ZKcr0a*hA^PdnO`tEyT4RiP3zs#ZCMW+JfI1(PA=ppHXR0SMx!*NcUAwt
>zal=ZbQWEPaxt3ZR3(G)@@bK~37xM>lG~-kLl!i2iD|G`y-JfAGIfH`5Z(_T0l<eKS
>z-nsGw*?k$>4i$%f*(O^9+zUJtO~koybL(z2ESQd)IV`P+yF-3-8&B)5G%>eiliQ<O
>z$yI(3OKf_`?Fk99?NiELv(QD656tVTDSc5He_a}vCVAd#=+qszyV<a}s4!SPo>9d&
>z9Pu#%FTWSpTUFE%U`dS#>D)n7iXUn_>rhr}Q?V^#0_pU#fQlM86??i4VIXXi<;NZ0
>zfz$*Ep9K=f2oE5^kp_0=lF&6|a$!_MnaXZXuoJwwKUd$TS%ho3s^r;M^M+izn<w>7
>z{7lkJZL?3Y9|^q_;uoDcx)_Y=vTvmB*$Cb^{gw@|V$8Cdi|-BTV&v6g0J;voYjmNC
>zl@UU-8qE$FYGz`gW^fHC$S$`Cvk8jBxFfxrYT{b^yLo}$H~JvQ!R(B7xLe3Z%#OWc
>zIyzpvv~F~aC17#|!bO^ye6DZ}*b@93roeAsE}=?w3zFT8Sao#k$+h5(m%mFj(dFk7
>zTfy#HTAzbA;->?4HyO<@JA<sxcxR3c9Ux8k3i?*RH5%_~;!`{m5f(Lp!)hfaWNTvW
>zJ!`B`(=$k~lhWz%^6XsiFTl&6U%yDNK0N$5)qw(U&_ZccII*yP>FI2ip_aCorzUzJ
>zBPtVQ7#v;yEVxB24kde)QKh_a)Z;AfhA?iVG8{3{@DvJq%|jG_ONbfKee^nCAA4|X
>zlgpkXjZ2N(7>1>d&H0OmYZ}VzE0<JTbbU*lvIN;(hW!M#+u8>B|M)*n(R>jOdmD^m
>z%tVr>r=eH4syF{PG3akZ_-j~#Oad!gF^LtESTTveUr2;?>Gw_9NQIM~hTP6IMSh2e
>zl^)J;D3L3F>Bl7Ud#5V#E>15f<jEyf^RdxKw=;!x$+*nzGL-epKS9D60STl2tbNBS
>zasrg-t9U(Sia=+W>ow`72F2DB*&eVPy>5d;-TPrvzR+>L#4D9YRKGgqt&htwkx%0J
>z)|>(TZbnP$s35*pK2fO~2~~H!l4<x*toMDZxkHo(gOYQhp^#A%K0)Sa7AGogMWP^5
>zyFr;jI=m~gNwP4qE)DCo$rm}0;$()A4-N->s;z*V!aQ#*4npL2|MR=VBrD565?t(%
>z-UnriIcQnWnCrDfj(HTLlWl#fghjoOkd@gpWKW==S^>qxqXW9*RiE0kG=2uWRHS=8
>zicgvuzn4LDiH_o`AFw%h`&0Q&04NGh9m{ktBTw3@=X<DXzmIV+^)XMVL(093maBeF
>zs?yVW04#4RnA`r$uUucBnoKLVxA1_It_Yp_+0R-8`e`VaN}%NgV~v(HO}VEWRL8Zd
>zM&2s><TdD7q}?XN-0RXhle~xN(lbo`*1-<O%w_1FhrY7*h;Z(!mIUdvv;%VKlrGqd
>zaVnDlM+(gr_2)@XB1ZWfVUYGaI$?2y;gseHiS=VDW|?YPO|6^9@`zW*PeVyknP|`w
>zG=%fSPmV~hnSi4~zZzyx`#%K!?Efs*52rEXf5KL;1_kc$->Am?N`Ypj>0eZ@Rwid<
>wa{jrZxq^umOsrtyx9a^X?D+q|4uemjYtuLG;d>O&A0~kOB4~fr=f}SJZ^jo!Z2$lO
>
>literal 0
>HcmV?d00001
>
>diff --git a/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.txt b/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..37166199987504fb7ac9edd66d66ae336aa3964b
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.txt
>@@ -0,0 +1,32 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x600
>+  RenderBlock {HTML} at (0,0) size 800x600
>+    RenderBody {BODY} at (8,8) size 784x576
>+      RenderBlock {UL} at (0,0) size 640x147
>+layer at (0,0) size 800x600
>+  RenderBlock (positioned) {DIV} at (0,0) size 800x600
>+layer at (48,8) size 600x24
>+  RenderListItem {LI} at (40,0) size 600x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 1"
>+layer at (48,32) size 600x25
>+  RenderListItem {LI} at (40,24) size 600x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 2"
>+layer at (48,57) size 600x24
>+  RenderListItem {LI} at (40,48) size 600x26 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,81) size 600x24
>+  RenderListItem {LI} at (40,73) size 600x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,105) size 600x25
>+  RenderListItem {LI} at (40,97) size 600x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,130) size 600x24
>+  RenderListItem {LI} at (40,121) size 600x26 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 1"
>diff --git a/LayoutTests/compositing/repaint-element-on-subpixel-position.html b/LayoutTests/compositing/repaint-element-on-subpixel-position.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9e4481ccf5e03200863c7884f4839345a0602cf8
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint-element-on-subpixel-position.html
>@@ -0,0 +1,41 @@
>+<html>
>+<head>
>+<style>
>+ul {
>+    width: 600px;
>+    list-style: none;
>+}
>+
>+li {
>+    position: relative;
>+    height: 24.36px;
>+    background-color: white;
>+}
>+
>+li.selected {
>+    background-color: orange;
>+}
>+</style>
>+<script src="../fast/repaint/resources/repaint.js"></script>
>+<script>
>+    function repaintTest() {
>+        document.getElementById("opt2").className = "selected";
>+        document.getElementById("opt3").className = "selected";
>+        document.getElementById("opt4").className = "selected";
>+        document.getElementById("opt5").className = "selected";
>+    }
>+</script>
>+</head>
>+
>+<body onload="runRepaintTest()">
>+<div id="forceAcceleratedCompositing" style="position:absolute;left:0;right:0;top:0;bottom:0;-webkit-transform: translate3d(0, 0, 0);"></div>
>+<ul>
>+  <li class="selected">TEST 1</li>
>+  <li class="" id="opt2">TEST 2</li>
>+  <li class="" id="opt3">TEST 3</li>
>+  <li class="" id="opt4">TEST 3</li>
>+  <li class="" id="opt5">TEST 3</li>
>+  <li class="selected">TEST 1</li>
>+</ul>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192554: 356955 | 356956 | 356957 | 356958 | 356959 | 357048 | 357050 | 357051 | 357056 | 357057 | 357121 | 357124 | 357127 | 357132 | 357134 | 357138 | 357180 | 357566 | 358887 | 358888 | 358914 | 358918 | 358947 | 359367 | 359370 | 359371 | 359372 | 359373 | 359377 | 359378 | 359456 | 359458 | 359488 | 359552 | 363081