Last Comment Bug 669602 - Fennec/OGL: Paint Artifacts on chrome pages' background after scrolling
: Fennec/OGL: Paint Artifacts on chrome pages' background after scrolling
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: mozilla8
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: opengl-mobile
  Show dependency treegraph
 
Reported: 2011-07-06 05:55 PDT by Florian Hänel [:heeen]
Modified: 2011-08-10 08:25 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
missing background fills (27.60 KB, image/png)
2011-07-06 05:55 PDT, Florian Hänel [:heeen]
no flags Details
second example (31.57 KB, image/png)
2011-07-06 05:55 PDT, Florian Hänel [:heeen]
no flags Details
layers log for settings page (11.35 KB, text/x-log)
2011-07-06 06:02 PDT, Florian Hänel [:heeen]
no flags Details
isolated fix for this draw bug (2.24 KB, patch)
2011-07-06 08:51 PDT, Florian Hänel [:heeen]
joe: review+
Details | Diff | Review
Minimal version (1.54 KB, patch)
2011-08-08 17:10 PDT, Oleg Romashin (:romaxa)
roc: review+
Details | Diff | Review

Description Florian Hänel [:heeen] 2011-07-06 05:55:04 PDT
Created attachment 544212 [details]
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] 2011-07-06 05:55:33 PDT
Created attachment 544213 [details]
second example
Comment 2 Florian Hänel [:heeen] 2011-07-06 06:02:54 PDT
Created attachment 544217 [details]
layers log for settings page
Comment 3 Florian Hänel [:heeen] 2011-07-06 07:44:30 PDT
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] 2011-07-06 08:51:17 PDT
Created attachment 544258 [details] [diff] [review]
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) 2011-07-06 18:40:18 PDT
this must be applied for good UI OGL rendering
Comment 6 Joe Drew (not getting mail) 2011-07-07 15:28:41 PDT
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) 2011-07-25 06:48:17 PDT
Review ping jmuizelaar
Comment 8 Jeff Muizelaar [:jrmuizel] 2011-07-26 17:21:58 PDT
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) 2011-07-27 14:38:14 PDT
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) 2011-07-28 09:07:30 PDT
Roc seems on vacation... Matt could you check this patch? try build seems ok
Comment 11 Oleg Romashin (:romaxa) 2011-07-28 17:20:10 PDT
Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

Roc is on vacation.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-08-01 16:26:08 PDT
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) 2011-08-01 18:12:38 PDT
> >        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) 2011-08-08 17:10:40 PDT
Created attachment 551628 [details] [diff] [review]
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) 2011-08-08 20:35:52 PDT
I can check this in tomorrow. Does it need a try run?
Comment 16 Oleg Romashin (:romaxa) 2011-08-08 22:56:53 PDT
It had some try build in comment 9
Comment 17 Benoit Girard (:BenWa) 2011-08-09 08:39:37 PDT
Inbound:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=aee7dcfde223
Comment 18 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-10 08:25:35 PDT
http://hg.mozilla.org/mozilla-central/rev/aee7dcfde223

Note You need to log in before you can comment on or make changes to this bug.