RESOLVED FIXED 90031
[chromium] Should schedule a commit when dropping contents textures
https://bugs.webkit.org/show_bug.cgi?id=90031
Summary [chromium] Should schedule a commit when dropping contents textures
James Robinson
Reported 2012-06-26 17:56:16 PDT
[chromium] Should schedule a commit when dropping contents textures
Attachments
Patch (5.12 KB, patch)
2012-06-26 17:57 PDT, James Robinson
no flags
James Robinson
Comment 1 2012-06-26 17:57:57 PDT
Adrienne Walker
Comment 2 2012-06-28 09:36:01 PDT
Can you explain more why this commit needs to be here and why the proxies behave differently?
James Robinson
Comment 3 2012-06-28 10:39:35 PDT
Without this commit we have a bit of stale state in CCLayerTreeHostImpl::canDraw(), which is used only by the thread scheduler. In single threaded mode, render_widget forces us to make a frame no matter what our state is so we do a commit regardless.
Dana Jansens
Comment 4 2012-06-28 10:43:01 PDT
FWIW CCSingleThreadProxy::doComposite does use canDraw as well.
James Robinson
Comment 5 2012-06-28 10:43:32 PDT
Oh! So it does...interesting
James Robinson
Comment 6 2012-06-28 10:58:40 PDT
(In reply to comment #4) > FWIW CCSingleThreadProxy::doComposite does use canDraw as well. In CCSingleThreadProxy commitAndComposite() does an unconditional doCommit(), then calls doComposite() which checks canDraw. Actually though I was wrong that this is why the proxies behave differently for this patch. If we did a commit in threaded mode we would reset the canDraw state as well. In single threaded mode, we get a call to commitAndComposite() from render_widget whenever we become visible. In threaded mode, the scheduler won't schedule a commit unless something tells it to. That's what this patch does.
Adrienne Walker
Comment 7 2012-06-28 11:27:22 PDT
(In reply to comment #6) > (In reply to comment #4) > > FWIW CCSingleThreadProxy::doComposite does use canDraw as well. > > In CCSingleThreadProxy commitAndComposite() does an unconditional doCommit(), then calls doComposite() which checks canDraw. > > Actually though I was wrong that this is why the proxies behave differently for this patch. If we did a commit in threaded mode we would reset the canDraw state as well. In single threaded mode, we get a call to commitAndComposite() from render_widget whenever we become visible. In threaded mode, the scheduler won't schedule a commit unless something tells it to. That's what this patch does. Should we do something parallel here, like calling setNeedsCommit() when we become visible? Maybe I just want more symmetry, and it seems a little weird to have this extra commit to notify the main thread that the contents textures earlier so we can clear a flag.
James Robinson
Comment 8 2012-06-28 11:29:16 PDT
It's not quite just that, we actually do need to do a commit and paint, upload, etc if we've dropped contents textures. I don't think we want to do setNeedsCommit() unconditionally when becoming visible - if we become visible but we haven't evicted anything (say you're switching back and forth rapidly between two tabs), then we should just put a frame up. There's no need for a commit in that case, unless something else has changed.
Adrienne Walker
Comment 9 2012-06-28 12:12:26 PDT
(In reply to comment #8) > It's not quite just that, we actually do need to do a commit and paint, upload, etc if we've dropped contents textures. > > I don't think we want to do setNeedsCommit() unconditionally when becoming visible - if we become visible but we haven't evicted anything (say you're switching back and forth rapidly between two tabs), then we should just put a frame up. There's no need for a commit in that case, unless something else has changed. Ah, ok. The piece I was missing was that we also don't commit when not visible, so this commit gets saved for when we need it and come visible again. Sounds good.
Adrienne Walker
Comment 10 2012-06-28 12:12:39 PDT
Comment on attachment 149649 [details] Patch R=me.
James Robinson
Comment 11 2012-06-28 12:20:27 PDT
Comment on attachment 149649 [details] Patch Thanks!
WebKit Review Bot
Comment 12 2012-06-28 12:28:02 PDT
Comment on attachment 149649 [details] Patch Clearing flags on attachment: 149649 Committed r121450: <http://trac.webkit.org/changeset/121450>
WebKit Review Bot
Comment 13 2012-06-28 12:28:07 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.