STR: 1. Run firefox -chrome chrome://global/content/test-ipcbrowser.xul 2. Go to http://google.com/search?q=test 3. Set displayport to 0, 0, 600, 600 4. Set a breakpoint at the end of ThebesLayerBuffer::BeginPaint 5. Set displayport to 0, 20, 600, 600 For step 5, result.mRegionToDraw is entire displayport when it only needs to be the bottom strip, since most of the pixels are the same.

(this is on Linux desktop)

google.com is actually a fairly complicated page nowadays. If you can repro this on just a tall page of text, it would help simplify analysis. If this is a build with --enable-mobile-optimize (default for mobile, custom mozconfig setting on desktop), then this isn't unexpected because we can't self-copy image surfaces. If this isn't an --enable-mobile-optimize build, then the things to check are - retained layers aren't being tossed out. If they are, you'll see a NS_WARNING("Flushing retained layers!") message. - the "right" layers are retained. Check the layer tree before/after the paint that makes the too-large redraw. If the layer pointers have changed, then the layers are perhaps being recreated unexpectedly.

There are other candidates, like a too-large invalidate coming from somewhere, but the ones above are most likely and easier to diagnose.

OK, using http://people.mozilla.org/~bstover/font.html now. I commented out the SupportsSelfCopy check (that would be equivalent?). mRegionToDraw is still entire displayport. In fact, when we calculate draw bounds: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ThebesLayerBuffer.cpp#210 It is the entire displayport, so I'm assuming this is because our valid region is completely empty. This makes sense to me, since we invalidate everything every time we call displayport. Am I missing something?

See what the layer tree says. Try running with MOZ_DUMP_PAINT_LIST=1.

Comment on attachment 512607 [details] [diff] [review] Setting displayport redraws entire displayport Something like this seems to do the trick for me. Unforuntately, some sites like news.google.com have some corruption issues (you can see this as well when you call content.scrollTo instead of setting displayport, so it doesn't have anything to do with this patch).

Comment on attachment 512607 [details] [diff] [review] Setting displayport redraws entire displayport Nice! I /think/ this is OK, but you would need real review from a layout person (obviously ;) ).

(Oops, didn't mean to reset assignee.)

Comment on attachment 512607 [details] [diff] [review] Setting displayport redraws entire displayport >+ rootFrame->InvalidateInternal(rootFrame->GetVisualOverflowRectRelativeToSelf() + rootFrame->GetPosition(), >+ 0, 0, rootFrame, nsIFrame::INVALIDATE_NO_THEBES_LAYERS); This seems ok. Use InvalidateWithFlags instead. And the damage rect is relative to the frame itself, not its parent, so get rid of rootFrame->GetPosition().

Changing dependencies due to changing the meaning of bug 634759. This bug can be checked in once ThebesLayerBuffer renders accurately and we fix the assertion that causes corruption.

Comment on attachment 514925 [details] [diff] [review] Setting displayport redraws entire displayport what are the dependencies that are needed for this to land. The bugs that block this one are marked as soft blockers and have been denied approval to land.

Re-nom'ing for 4.x based on bug 637852. We could take this and bug 635035 for 4.0 if we can live with the seaming, but I don't think that's a good idea.

Perhaps we could add a redraw call at the very end of a scroll to Fennec and include this patch? As I understand we really want this for 4.0.

We have to get this in to speed up scrolling, reduce checkerboarding, and remove text jiggling

(In reply to comment #16) > Perhaps we could add a redraw call at the very end of a scroll to Fennec and > include this patch? Can you elaborate a bit?

Sure. We don't care *that* much about how precise the pixels look while we are panning, but when the pan is finished we expect the results to be correct. Fennec could ask for a redraw when a pan is finished, clearing up any inconsistencies that have been introduced by only doing partial invalidations.

I see. Sounds worth trying.

Does bug 633344 block this?

I don't think so.

No longer depends on 635373 because appropriate patches have landed.

Comment on attachment 517895 [details] [diff] [review] Mobile workaround: redraw after pan is finished Looks good to me.

Pushed mobile: http://hg.mozilla.org/mobile-browser/rev/e3b54fa2e3c6 m-c: http://hg.mozilla.org/mozilla-central/rev/71c15b2de18e