Attachment 356955 Details for Bug 192554 – Patch

[patch] Patch

bug-192554-20181210142449.patch (text/plain), 13.79 KB, created by Karl Leplat on 2018-12-10 05:24:54 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Karl Leplat

Created: 2018-12-10 05:24:54 PST

Size: 13.79 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..360c84412cac655e5a7a6a23f5ced687de291b52
>GIT binary patch
>literal 6893
>zcmeI1X;f3!7RL`_!RG+AS}C&?l(rzqJSVnlAc9bbRs<v{5N3i*fe?twv%D5@02u;=
>z6hSDJNk%aw0WC;?Kncp2FbDwyB#{I{!VntYdj0TL`(Ewpd*4|1!(I2>b=Eoe?*IPn
>zz0dykOBaV-+ts!M0CvG0kDmcRem?+P+_r6zjeNNGLp|B;)2I_}Q9c2FKEaXEp}^#b
>zskODWv9*u6pRtv(kB_CX)saiSCRWC#m(6_rERQ_Oxe_3oq;T2E;W*fMdwowXNe4hJ
>z6n^~kbFq0`*2S~;{3-kegiq(O_us$c?|g>lZoE6_<^0g|he2Plqll|&LM0^E8n3}U
>zSNvJ<nSEWly+7=}e*L@aFfBjrBE6FeuzMRn!63xhBvK`5^;OU<N;C_7Y_cOmUk;2D
>zb8>TSS5j=H(Q^P;5w`9FM~$z&2flRnmIHhDxB<}me;F#16sx>Z4oeUfp9kR0ZRBm~
>z8QWhMdsI(4E`GLdH)<t*;fP?gcMQvw6elyr`fL)b@gvHDD1FV;#TdV!BlZL%d`!{C
>z)rE2v)05^fS-J2rxO%pn8&j3|IICs!rn2uVYz2!(Ub?x<w+Ko%Z4`c@lhF8c*29Lr
>z-uA(xu+_){BlVHE#o8rN>r1S(VW((1oz`TV_aOO#SG=T_THTPv=S@+m-3S$2Uyl-K
>z(i{#`oFh~?=b<#wQAIWQiAXqOES=BOh5MVrS1|6AaFM9GZp0?vqdmD$b<Q=7SJP;h
>zL9D-J#!cbbcZVYkPi;2<<I%Mjz1OAdNzX|+cmhQHV(_R)n%J~z&>1WVIFPDKd(*g+
>zx>Bo?0FOYe{}8MIVo#4;Jb?WQ*O*b1aTZ>jESL<E#+qAxT`ioaAKn41THkK#AJZz4
>zKGyJlO95#)|8|c{Nukb|A-i)<GK(}k<%|3*we3<L055DyuPxDhixtwRQi508)Kj~h
>z*^{)j$2q7>lZHDmlgReE6W}WDMmpx(s@?#5q>)y*$XQ1vS-^2_Q>cD3Zi&zU;ZF!J
>znpaLB&}rD37Z{t3r^`|`+F-M~+Xo=LgCK5R8Dc!^Z+SIKi67xZI;O_eFYE^4m^zNV
>zoBrGd>>WCRiOYM~AZ!W1klZH+_=jj~mnjIBG%`7akBlr?YKy!mc{ZR5C3Yiu<^JFM
>zkL_|xKDDZk)@JkEGY6{4xf_jmUbknCW3nw<?rLG6XJZ8WROKbA2T?QfI@veUwnxP1
>zQ$4*E+{=!)i(gb};3)N9F_W1Bh1m_7KNV#%limYAL;j?!%eYdN3FT#B?Bw`anPqhJ
>zgB9~~v`*q&=fpH*7__~wJj#qQl2O{S7)9|NzAH`8Y0#ePbsZXoqy38&h&?+&_}<Wl
>zq@`!%ogyT5|DKshu8ZvRLiX#d2j<fV4yv5a8x&76;TEh-!DN`YBVlN;yRf9Z!Kh+d
>zNcF3-Bsx#CCJgMm1jC&r>lu<8H<hO*OBa`>HoWLuMJfj+b6(D0Q0J&N?FK~`uhsR!
>zTlyqB<=7H&FNpQ1dl#K%9m>}xh9V<Z*_{eNeBwVzwai<pr>@G=&sGFh_U|g#rk`9r
>z8kenz-Fpo_l1+07@=h?pm$v9iUJX+gU^=E}C9x*Zis{@3U5AVAc4<U}E)e7Evy3Wd
>za8#m@|F2222cN<Um$J#R%|oWCo*Eq`rmE`gPlFQ_!_h_vY?d-iC~Y@Bza8Y+=$Tg~
>z_6`tq#|-N7yNw{wmyT@;UjE%&)*2NhL`}w_#e;M5z=gO++YRc08MJZpWT8>3EC;*>
>zwyx9yEs!8g&F@X)?P3`iyODT_x*SOCFJAU8FY{mc+yWi3aiwCkTd|PC4N65&b9IF&
>zl0@sb#31L=3H|Cl(QIXe{~FdNx3MArT7H71x2A;*8_+O&uSJ}6K(2Btu=?^XwNaQ|
>ztE`pz>^<`bb4TLpv+}!}cms-ZK!pDv>iwVm4<_M6Ywu2C+*te5q%sIi6!xSCxrY4)
>zgIJXzr{weq*LywOD2c49-TsrrlawAmCo_N}F2_G&<}_?2qQK}ps-*DlqiZ)sGhZMq
>z>V>ZZ=eQ{kC2J_vdAeSD9NNlBxu!`Z2*mj%8vDr0h*}US>BClxt)IflgJ$?ZjIA3#
>z%I_6+?s1_GfxeUIlqSu`qV?c5xp2lnpPH+T<K?XW1?CqJ4Z0gHEs98Im#)=D7X=~~
>zM{nE|>CwbTi@%BBhCwtQZWMv$ksrBR=9c-|cB3!5<`-K^WZ+v~o@Du7@alBf6Q~RS
>zrpHhGvV?55uqJkXE~LqUTlcw!yD2|XnI{<?rMi8`Bv;IFO*!iNLvb-o8Zpc-(0l`u
>z$c&T4rU$avbSogy*fKNSH$jIQ&zFejqik#h+%uG>{K~Lt#&KjZBS6D~UcjsHOZ@nu
>ztDyHd<fFqL79d<LPX=9Ile0;1L2F&YWGW0<ELiYO^rTxl;XNF6@YYkqub{^L#7CV&
>z!!#nbhd3=nWC>^QDuBE*dP^T%D(uB{dD)H6<6(wM>^{6KBu=!<ZQScdN+($cm3|U!
>zO5ipSo}I4dMtwc!l7`B3cb!oo5`%Po<SXqa+FbHOAh_XP$26ADSoeD%PfHI{l~})6
>z6<ziC34c^Xx@}@D4mxYk>Y``Nxbp3s{Y{@GJoh>P!h>pmAN<gbp&m&qmYSy<A3roo
>zpLUr`op~Zit~5=A5a>*go})b!^(v)Y(Fxg;8*K$P%IQc?EkjK~jUGHag>wZ*ZG(Q7
>z{E`)PF^cCKug4+;QVzyrLGz<7`+WPAl(DA-JdBYbdYm3*&UKdql{D$jKcDOblQtTb
>zH(g@WB{p5+*D7JtB{rkPW|a7C;XKUKR@(@N-nLYNm@~4>s;gs~OF0FL%eA+NA=9EH
>zJJv}D9r8uInzQSSQ?Vuo>WvMyv^+296$fLtf$?*}B+W8yo-FmBl*d~UgV7NvdmnjK
>zj;qj_XV>9~FkEEMa-+;lEbjD5JzsaW#c0`wk`u0&6E_YgktMz+d?kO*rOW{@2)A;c
>z%O=@M_?5ze*0IGJd4Ni;A8Emq=<k{Fc&@3*2|<cI(-xPfNVagskaHR(zd!bEF~+UG
>zHM37J5zw}hUbGrw=*liB*EzL%KmN8UGs`J`%MK8$8LHw&#chl)R3MQh$`EslJ7lUh
>zAJQpshvYcwz)i4*&nh8YM?@Oa13H@{2fSXyWNkydM1&m-WS>jp6v)a>s24poW8N-4
>zvU3Y?c_iDG&3#EL)6FGqDRqrUo|&xa<GgNfaFPfxlqYznJ$U@d!YhMj04)7+DX<%i
>zBPNQx4uqJw-yTX)%(L%nZ!N&uq(8c5U?--lD1yAOf6Mo&Z^%@biNza*%&j0V;&;*;
>z*w?ZbUZEJ}{tnDOZQn?}W$l?+F7`;yXhpwxi3B&HX7jVseNjduq#<S?dBO9%^i6bI
>zPe~14mhiqFLd*AG>XUY`92!ZP?79&CG%l=|WSVt|G2s#i#sk#OS9r$t1rPH3{h#M~
>z8x1kLYsiaHu)T8N;Ivkc)_QB`lcZ>k)S}h-T(7Ml{5$f&W!Vu;t@0lp)(rENQz-*T
>z+yyPLpT=bNB7zlnfbX-Lw}4L$RRQqb-weRj%QACr4sHI4O-*dN!X^|pS-2S${%d?R
>zA6u05dt9#e_VyN12bF%C`u)kn<#%hglv!I^E*B2~5dMQ~VN3GgbyZbWtyfaGf4x;B
>ag;r*E7IYlBPn5Ny0Q`i@@rp0Jum1wA-LKmK
>
>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..3409fc793a4999bd8672800a78d547b8ddaae316
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint-element-on-subpixel-position-expected.txt
>@@ -0,0 +1,33 @@
>+layer at (0,0) size 833x609
>+  RenderView at (0,0) size 785x585
>+layer at (0,0) size 785x585
>+  RenderBlock {HTML} at (0,0) size 785x585
>+    RenderBody {BODY} at (8,8) size 769x569
>+layer at (0,439) size 785x146
>+  RenderBlock (positioned) {DIV} at (0,438) size 785x147
>+layer at (8,24) size 825x585
>+  RenderBlock (positioned) {UL} at (8,24) size 825x585
>+layer at (48,24) size 785x24
>+  RenderListItem {LI} at (40,0) size 785x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 1"
>+layer at (48,48) size 785x25
>+  RenderListItem {LI} at (40,24) size 785x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 2"
>+layer at (48,73) size 785x24
>+  RenderListItem {LI} at (40,48) size 785x26 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,97) size 785x24
>+  RenderListItem {LI} at (40,73) size 785x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,121) size 785x25
>+  RenderListItem {LI} at (40,97) size 785x25 [bgcolor=#FFA500]
>+    RenderText {#text} at (0,0) size 51x17
>+      text run at (0,0) width 51: "TEST 3"
>+layer at (48,146) size 785x24
>+  RenderListItem {LI} at (40,121) size 785x26 [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..30575b58a25396d1a275dede2fd048e8ec16b8f9
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint-element-on-subpixel-position.html
>@@ -0,0 +1,43 @@
>+<html>
>+<head>
>+<style>
>+ul {
>+   height: 100%;
>+   width: 100%;
>+   position: absolute;
>+   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:75%;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