Bug 169094 - [GTK] Rendering glitches in HiDPI in long GitHub Gist pages when focusing the comments textarea
Summary: [GTK] Rendering glitches in HiDPI in long GitHub Gist pages when focusing the...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Gwang Yoon Hwang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-02 13:42 PST by Adrian Perez
Modified: 2017-05-15 02:06 PDT (History)
7 users (show)

See Also:


Attachments
Screenshot of Epiphany running 2.15.91 (207.29 KB, image/png)
2017-03-02 13:43 PST, Adrian Perez
no flags Details
If you see the red when you input the space, it faiils. (798 bytes, text/html)
2017-04-09 05:11 PDT, Gwang Yoon Hwang
no flags Details
Patch (6.05 KB, patch)
2017-05-12 07:37 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff
Patch (6.02 KB, patch)
2017-05-12 07:57 PDT, Gwang Yoon Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2017-03-02 13:42:46 PST
Steps to reproduce:
 1. Open https://gist.github.com/aperezdc/157dabed39648865b692022ec37b44bf
 2. Scroll to the bottom.
 3. Click on the textarea to write a comment.

The area of the screen occupied by the textarea will get garbled and include
fragments of the previous screenfuls of the webpage. I can reproduce the issue
in Epihany both with WebKitGTK+ releases 2.14.5 (latest stable) and 2.15.91
(latest development), and also with MiniBrowser built from trunk.
Comment 1 Adrian Perez 2017-03-02 13:43:26 PST
Created attachment 303231 [details]
Screenshot of Epiphany running 2.15.91
Comment 2 Adrian Perez 2017-03-02 13:48:52 PST
Important data point:

 - The issue happens when using a scale-factor of 2x (HiDPI).
 - With no scaling (the scale-factor is 1x) the bug is *not reproducible*.

Could this be related to bug #168128 somehow?
Comment 3 Miguel Gomez 2017-03-03 00:18:13 PST
I'll give this a look as soon as I can.
Comment 4 Gwang Yoon Hwang 2017-04-09 05:10:48 PDT
This bug was reported to occur only at the HiDPI rendering case. However, when
I made a minimal test case, the test runner with a 1.0 scaled Weston instance
could reproduce this problem.

This bug happens when we are drawing the border of the inline box which has a
complex border (such as dashed, dotted..) with a border radius property.

When we made a change inside of text box, for example, typing space, we are 
going to invalidate the area of the inline box.

While rendering decorations of the render box,
RenderBoxModelObject::paintBorder defines a clip out region to prepare the inner
border to avoid the background bleeding out behind the border.

We are facing a problem from now on. 

The RenderBoxModelObject is going to make a clip to render each side of the
border area. But when we apply this clipping to the cairo context, it seems
cairo context looses the previous clipped out area, which was defined to avoid
bleeding.

It is okay when we draw dashed border side since it is a stroke. But when we
draw a solid border side, it uses fill operation which would fill the whole
quad of the border side with a border color.

