Closed Bug 669602 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Graphics, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: heeen, Assigned: romaxa)

References

Details

Attachments

(4 files, 1 obsolete file)

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.
Attached image second example
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.
Attached patch isolated fix for this draw bug (obsolete) — Splinter Review
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.
Attachment #544258 - Flags: review?(joe)
this must be applied for good UI OGL rendering
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.
Attachment #544258 - Flags: review?(joe)
Attachment #544258 - Flags: review?(jmuizelaar)
Attachment #544258 - Flags: review+
Review ping jmuizelaar
Whiteboard: review-needed
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.
Attachment #544258 - Flags: review?(jmuizelaar) → review?(roc)
Looks like try build does not show any reftest failures on Mac
http://tbpl.mozilla.org/?tree=Try&rev=f26698224ed3
Attachment #544258 - Flags: review?(matt.woodrow)
Roc seems on vacation... Matt could you check this patch? try build seems ok
Comment on attachment 544258 [details] [diff] [review]
isolated fix for this draw bug

Roc is on vacation.
Attachment #544258 - Flags: review?(roc)
Attachment #544258 - Flags: review?(matt.woodrow) → review?(roc)
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?
> >        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)
Attached patch Minimal versionSplinter Review
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.
Attachment #544258 - Attachment is obsolete: true
Attachment #551628 - Flags: review?(roc)
Attachment #544258 - Flags: review?(roc)
Keywords: checkin-needed
Whiteboard: review-needed
I can check this in tomorrow. Does it need a try run?
It had some try build in comment 9
Assignee: nobody → romaxa
http://hg.mozilla.org/mozilla-central/rev/aee7dcfde223
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.