The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla8

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: heeen, Assigned: romaxa)

Tracking

Trunk
mozilla8
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
Created attachment 544213 [details]
second example
(Reporter)

Comment 2

6 years ago
Created attachment 544217 [details]
layers log for settings page
(Reporter)

Comment 3

6 years ago
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.
(Reporter)

Comment 4

6 years ago
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.
Attachment #544258 - Flags: review?(joe)
(Assignee)

Comment 5

6 years ago
this must be applied for good UI OGL rendering
Blocks: 607684
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

Updated

6 years ago
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)
(Assignee)

Comment 9

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

Updated

6 years ago
Attachment #544258 - Flags: review?(matt.woodrow)
(Assignee)

Comment 10

6 years ago
Roc seems on vacation... Matt could you check this patch? try build seems ok
(Assignee)

Comment 11

6 years ago
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?
(Assignee)

Comment 13

6 years ago
> >        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)
(Assignee)

Comment 14

6 years ago
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.
Attachment #544258 - Attachment is obsolete: true
Attachment #551628 - Flags: review?(roc)
Attachment #544258 - Flags: review?(roc)
Attachment #551628 - Flags: review?(roc) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Whiteboard: review-needed
I can check this in tomorrow. Does it need a try run?
(Assignee)

Comment 16

6 years ago
It had some try build in comment 9

Updated

6 years ago
Assignee: nobody → romaxa
Inbound:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=aee7dcfde223
Keywords: checkin-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/aee7dcfde223
Status: NEW → RESOLVED
Last Resolved: 6 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.