Bug 192230 - REGRESSION(r235165): [GTK][WPE] Garbled rendering on GitLab
Summary: REGRESSION(r235165): [GTK][WPE] Garbled rendering on GitLab
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 192523
  Show dependency treegraph
 
Reported: 2018-11-30 09:34 PST by Michael Catanzaro
Modified: 2018-12-18 23:41 PST (History)
11 users (show)

See Also:


Attachments
Screenshot of garbled rendering (358.39 KB, image/png)
2018-11-30 09:34 PST, Michael Catanzaro
no flags Details
Different symptom: mismatched tiles (198.11 KB, image/png)
2018-12-01 14:50 PST, Michael Catanzaro
no flags Details
WIP (5.61 KB, patch)
2018-12-17 07:19 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (5.99 KB, patch)
2018-12-17 12:07 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch for landing (6.12 KB, patch)
2018-12-18 23:37 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2018-11-30 09:34:15 PST
Created attachment 356188 [details]
Screenshot of garbled rendering

2.23.1 introduces a regression where renedring is sometimes garbled on gitlab.gnome.org. See attached screenshot. Unfortunately this regression occurs only rarely, so it is not possible to reproduce or bisect. And 2.23.1 was relatively late, so there is a huge range of commits (any commit since 2.22!) that could be a suspect.
Comment 1 Michael Catanzaro 2018-12-01 14:50:08 PST
Created attachment 356323 [details]
Different symptom: mismatched tiles

My guess is that AC mode is broken in 2.23.1, not sure.
Comment 2 Michael Catanzaro 2018-12-04 13:41:15 PST
Indeed I haven't noticed any problems with AC mode disabled.
Comment 3 Michael Catanzaro 2018-12-07 16:09:20 PST
Second screenshot here looks like bug #178387.
Comment 4 Zan Dobersek 2018-12-08 10:08:20 PST
Bisected all the way down to r235165.
https://trac.webkit.org/changeset/235165/webkit
Comment 5 Michael Catanzaro 2018-12-08 10:30:46 PST
Thanks Zan!

Note that we would have caught this months ago had we done 2.23.1 on our usual schedule. In retrospect, delaying that release while Carlos was on leave was a mistake.
Comment 6 Carlos Bentzen 2018-12-15 13:47:09 PST
(In reply to Zan Dobersek from comment #4)
> Bisected all the way down to r235165.
> https://trac.webkit.org/changeset/235165/webkit

Zan, how did you reproduce this reliably? I guess you found a way to trigger this bug 100% to be able to bisect?
Comment 7 Michael Catanzaro 2018-12-15 17:54:44 PST
And are the two screenshots the same bug? The second screenshot looks like bug #178387.
Comment 8 Zan Dobersek 2018-12-17 07:15:20 PST
(In reply to Carlos Eduardo Ramalho from comment #6)
> (In reply to Zan Dobersek from comment #4)
> > Bisected all the way down to r235165.
> > https://trac.webkit.org/changeset/235165/webkit
> 
> Zan, how did you reproduce this reliably? I guess you found a way to trigger
> this bug 100% to be able to bisect?

It's not reliably reproducible.
Comment 9 Zan Dobersek 2018-12-17 07:18:27 PST
(In reply to Michael Catanzaro from comment #7)
> And are the two screenshots the same bug? The second screenshot looks like
> bug #178387.

I don't know. First screenshot is what I was able to fix. #178387 does not look related to this problem, but I don't know for sure what bug the second screenshot is capturing.
Comment 10 Zan Dobersek 2018-12-17 07:19:27 PST
Created attachment 357438 [details]
WIP

Please test.
Comment 11 Michael Catanzaro 2018-12-17 09:19:09 PST
(In reply to Zan Dobersek from comment #10)
> Created attachment 357438 [details]
> WIP
> 
> Please test.

Since the bug is not reproducible, I don't think this will get significant testing until it's released with 2.23.2. Alternatively, I can add the patch to the GNOME runtime now, if you want.
Comment 12 Carlos Bentzen 2018-12-17 10:38:49 PST
(In reply to Michael Catanzaro from comment #11)
> (In reply to Zan Dobersek from comment #10)
> > Created attachment 357438 [details]
> > WIP
> > 
> > Please test.
> 
> Since the bug is not reproducible, I don't think this will get significant
> testing until it's released with 2.23.2. Alternatively, I can add the patch
> to the GNOME runtime now, if you want.

I hit the bug somewhat frequently scrolling Gitlab issues page. Let me just try it out first and see if is not there at least. I shall be able to test it later tonight.

But I agree that if we can't have a reduction for it and add a new test, then just a development release to ensure it's been fixed.
Comment 13 Zan Dobersek 2018-12-17 12:07:05 PST
Created attachment 357464 [details]
Patch
Comment 14 Carlos Bentzen 2018-12-17 20:07:16 PST
(In reply to Zan Dobersek from comment #13)
> Created attachment 357464 [details]
> Patch

Bug doesn't happen for me anymore on gitlab issues page after applying this patch.

It was pretty frequent to reproduce on 2.23.1.
Comment 15 Carlos Garcia Campos 2018-12-18 00:07:13 PST
Comment on attachment 357464 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357464&action=review

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBackingStore.cpp:60
> +    m_updates.clear();

Could updateTile be called during the loop? In that case I would move the vector before iterating it.
Comment 16 Zan Dobersek 2018-12-18 23:24:23 PST
Comment on attachment 357464 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357464&action=review

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBackingStore.cpp:60
>> +    m_updates.clear();
> 
> Could updateTile be called during the loop? In that case I would move the vector before iterating it.

It shouldn't be possible, but it make sense to do the move.
Comment 17 Zan Dobersek 2018-12-18 23:37:24 PST
Created attachment 357664 [details]
Patch for landing
Comment 18 Zan Dobersek 2018-12-18 23:41:21 PST
Comment on attachment 357664 [details]
Patch for landing

Clearing flags on attachment: 357664

Committed r239371: <https://trac.webkit.org/changeset/239371>
Comment 19 Zan Dobersek 2018-12-18 23:41:25 PST
All reviewed patches have been landed.  Closing bug.