Use Windowing system-specific WindowContext factories.

Use Windowing system-specific WindowContext factories.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002

Committed: https://skia.googlesource.com/skia/+/d1bdd1fcbd308afb9903f39d231742f5c951cf07

[bsalomon, 2016-07-20 16:51:28]

Description was changed from

==========
Working on X

BUG=skia:
==========

to

==========
Working on X

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002
==========

[bsalomon, 2016-07-20 17:05:06]

bsalomon@google.com changed reviewers:
+ jvanverth@google.com

[bsalomon, 2016-07-20 17:05:06]

This is the direction I was thinking about going with WindowContext creation. So
far this only works on X (and probably fails to compile on other platforms).
What you think of this idea?

https://codereview.chromium.org/2169543002/diff/20001/tools/viewer/sk_app/uni...
File tools/viewer/sk_app/unix/WindowContextFactory_unix.h (right):

https://codereview.chromium.org/2169543002/diff/20001/tools/viewer/sk_app/uni...
tools/viewer/sk_app/unix/WindowContextFactory_unix.h:17: namespace
window_context_factory {
There would be equivalent files for other windowing backends, e.g.
WindowContextFactory_win.h

Once raster support has landed I was thinking there would be a helper
NewForXWindows(BackendType, XWindowsInfo)

[bsalomon, 2016-07-21 20:52:52]

This now works on Android, Linux, and Windows. I saw there are some Mac files
checked in. Should I look at modifying those or are they not ready yet?

[bsalomon, 2016-07-21 20:53:16]

Description was changed from

==========
Working on X

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002
==========

to

==========
Use Windowing system-specific WindowContext factories.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002
==========

[jvanverth1, 2016-07-22 14:18:24]

On 2016/07/21 20:52:52, bsalomon wrote:
> This now works on Android, Linux, and Windows. I saw there are some Mac files
> checked in. Should I look at modifying those or are they not ready yet?

Yeah, they don't do anything (just there to make sure Viewer builds on the bots)
but I suspect you'll have to update the interfaces. Feel free to rip out what
you need to -- it's based on the old Unix code and I left a good part of that in
there, but that was only so I could remember the point of each method.

[bsalomon, 2016-07-25 15:04:01]

Ok this now has changes to continue building on Mac and is ready for review.

[jvanverth1, 2016-07-26 17:54:40]

Some minor stuff:

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
File tools/viewer/sk_app/android/GLWindowContext_android.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
tools/viewer/sk_app/android/GLWindowContext_android.cpp:18: namespace {
Do we need a namespace here?

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
File tools/viewer/sk_app/android/VulkanWindowContext_android.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
tools/viewer/sk_app/android/VulkanWindowContext_android.cpp:21:
"vkCreateAndroidSurfaceKHR");
100 chars?

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/ma...
File tools/viewer/sk_app/mac/GLWindowContext_mac.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/ma...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:18: namespace {
Is there a reason to not put this in sk_app? Then you don't have to use 'using',
above.

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
File tools/viewer/sk_app/unix/GLWindowContext_unix.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
tools/viewer/sk_app/unix/GLWindowContext_unix.cpp:106: WindowContext*
NewGLForXWindows(const XWindowsInfo& winInfo, const DisplayParams& params) {
Xlib rather than XWindows? (could be XCB or Mir or the like)

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
File tools/viewer/sk_app/unix/VulkanWindowContext_unix.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
tools/viewer/sk_app/unix/VulkanWindowContext_unix.cpp:22: WindowContext*
NewVulkanForXWindows(const XWindowsInfo& info, const DisplayParams&
displayParams) {
Xlib rather than XWindows?

[bsalomon, 2016-07-26 18:20:51]

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
File tools/viewer/sk_app/android/GLWindowContext_android.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
tools/viewer/sk_app/android/GLWindowContext_android.cpp:18: namespace {
On 2016/07/26 17:54:40, jvanverth1 wrote:
> Do we need a namespace here?

I just wanted to avoid polluting the global namespace with a class that only
needs to be named in this file and doesn't need external linkage.

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
File tools/viewer/sk_app/android/VulkanWindowContext_android.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/an...
tools/viewer/sk_app/android/VulkanWindowContext_android.cpp:21:
"vkCreateAndroidSurfaceKHR");
On 2016/07/26 17:54:40, jvanverth1 wrote:
> 100 chars?

Done.

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/ma...
File tools/viewer/sk_app/mac/GLWindowContext_mac.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/ma...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:18: namespace {
On 2016/07/26 17:54:40, jvanverth1 wrote:
> Is there a reason to not put this in sk_app? Then you don't have to use
'using',
> above.

The type is local to this file, it doesn't need external linkage.

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
File tools/viewer/sk_app/unix/GLWindowContext_unix.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
tools/viewer/sk_app/unix/GLWindowContext_unix.cpp:106: WindowContext*
NewGLForXWindows(const XWindowsInfo& winInfo, const DisplayParams& params) {
On 2016/07/26 17:54:40, jvanverth1 wrote:
> Xlib rather than XWindows? (could be XCB or Mir or the like)

Done.

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
File tools/viewer/sk_app/unix/VulkanWindowContext_unix.cpp (right):

https://codereview.chromium.org/2169543002/diff/120001/tools/viewer/sk_app/un...
tools/viewer/sk_app/unix/VulkanWindowContext_unix.cpp:22: WindowContext*
NewVulkanForXWindows(const XWindowsInfo& info, const DisplayParams&
displayParams) {
On 2016/07/26 17:54:40, jvanverth1 wrote:
> Xlib rather than XWindows?

Done.

[jvanverth1, 2016-07-26 18:34:42]

lgtm

[bsalomon, 2016-07-26 18:35:48]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2016-07-26 18:35:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 19:02:53]

Description was changed from

==========
Use Windowing system-specific WindowContext factories.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002
==========

to

==========
Use Windowing system-specific WindowContext factories.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169543002

Committed:
https://skia.googlesource.com/skia/+/d1bdd1fcbd308afb9903f39d231742f5c951cf07
==========

[commit-bot: I haz the power, 2016-07-26 19:02:53]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/d1bdd1fcbd308afb9903f39d231742f5c951cf07

[tomhudson, 2016-07-27 12:22:03]

This CL apparently broke the Android framework.