WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
88940
DrawingArea: Painting is being resumed while the view is not visible
https://bugs.webkit.org/show_bug.cgi?id=88940
Summary
DrawingArea: Painting is being resumed while the view is not visible
Tim Horton
Reported
2012-06-12 17:22:25 PDT
From my ChangeLog: Using requestAnimationFrame and the fullscreen API on a DrawingArea-backed window would cause rAF to permanently suspend animations when fullscreened, because of the following: 0. JavaScript causes fullscreen transition to start. 1. Painting (and rAF) are suspended. 2. The page changes size. a. DrawingAreaProxyImpl::sizeDidChange() calls DrawingAreaImpl::updateBackingStoreState, which calls DrawingAreaImpl::resumePainting. b. DrawingAreaImpl::resumePainting resumes painting, but does *not* resume rAF, because windowIsVisible is (legitimately) false. 3. The view becomes visible, windowIsVisible is updated to true. 4. visibilityDidChange() calls resumePainting again, but this time it early exits because painting is not suspended. Notice that because of the early exit in 4, rAF is never resumed. I have a fix: If DrawingAreaProxyImpl::sizeDidChange or DrawingAreaProxyImpl::deviceScaleFactorDidChange are called while the view is not visible, defer their call to update the backing store state until the view is once again made visible. <
rdar://problem/11587039
>
Attachments
patch, but needs more testing before landing
(5.16 KB, patch)
2012-06-12 17:45 PDT
,
Tim Horton
no flags
Details
Formatted Diff
Diff
patch (disable suspend/resumePainting messages when doing fullscreen animation)
(7.72 KB, patch)
2012-06-13 15:53 PDT
,
Tim Horton
dino
: review-
Details
Formatted Diff
Diff
patch, name is no better, changelog is improved
(9.15 KB, patch)
2012-06-14 16:20 PDT
,
Tim Horton
dino
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Tim Horton
Comment 1
2012-06-12 17:32:53 PDT
The radar is actually <
rdar://problem/11652545
>
Tim Horton
Comment 2
2012-06-12 17:45:24 PDT
Created
attachment 147201
[details]
patch, but needs more testing before landing
Tim Horton
Comment 3
2012-06-13 15:53:42 PDT
Created
attachment 147433
[details]
patch (disable suspend/resumePainting messages when doing fullscreen animation) The old patch didn't work, because we actually depend on painting *not* being frozen to paint the image that's animated to full-screen! So, it caused the animation to look terrible. This one is a bit more wonky/scary, but I haven't been able to break it yet. Still needs more testing.
Jer Noble
Comment 4
2012-06-14 13:39:20 PDT
Looks good to me. Informal r+.
Dean Jackson
Comment 5
2012-06-14 13:48:46 PDT
Comment on
attachment 147433
[details]
patch (disable suspend/resumePainting messages when doing fullscreen animation) View in context:
https://bugs.webkit.org/attachment.cgi?id=147433&action=review
> Source/WebKit2/ChangeLog:9 > + Don't send any SuspendPainting/ResumePainting messages during full-screen animations on Mac.
Could you add more explanation here? Why was this a problem, and what are you doing now (transitions to and from fullscreen can continue to paint, etc).
> Source/WebKit2/UIProcess/API/mac/WKView.h:47 > +@property BOOL automaticallySuspendAndResumePainting;
I don't like the name of this property. Also, it's probably a bit backwards isn't it? You are now adding the functionality to NOT automatically suspend painting. Maybe it should be just called suspendsPaintingDuringWindowAnimations or something (that's a bad name too)
Tim Horton
Comment 6
2012-06-14 15:18:43 PDT
(In reply to
comment #5
)
> (From update of
attachment 147433
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=147433&action=review
> > > Source/WebKit2/ChangeLog:9 > > + Don't send any SuspendPainting/ResumePainting messages during full-screen animations on Mac. > > Could you add more explanation here? Why was this a problem, and what are you doing now (transitions to and from fullscreen can continue to paint, etc). > > > Source/WebKit2/UIProcess/API/mac/WKView.h:47 > > +@property BOOL automaticallySuspendAndResumePainting; > > I don't like the name of this property. Also, it's probably a bit backwards isn't it? You are now adding the functionality to NOT automatically suspend painting. Maybe it should be just called suspendsPaintingDuringWindowAnimations or something (that's a bad name too)
"DuringWindowAnimations" is irrelevant, WKView has no idea that's why this is being used, just that it should stop sending suspend/resume messages.
Tim Horton
Comment 7
2012-06-14 15:18:53 PDT
I still don't have a good name.
Tim Horton
Comment 8
2012-06-14 16:20:11 PDT
Created
attachment 147676
[details]
patch, name is no better, changelog is improved
Dean Jackson
Comment 9
2012-06-14 16:32:32 PDT
Comment on
attachment 147676
[details]
patch, name is no better, changelog is improved View in context:
https://bugs.webkit.org/attachment.cgi?id=147676&action=review
Great changelog. Still don't like the name. Maybe suspendPaintingWhileWindowHostingChanges ? Marking r+ and we can work out the name on irc.
> Source/WebKit2/ChangeLog:10 > + would cause rAF to permanently suspend animations when fullscreened, because of the following:
I would invent a word like "fullscreened" in my own patch, but in review I must strangely request something like "when the page is made fullscreen" :)
Tim Horton
Comment 10
2012-06-14 17:04:26 PDT
http://trac.webkit.org/changeset/120376
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