1009184 - ASSERTION: Empty paint region '!aPaintRegion.GetBounds().IsEmpty()'

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Ever since the tiling code landed in bug 783368 there has been a codepath that unconditionally triggers this assertion failure. In current code, the call site at [2] passes in a empty paint region to the function which then asserts at [1]. This code appears to have not really been exercised, but in bug 1001438 the changes I'm making seem to make it exercised in some cases (some Android 4.0 debug jobs on tbpl).

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/TiledContentClient.cpp?rev=9e5a13343d60#640
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientTiledThebesLayer.cpp?rev=bd59c5ef0677#396

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Explicitly clear the valid region and retained tiles instead of doing an empty paint

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Try push including the above patch at https://tbpl.mozilla.org/?tree=Try&rev=23dc3aa2306e

Comment 3

[Chris Lord [:cwiiis]]

Comment on attachment 8421689 [details] [diff] [review]
Explicitly clear the valid region and retained tiles instead of doing an empty paint

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

LGTM, but BenWa might want to comment on changes to TiledLayerBuffer.

Comment 4

[Benoit Girard (:BenWa)]

Comment on attachment 8421689 [details] [diff] [review]
Explicitly clear the valid region and retained tiles instead of doing an empty paint

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

f+ with the following change.

::: gfx/layers/TiledLayerBuffer.h
@@ +125,4 @@
>    void ClearPaintedRegion() { mPaintedRegion.SetEmpty(); }
>  
> +  void ClearRetainedTiles() {
> +    mRetainedWidth = 0;

ClearRetainedTiles is a public API but if called directly without a ClearValidRegion then this object will be left in an inconsistent state. It will have a valid region but it wont have the tiles for that valid region. This function should include a call to ClearValidRegion().

We should probably also clear mPaintedRegion.

Likewise can you justify ClearValidRegion being a public API on its own? If it's called it will leave the object in an inconsistent state.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I wasn't sure if clearing mValidRegion without clearing mRetainedTiles was actually an inconsistent state. But it certainly isn't useful. You're right, I'll combine all of it into a single function that clears everything.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Explicitly clear the valid region and retained tiles instead of doing an empty paint (v2)

Updated

Comment 7

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/651bb7ad725c

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/651bb7ad725c