669602 - Fennec/OGL: Paint Artifacts on chrome pages' background after scrolling

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Florian Hänel [:heeen]]

Attachment: missing background fills

There seems to be a bug in painting to OGL layers where background images are not filled correctly or the component alpha split layers are not painted correctly. See the images for examples.

Comment 1

[Florian Hänel [:heeen]]

Attachment: second example

Comment 2

[Florian Hänel [:heeen]]

Attachment: layers log for settings page

Comment 3

[Florian Hänel [:heeen]]

I found that this gets fixed with 
https://bugzilla.mozilla.org/attachment.cgi?id=542650
and
https://bugzilla.mozilla.org/attachment.cgi?id=542649
which already are in my patch queue - I only noticed this bug when I tested a single patch from the queue.

Comment 4

[Florian Hänel [:heeen]]

Attachment: isolated fix for this draw bug

This is the fix from the other bug. It seems we are drawing a concave region which causes the fill operation to behave differently to the image op.

Comment 5

[Oleg Romashin (:romaxa)]

this must be applied for good UI OGL rendering

Comment 6

[Joe Drew (not getting mail)]

Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

Review of attachment 544258 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, though I would like additional review from Jeff too.

Comment 7

[Benoit Girard (:BenWa)]

Review ping jmuizelaar

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

I'm not really qualified to review this. At a minimum the comment near the OR needs to be fixed.

Comment 9

[Oleg Romashin (:romaxa)]

Looks like try build does not show any reftest failures on Mac
http://tbpl.mozilla.org/?tree=Try&rev=f26698224ed3

Comment 10

[Oleg Romashin (:romaxa)]

Roc seems on vacation... Matt could you check this patch? try build seems ok

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

Roc is on vacation.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

Review of attachment 544258 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/ThebesLayerOGL.cpp
@@ +617,5 @@
> +  // with concave regions. Right now the scrollbars invalidate a narrow strip of the awesomebar
> +  // although they never cover it. This leads to two draw rects, the narow strip and the actually
> +  // newly exposed area. It would be wise to fix this glitch in any way to have simpler
> +  // clip and draw regions.
> +  gfxUtils::ClipToRegion(result.mContext, result.mRegionToDraw);

This is definitelyright. r+ on this part.

@@ +722,5 @@
>        // OR-ing with aRegionToDraw, since that can lead to a very complex region
>        // here (OR doesn't automatically simplify to the simplest possible
>        // representation of a region.)
>        nsIntRegion tmp;
> +      tmp.And(mVisibleRegion, state.mRegionToDraw);

I don't understand this change. What's wrong with the existing code here?

Comment 13

[Oleg Romashin (:romaxa)]

> >        nsIntRegion tmp;
> > +      tmp.And(mVisibleRegion, state.mRegionToDraw);
> 
> I don't understand this change. What's wrong with the existing code here?

Not sure what is real reason, probably florian will explain tomorrow, but without this change I see black rendering on fennec sidebars when those swiping out to the screen (when layer is visible then it is painted correctly)

Comment 14

[Oleg Romashin (:romaxa)]

Attachment: Minimal version

Double checked this fix, and it seems second part only needed for patches in another bug 608983.
Tested only this small change and it is fixing artifacts problem.

Comment 15

[Benoit Girard (:BenWa)]

I can check this in tomorrow. Does it need a try run?

Comment 16

[Oleg Romashin (:romaxa)]

It had some try build in comment 9

Comment 17

[Benoit Girard (:BenWa)]

Inbound:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=aee7dcfde223

Comment 18

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/aee7dcfde223