This is probably a dupe of Bug 715658, but I'll file just to be sure. <div> backgrounds are sometimes garbled or not updated on scroll. Its hard to describe and examples are sparsely random. Usually images that have a transparent background contained in a <div> or backgrounds of <div> elements are either dark or leave an after image of what was scrolled through. In some rare cases, the <div> background images were corrupted. That last part sounds similar to the other progressive jpeg bug that was fixed not too long ago. An example can be found using Google Reader. Here's a video from Fanolian who also noticed the bug: https://www.youtube.com/watch?v=xPBugPaWGoQ Keep your eye on the gray shortcut marker next to the Title of the feed entry. I'll add more examples once I find more consistent sources.

I can give a reliable example from the background of this page... This one the background won't paint at all. Fine if azure is off though. http://support.amd.com/us/Pages/Catalyst-Hotfixes.aspx If you click on the links from the page above (any link that takes you to a scrollable page that is) scroll it up and down rapidly and you will see parts of the background simply disappear and reappear while scrolling. This is also on on nightly windows 7 and windows 8 with HW Acceleration on.

Here's another example: 1. Visit http://penny-arcade.com/ 2. Scroll down 3. Scroll back to the top Notice that the orange tabs bleed into the <li> background element.

(In reply to Leman Bennett (Omega X) from comment #2) > Here's another example: > > 1. Visit http://penny-arcade.com/ > 2. Scroll down > 3. Scroll back to the top > > Notice that the orange tabs bleed into the <li> background element. I can see this, this is very fascinating, I have no idea at this point which causes this.

I believe this bug has to do with incorrect functioning of PushGroupAndCopyBackground. Mainly due to some oversights in implementing features there. I'm working on confirming that diagnosis.

Comment on attachment 616134 [details] [diff] [review] Part 1: Add OpaqueRect API to DrawTargets. Review of attachment 616134 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/2D.h @@ +785,5 @@ > + } > + > + const IntRect &GetOpaqueRect() const { > + return mOpaqueRect; > + } This desperately needs some (precise) documentation!

Maybe it would make more sense to just have a userdata API for DrawTargets and attach the opaque rect with that API?

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (offline April 22-25) from comment #9) > Maybe it would make more sense to just have a userdata API for DrawTargets > and attach the opaque rect with that API? Ugh, I missed your review comments. Well, we could, but this information could potentially be useful for DrawTargets to, for example, know when they can do subpixel AA, or other kinds of optimizations. (i.e. I was created as an alpha surface, but my opaque rect now covers all of my surface, which can happen in PNG decoding for example. I can now memcpy rather than blend, etc.)

OK, then you need to update the patch to document the precise semantics of what this means, e.g. does it mean the current pixels are already opaque alpha values, or we will set them to opaque alpha values, etc.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #11) > OK, then you need to update the patch to document the precise semantics of > what this means, e.g. does it mean the current pixels are already opaque > alpha values, or we will set them to opaque alpha values, etc. Agreed, my current thought is as follows: 'Within this rectangle all pixels will be opaque by the time the result of this DrawTarget is first used as a source for drawing.' This would be sufficient, as any pixels that are subpixel AA'ed would, within this definition, either be overwritten by an opaque pixel or be on top of an opaque pixel once rendering is finished.