I tried to make simple cairo problem to reproduce the problem but it is not so
successful so far.
Comment 5 Gwang Yoon Hwang 2017-04-09 05:11:59 PDT
Created attachment 306618 [details]
If you see the red when you input the space, it faiils.
Comment 6 Michael Catanzaro 2017-04-09 07:10:59 PDT
(In reply to Gwang Yoon Hwang from comment #5)
> Created attachment 306618 [details]
> If you see the red when you input the space, it faiils.

That is definitely bug #126124, but it doesn't look anything like the screenshot in comment #1. Are these the same issue?
Comment 7 Gwang Yoon Hwang 2017-04-09 23:22:46 PDT
(In reply to Michael Catanzaro from comment #6)
> (In reply to Gwang Yoon Hwang from comment #5)
> > Created attachment 306618 [details]
> > If you see the red when you input the space, it faiils.
> 
> That is definitely bug #126124, but it doesn't look anything like the
> screenshot in comment #1. Are these the same issue?

Ah.. It is my mistake. It was #126124.
I was confused this with #126124. I will update that bug instead of this.
(this bug was my next target. :p)
Comment 8 Gwang Yoon Hwang 2017-04-09 23:23:10 PDT
Comment on attachment 306618 [details]
If you see the red when you input the space, it faiils.

wrong test cast for this bug.
Comment 9 Gwang Yoon Hwang 2017-05-12 07:37:40 PDT
Created attachment 309892 [details]
Patch
Comment 10 Gwang Yoon Hwang 2017-05-12 07:41:02 PDT
(In reply to Gwang Yoon Hwang from comment #9)
> Created attachment 309892 [details]
> Patch

I made a patch and a ref-test for it.
However, I have a problem with a ref-test.
Since this bug break every renderings after it appears, I need to restart
webprocess after loading the test case not to break ref results and other test cases.
Is there any API to reset webprocess before loading expected.html?
Comment 11 Gwang Yoon Hwang 2017-05-12 07:57:33 PDT
Created attachment 309894 [details]
Patch
Comment 12 Gwang Yoon Hwang 2017-05-12 07:59:54 PDT
(In reply to Gwang Yoon Hwang from comment #10)
> (In reply to Gwang Yoon Hwang from comment #9)
> > Created attachment 309892 [details]
> > Patch
> 
> I made a patch and a ref-test for it.
> However, I have a problem with a ref-test.
> Since this bug break every renderings after it appears, I need to restart
> webprocess after loading the test case not to break ref results and other
> test cases.
> Is there any API to reset webprocess before loading expected.html?

No, it was enough to use non-accelerated composited mode to reset the update atlas.
Updated patch is ready for review.
Comment 13 Carlos Garcia Campos 2017-05-13 01:00:10 PDT
(In reply to Gwang Yoon Hwang from comment #12)
> (In reply to Gwang Yoon Hwang from comment #10)
> > (In reply to Gwang Yoon Hwang from comment #9)
> > > Created attachment 309892 [details]
> > > Patch
> > 
> > I made a patch and a ref-test for it.
> > However, I have a problem with a ref-test.
> > Since this bug break every renderings after it appears, I need to restart
> > webprocess after loading the test case not to break ref results and other
> > test cases.
> > Is there any API to reset webprocess before loading expected.html?
> 
> No, it was enough to use non-accelerated composited mode to reset the update
> atlas.
> Updated patch is ready for review.

We should ensure tests are independent to each other, if the update atlas cache, or any other coord graphics cache might affect following tests we should ensure we clear everything when resetting tests to consistent values, adding internal API if needed. Maybe this is the reason why we had so many flaky tests when we ran all the tests with AC mode forced.
Comment 14 Zan Dobersek 2017-05-14 23:35:43 PDT
Comment on attachment 309894 [details]
Patch

What's the 'limit of Pixman' here? 32768, as in r212431?
Comment 15 Gwang Yoon Hwang 2017-05-15 00:18:06 PDT
(In reply to Carlos Garcia Campos from comment #13)
> (In reply to Gwang Yoon Hwang from comment #12)
> > (In reply to Gwang Yoon Hwang from comment #10)
> > > (In reply to Gwang Yoon Hwang from comment #9)
> > > > Created attachment 309892 [details]
> > > > Patch
> > > 
> > > I made a patch and a ref-test for it.
> > > However, I have a problem with a ref-test.
> > > Since this bug break every renderings after it appears, I need to restart
> > > webprocess after loading the test case not to break ref results and other
> > > test cases.
> > > Is there any API to reset webprocess before loading expected.html?
> > 
> > No, it was enough to use non-accelerated composited mode to reset the update
> > atlas.
> > Updated patch is ready for review.
> 
> We should ensure tests are independent to each other, if the update atlas
> cache, or any other coord graphics cache might affect following tests we
> should ensure we clear everything when resetting tests to consistent values,
> adding internal API if needed. Maybe this is the reason why we had so many
> flaky tests when we ran all the tests with AC mode forced.

I agree, maybe we need some codes to detect same pixman failure when running AC tests.
Also, I will create another bug to reset the update atlas for root compositing layer.
Comment 16 Gwang Yoon Hwang 2017-05-15 00:20:17 PDT
(In reply to Zan Dobersek from comment #14)
> Comment on attachment 309894 [details]
> Patch
> 
> What's the 'limit of Pixman' here? 32768, as in r212431?

Yes, same here. PIXMAN_MAX_INT aka 32768.
Comment 17 WebKit Commit Bot 2017-05-15 02:06:57 PDT
Comment on attachment 309894 [details]
Patch

Clearing flags on attachment: 309894

Committed r216859: <http://trac.webkit.org/changeset/216859>
Comment 18 WebKit Commit Bot 2017-05-15 02:06:59 PDT
All reviewed patches have been landed.  Closing bug.