Bug 192230 – REGRESSION(r235165): [GTK][WPE] Garbled rendering on GitLab

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

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.

[Michael Catanzaro]

Created attachment 356323 [details]
Different symptom: mismatched tiles

My guess is that AC mode is broken in 2.23.1, not sure.

[Michael Catanzaro]

Indeed I haven't noticed any problems with AC mode disabled.

[Michael Catanzaro]

Second screenshot here looks like bug #178387.

[Zan Dobersek]

Bisected all the way down to r235165.
https://trac.webkit.org/changeset/235165/webkit

[Michael Catanzaro]

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.

[Carlos Bentzen]

(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?

[Michael Catanzaro]

And are the two screenshots the same bug? The second screenshot looks like bug #178387.

[Zan Dobersek]

(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.

[Zan Dobersek]

(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.

[Zan Dobersek]

Created attachment 357438 [details]
WIP

Please test.

[Michael Catanzaro]

(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.

[Carlos Bentzen]

(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.

[Zan Dobersek]

Created attachment 357464 [details]
Patch

[Carlos Bentzen]

(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.

[Carlos Garcia Campos]

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.

[Zan Dobersek]

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.

[Zan Dobersek]

Created attachment 357664 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 357664 [details]
Patch for landing

Clearing flags on attachment: 357664

Committed r239371: <https://trac.webkit.org/changeset/239371>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.