Last Comment Bug 748645 - Upload TiledThebesLayerOGL outside the transaction
: Upload TiledThebesLayerOGL outside the transaction
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
-- normal (vote)
: mozilla15
Assigned To: Benoit Girard (:BenWa)
: Milan Sreckovic [:milan]
Depends on:
  Show dependency treegraph
Reported: 2012-04-24 20:10 PDT by Benoit Girard (:BenWa)
Modified: 2012-04-26 10:35 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (696 bytes, patch)
2012-04-24 20:10 PDT, Benoit Girard (:BenWa) review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Benoit Girard (:BenWa) 2012-04-24 20:10:19 PDT
Created attachment 618150 [details] [diff] [review]

This is causing some regression since we're doing this upload while content is blocked. I wanted to let TiledThebesLayerOGL bake for a day with uploads during the transaction. Note we have another ProcessUploadQueue() in 'TiledThebesLayerOGL::RenderLayer'.
Comment 1 User image Chris Lord [:cwiiis] 2012-04-25 08:16:31 PDT
Comment on attachment 618150 [details] [diff] [review]

Review of attachment 618150 [details] [diff] [review]:

Looks good to me. Is there a possibility that content could start reusing/destroying tiles before the compositor gets to process them?

::: gfx/layers/opengl/TiledThebesLayerOGL.cpp
@@ +119,5 @@
>  TiledThebesLayerOGL::PaintedTiledLayerBuffer(const BasicTiledLayerBuffer* mTiledBuffer)
>  {
>    mMainMemoryTiledBuffer = *mTiledBuffer;
>    mRegionToUpload.Or(mRegionToUpload, mMainMemoryTiledBuffer.GetLastPaintRegion());

And maybe get rid of this blank line too?
Comment 2 User image Benoit Girard (:BenWa) 2012-04-25 08:31:20 PDT
It is, however the code should copy out the tiles because the compositor will still have a read lock. If that doesn't work then there's a bug with that part of the tiling work and well have to fix it.
Comment 4 User image Benoit Girard (:BenWa) 2012-04-25 10:33:41 PDT
Comment on attachment 618150 [details] [diff] [review]

[Approval Request Comment]
User impact if declined: Checkerboard reduction
Testing completed (on m-c, etc.): local testing, m-c testing
Risk to taking this patch (and alternatives if risky): Small risk of regressing correctness.
String changes made by this patch: None
Comment 5 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-04-25 12:44:45 PDT
Comment on attachment 618150 [details] [diff] [review]

[Triage Comment]
Approving, Fennec beta blocker.
Comment 6 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-04-25 20:34:06 PDT
Comment 7 User image Brad Lassey [:blassey] (use needinfo?) 2012-04-26 10:01:55 PDT

Note You need to log in before you can comment on or make changes to this bug.