Bug 186163 - Add OpenGL display mask to WebPage creation parameters.
Summary: Add OpenGL display mask to WebPage creation parameters.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-31 14:42 PDT by Per Arne Vollan
Modified: 2018-05-31 16:44 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.58 KB, patch)
2018-05-31 14:59 PDT, Per Arne Vollan
bfulgham: review+
Details | Formatted Diff | Diff
Patch (6.87 KB, patch)
2018-05-31 15:35 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (7.08 KB, patch)
2018-05-31 15:54 PDT, Per Arne Vollan
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2018-05-31 14:42:01 PDT
To make sure the OpenGL display mask is always available, include it in the WebPage creation parameters.
Comment 1 Per Arne Vollan 2018-05-31 14:59:59 PDT
Created attachment 341695 [details]
Patch
Comment 2 Per Arne Vollan 2018-05-31 15:00:36 PDT
rdar://problem/40634504
Comment 3 Brent Fulgham 2018-05-31 15:17:20 PDT
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?
Comment 4 Sam Weinig 2018-05-31 15:23:10 PDT
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.
Comment 5 Per Arne Vollan 2018-05-31 15:35:26 PDT
Created attachment 341698 [details]
Patch
Comment 6 Per Arne Vollan 2018-05-31 15:36:52 PDT
(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!
Comment 7 Per Arne Vollan 2018-05-31 15:38:26 PDT
(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!
Comment 8 Sam Weinig 2018-05-31 15:47:56 PDT
(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.
Comment 9 Per Arne Vollan 2018-05-31 15:54:32 PDT
Created attachment 341699 [details]
Patch
Comment 10 Brent Fulgham 2018-05-31 16:02:07 PDT
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.
Comment 11 WebKit Commit Bot 2018-05-31 16:16:38 PDT
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
Comment 12 Simon Fraser (smfr) 2018-05-31 16:25:09 PDT
When I move a window between screens are we doing to update the display mask?
Comment 13 Brent Fulgham 2018-05-31 16:27:09 PDT
(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.
Comment 14 Per Arne Vollan 2018-05-31 16:44:28 PDT
Committed r232375: <https://trac.webkit.org/changeset/232375/webkit>