Bug 186163 – Add OpenGL display mask to WebPage creation parameters.

• Component: WebKit2
• Status: RESOLVED FIXED

[Per Arne Vollan]

To make sure the OpenGL display mask is always available, include it in the WebPage creation parameters.

[Per Arne Vollan]

Created attachment 341695 [details]
Patch

[Per Arne Vollan]

rdar://problem/40634504

[Brent Fulgham]

Comment on attachment 341695 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/WebPageProxy.cpp:6151
> +#endif

Is there a startup message that had previously been used to send the display mask that can now be removed?

[Sam Weinig]

Comment on attachment 341695 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:623
> +#if PLATFORM(IOS)
> +    GraphicsContext3D::setOpenGLDisplayMask(parameters.displayMask);
> +#endif

I don't this this will work with two WKWebViews in the same web process on different displays. This will just set the the global display mask two different values.

[Per Arne Vollan]

Created attachment 341698 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 341695 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341695&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:6151
> > +#endif
> 
> Is there a startup message that had previously been used to send the display
> mask that can now be removed?

The message is sent when the platform display ID is changing, which we still need to do.

Thanks for reviewing!

[Per Arne Vollan]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 341695 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341695&action=review
> 
> > Source/WebKit/WebProcess/WebPage/WebPage.cpp:623
> > +#if PLATFORM(IOS)
> > +    GraphicsContext3D::setOpenGLDisplayMask(parameters.displayMask);
> > +#endif
> 
> I don't this this will work with two WKWebViews in the same web process on
> different displays. This will just set the the global display mask two
> different values.

That makes sense. I think the OpenGL display mask could be moved to the Chrome class. Are you ok with that being done in a follow-up patch?

Thanks for reviewing!

[Sam Weinig]

(In reply to Per Arne Vollan from comment #7)
> (In reply to Sam Weinig from comment #4)
> > Comment on attachment 341695 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=341695&action=review
> > 
> > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:623
> > > +#if PLATFORM(IOS)
> > > +    GraphicsContext3D::setOpenGLDisplayMask(parameters.displayMask);
> > > +#endif
> > 
> > I don't this this will work with two WKWebViews in the same web process on
> > different displays. This will just set the the global display mask two
> > different values.
> 
> That makes sense. I think the OpenGL display mask could be moved to the
> Chrome class. Are you ok with that being done in a follow-up patch?
> 
> Thanks for reviewing!

Ok. But given that bug, it seems like you should find a way to test this.

[Per Arne Vollan]

Created attachment 341699 [details]
Patch

[Brent Fulgham]

Comment on attachment 341699 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:623
> +    GraphicsContext3D::setOpenGLDisplayMask(parameters.displayMask);

For future: Might need this take the display mask as well as the displayID used by this page.

[WebKit Commit Bot]

Comment on attachment 341699 [details]
Patch

Rejecting attachment 341699 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 341699, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/7909090

[Simon Fraser (smfr)]

When I move a window between screens are we doing to update the display mask?

[Brent Fulgham]

(In reply to Simon Fraser (smfr) from comment #12)
> When I move a window between screens are we doing to update the display mask?

yes. That part was already in place and working.

This change is only about a startup problem we found during testing.

[Per Arne Vollan]

Committed r232375: <https://trac.webkit.org/changeset/232375/webkit>