Bug 210297 – First render after a process swap does not use accelerated rendering

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

First render after a process swap does not use accelerated rendering.

[Chris Dumez]

<rdar://problem/61432515>

[Chris Dumez]

Created attachment 395993 [details]
Proof of concept fix

I have a proof of concept that seems to be the issue (would need some cleanup though). I am uploading the gather early feedback.

[Chris Dumez]

Created attachment 395997 [details]
Proof of concept fix

[Chris Dumez]

Created attachment 395998 [details]
Proof of concept fix

[Chris Dumez]

Created attachment 395999 [details]
Proof of concept fix

[Chris Dumez]

Created attachment 396004 [details]
WIP Patch

[Chris Dumez]

Created attachment 396006 [details]
WIP Patch

Ok, I did a decent amount of cleanup and it appears to be working. Any early feedback?

[Chris Dumez]

Created attachment 396010 [details]
Patch

[Geoffrey Garen]

Are the EWS crashes in viewgesturecontroller real?

[Antti Koivisto]

Comment on attachment 396010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396010&action=review

Good idea!

> Source/WebKit/ChangeLog:26
> +        2. In the UIProcess, when we get the DrawingAreaProxy::EnterAcceleratedCompositingMode
> +           IPC and we already have a root layer, we add the new one behind the existing one
> +           instead of replacing the existing one. As a result, the new layer will get
> +           accelerated compositing but will not be visible on screen yet.

I wonder if there are cases with transparent layers or missing tiles where this could lead to both pages being partially visible at the same time.

Maybe a stronger hiding strategy is needed?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #9)
> Are the EWS crashes in viewgesturecontroller real?

EWS is green now so I guess not.

[Chris Dumez]

(In reply to Antti Koivisto from comment #10)
> Comment on attachment 396010 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396010&action=review
> 
> Good idea!
> 
> > Source/WebKit/ChangeLog:26
> > +        2. In the UIProcess, when we get the DrawingAreaProxy::EnterAcceleratedCompositingMode
> > +           IPC and we already have a root layer, we add the new one behind the existing one
> > +           instead of replacing the existing one. As a result, the new layer will get
> > +           accelerated compositing but will not be visible on screen yet.
> 
> I wonder if there are cases with transparent layers or missing tiles where
> this could lead to both pages being partially visible at the same time.
> 
> Maybe a stronger hiding strategy is needed?

Tim / Simon, what do you think? Can I land as is or do you think we need a better hiding strategy?

[Simon Fraser (smfr)]

(In reply to Chris Dumez from comment #12)
> (In reply to Antti Koivisto from comment #10)
> > Comment on attachment 396010 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=396010&action=review
> > 
> > Good idea!
> > 
> > > Source/WebKit/ChangeLog:26
> > > +        2. In the UIProcess, when we get the DrawingAreaProxy::EnterAcceleratedCompositingMode
> > > +           IPC and we already have a root layer, we add the new one behind the existing one
> > > +           instead of replacing the existing one. As a result, the new layer will get
> > > +           accelerated compositing but will not be visible on screen yet.
> > 
> > I wonder if there are cases with transparent layers or missing tiles where
> > this could lead to both pages being partially visible at the same time.
> > 
> > Maybe a stronger hiding strategy is needed?
> 
> Tim / Simon, what do you think? Can I land as is or do you think we need a
> better hiding strategy?

We do. WKWebViews can be transparent, which allows transparent page contents, and this hack would make your hidden views visible.

Maybe move them offscreen? Or set layer.hidden (checking to see whether that cuts of painting, which we don't want).

[Simon Fraser (smfr)]

There's a menu item in MiniBrowser to toggle on transparent web views. Navigate from a page with no root background color; you shouldn't see the new page behind the old one.

Ideally this would be testable.

[Ben Nham]

Created attachment 396111 [details]
test case for drawInContext: on hidden layer

Based on code inspection of CALayer.mm and this attached test case, it seems like -drawInContext: still gets called on a layer even if it's hidden. But we could ask the CA maintainer to be sure.

[Chris Dumez]

(In reply to Ben Nham from comment #15)
> Created attachment 396111 [details]
> test case for drawInContext: on hidden layer
> 
> Based on code inspection of CALayer.mm and this attached test case, it seems
> like -drawInContext: still gets called on a layer even if it's hidden. But
> we could ask the CA maintainer to be sure.

Yes, I have also verified in Safari that marking the layer as hidden does not prevent painting or hardware acceleration.

[Chris Dumez]

Created attachment 396112 [details]
Patch

[EWS]

Committed r259898: <https://trac.webkit.org/changeset/259898>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396112 [details].