(In reply to Bas Schouten (:bas) from comment #12) > 'Within this rectangle all pixels will be opaque by the time the result of > this DrawTarget is first used as a source for drawing.' Can you define that a bit more precisely?

FWIW this patch has fixed all of the remaining issues for me using gfx.content.azure.enabled true here locally. Haven't hit any bad backgrounds or corrupted images for almost a week.

(In reply to Michael from comment #14) > FWIW this patch has fixed all of the remaining issues for me using > gfx.content.azure.enabled true here locally. Haven't hit any bad backgrounds > or corrupted images for almost a week. The patches in this bug has not landed. Also, I can still reproduce the issues reported.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13) > (In reply to Bas Schouten (:bas) from comment #12) > > 'Within this rectangle all pixels will be opaque by the time the result of > > this DrawTarget is first used as a source for drawing.' > > Can you define that a bit more precisely? Hrm, I can add my explanation of why that description is sufficient? Do you have an idea of how you would you like to propose expanding the definition?

(In reply to Leman Bennett (Omega X) from comment #15) > (In reply to Michael from comment #14) > > FWIW this patch has fixed all of the remaining issues for me using > > gfx.content.azure.enabled true here locally. Haven't hit any bad backgrounds > > or corrupted images for almost a week. > > The patches in this bug has not landed. Also, I can still reproduce the > issues reported. I am using a private build that includes this patch :)

(In reply to Bas Schouten (:bas) from comment #16) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13) > > (In reply to Bas Schouten (:bas) from comment #12) > > > 'Within this rectangle all pixels will be opaque by the time the result of > > > this DrawTarget is first used as a source for drawing.' > > > > Can you define that a bit more precisely? > > Hrm, I can add my explanation of why that description is sufficient? Do you > have an idea of how you would you like to propose expanding the definition? What exactly does it mean to "be used as a source for drawing"?

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #18) > (In reply to Bas Schouten (:bas) from comment #16) > > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13) > > > (In reply to Bas Schouten (:bas) from comment #12) > > > > 'Within this rectangle all pixels will be opaque by the time the result of > > > > this DrawTarget is first used as a source for drawing.' > > > > > > Can you define that a bit more precisely? > > > > Hrm, I can add my explanation of why that description is sufficient? Do you > > have an idea of how you would you like to propose expanding the definition? > > What exactly does it mean to "be used as a source for drawing"? How about: 'Within this rectangle all pixels will be opaque by the time the result of this DrawTarget is first used for drawing. Either by the backing texture being used as the input, or a snapshot being taken.'

'Within this rectangle all pixels will be opaque by the time the result of this DrawTarget is first used for drawing. Either by the backing texture being used as the input, or a snapshot being taken.' What does "the backing texture being used as the input" mean, from the point of view of a DrawTarget user who doesn't know how all the DrawTarget backends are implemented? "A snapshot being taken" means a call to Snapshot() right? Just say Snapshot() instead.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #20) > 'Within this rectangle all pixels will be opaque by the time the result of > this DrawTarget is first used for drawing. Either by the backing texture > being used as the input, or a snapshot being taken.' > > What does "the backing texture being used as the input" mean, from the point > of view of a DrawTarget user who doesn't know how all the DrawTarget > backends are implemented? > > "A snapshot being taken" means a call to Snapshot() right? Just say > Snapshot() instead. New attempt :) 'Within this rectangle all pixels will be opaque by the time the result of this DrawTarget is first used for drawing. Either by the underlying surface being used as an input to external drawing, or Snapshot() being called.

(In reply to Bas Schouten (:bas) from comment #21) > 'Within this rectangle all pixels will be opaque by the time the result of > this DrawTarget is first used for drawing. Either by the underlying surface > being used as an input to external drawing, or Snapshot() being called. I'm still not completely happy with that --- what does it mean to be used as an input for external drawing? --- but I'll move on...

Comment on attachment 616134 [details] [diff] [review] Part 1: Add OpaqueRect API to DrawTargets. Review of attachment 616134 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the comment we agreed on. Also, I assume it's in "device space"?

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #23) > Comment on attachment 616134 [details] [diff] [review] > Part 1: Add OpaqueRect API to DrawTargets. > > Review of attachment 616134 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ with the comment we agreed on. Also, I assume it's in "device space"? Indeed, will add this to the documentation.

Comment on attachment 616136 [details] [diff] [review] Part 2: Manage OpaqueRect in BasicLayers for Azure-Thebes. Review of attachment 616136 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/basic/BasicLayers.cpp @@ +1930,4 @@ > const nsIntRect& bounds = visibleRegion.GetBounds(); > > if (aTarget->IsCairo()) { > const gfxRect& targetOpaqueRect = currentSurface->GetOpaqueRect(); Actually, I think this entire "if" statement needs to be inside an "if (is2D)"! Please fix! @@ +1954,5 @@ > + Rect(bounds.x, bounds.y, bounds.width, bounds.height)); > + opaqueRect.RoundIn(); > + aTarget->GetDrawTarget()->SetOpaqueRect( > + IntRect(int32_t(opaqueRect.x), int32_t(opaqueRect.y), > + int32_t(opaqueRect.width), int32_t(opaqueRect.height))); I think something needs to be done here to avoid overflow. Something like gfxUtils::GfxRectToIntRect.

Comment on attachment 616138 [details] [diff] [review] Part 3: Fix up PushGroupAndCopyBackground to behave consistent with non-Azure. Review of attachment 616138 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxContext.cpp @@ +1564,5 @@ > + gfxRect clipRect = GetRoundOutDeviceClipExtents(this); > + clipExtents = IntRect(clipRect.x, clipRect.y, clipRect.width, clipRect.height); > + } > + if (mDT->GetFormat() == FORMAT_B8G8R8X8 || > + mDT->GetOpaqueRect().Contains(clipExtents)) { Fix indent

Comment on attachment 620084 [details] [diff] [review] Part 2: Manage OpaqueRect in BasicLayers for Azure-Thebes. v2 Review of attachment 620084 [details] [diff] [review]: ----------------------------------------------------------------- r+ with that ::: gfx/thebes/gfxUtils.h @@ +129,5 @@ > + * If aIn can be represented exactly using an nsIntRect (i.e. > + * integer-aligned edges and coordinates in the PRInt32 range) then we > + * set aOut to that rectangle, otherwise return failure. > + */ > + static bool RectToIntRect(const mozilla::gfx::Rect& aIn, mozilla::gfx::IntRect* aOut); Actually, just make this a method of Rect: bool ToIntRect(IntRect* aOut);

https://hg.mozilla.org/integration/mozilla-inbound/rev/d32d6823f0af https://hg.mozilla.org/integration/mozilla-inbound/rev/f9a0af04be0b https://hg.mozilla.org/integration/mozilla-inbound/rev/0187b4f0c50f

Followup to address comment 28: https://hg.mozilla.org/integration/mozilla-inbound/rev/43b2b050af51

https://hg.mozilla.org/mozilla-central/rev/0187b4f0c50f https://hg.mozilla.org/mozilla-central/rev/f9a0af04be0b https://hg.mozilla.org/mozilla-central/rev/d32d6823f0af https://hg.mozilla.org/mozilla-central/rev/43b2b050af51