WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
James Robinson
Comment 1
2012-06-26 17:57:57 PDT
Created
attachment 149649
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug