Get Mac viewer working with SDL

Get Mac viewer working with SDL

Also fixes SkiaSDLExample.

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

Committed: https://skia.googlesource.com/skia/+/1ba1d372c2727db56021d0687825320b12d10d4e

[jvanverth1, 2016-08-03 20:46:22]

Description was changed from

==========
Get Mac viewer working with SDL

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

to

==========
Get Mac viewer working with SDL

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

[jvanverth1, 2016-08-03 20:47:47]

Description was changed from

==========
Get Mac viewer working with SDL

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

to

==========
Get Mac viewer working with SDL

Also fixes SkiaSDLExample.

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

[jvanverth1, 2016-08-03 20:51:59]

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

[jvanverth1, 2016-08-03 20:52:00]

I'm not 100% happy with this. 

First, the event loop seems to be about as slow as SampleApp is, in comparison
with the GLFW test. Second, the menu is not active on startup -- you have to
switch focus to another app and then back. They've already stated that they
won't fix this bug. 

But it gets us running on the Mac.

[robertphillips, 2016-08-03 21:22:05]

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
File tools/viewer/sk_app/mac/GLWindowContext_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:35: SDL_GLContext fGLContext;
typedef GLWindowContext INHERITED;

?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:38:
GLWindowContext_mac::GLWindowContext_mac(const MacWindowInfo& info, const
DisplayParams& params)
use INHERITED here ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
File tools/viewer/sk_app/mac/Window_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:62: uint32_t windowFlags =
SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE;
overlength ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:77: if (fWindow) {
Shouldn't we be removing the window from gWindowMap here ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:117: switch (event.type) {
// fall through

?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:127: }
I think the "break" is supposed to be inside the '}'

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:130: default:
bracket this guy due to "mod" ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
File tools/viewer/sk_app/mac/Window_mac.h (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.h:19: public:
put the ':' on its own line and line every one up ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.h:44: 
Does this need to be public ?

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/mai...
File tools/viewer/sk_app/mac/main_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/mai...
tools/viewer/sk_app/mac/main_mac.cpp:36: case SDL_KEYUP: {
overlength ?

[jvanverth1, 2016-08-04 15:28:33]

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
File tools/viewer/sk_app/mac/GLWindowContext_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:35: SDL_GLContext fGLContext;
On 2016/08/03 21:22:04, robertphillips wrote:
> typedef GLWindowContext INHERITED;
> 
> ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/GLW...
tools/viewer/sk_app/mac/GLWindowContext_mac.cpp:38:
GLWindowContext_mac::GLWindowContext_mac(const MacWindowInfo& info, const
DisplayParams& params)
On 2016/08/03 21:22:04, robertphillips wrote:
> use INHERITED here ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
File tools/viewer/sk_app/mac/Window_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:62: uint32_t windowFlags =
SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE;
On 2016/08/03 21:22:04, robertphillips wrote:
> overlength ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:77: if (fWindow) {
On 2016/08/03 21:22:04, robertphillips wrote:
> Shouldn't we be removing the window from gWindowMap here ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:117: switch (event.type) {
On 2016/08/03 21:22:04, robertphillips wrote:
> // fall through
> 
> ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:127: }
On 2016/08/03 21:22:04, robertphillips wrote:
> I think the "break" is supposed to be inside the '}'

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.cpp:130: default:
On 2016/08/03 21:22:04, robertphillips wrote:
> bracket this guy due to "mod" ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
File tools/viewer/sk_app/mac/Window_mac.h (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.h:19: public:
On 2016/08/03 21:22:04, robertphillips wrote:
> put the ':' on its own line and line every one up ?

Done.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/Win...
tools/viewer/sk_app/mac/Window_mac.h:44: 
On 2016/08/03 21:22:05, robertphillips wrote:
> Does this need to be public ?

The main loop used it to know which window to send the event to. I replaced that
access with a static wrapper function.

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/mai...
File tools/viewer/sk_app/mac/main_mac.cpp (right):

https://codereview.chromium.org/2210603003/diff/1/tools/viewer/sk_app/mac/mai...
tools/viewer/sk_app/mac/main_mac.cpp:36: case SDL_KEYUP: {
On 2016/08/03 21:22:05, robertphillips wrote:
> overlength ?

Done.

[robertphillips, 2016-08-04 15:33:15]

lgtm

https://codereview.chromium.org/2210603003/diff/40001/tools/viewer/sk_app/mac...
File tools/viewer/sk_app/mac/WindowContextFactory_mac.h (right):

https://codereview.chromium.org/2210603003/diff/40001/tools/viewer/sk_app/mac...
tools/viewer/sk_app/mac/WindowContextFactory_mac.h:11: 
Can we just predeclare SDL_Window ?

[jvanverth1, 2016-08-04 17:14:11]

https://codereview.chromium.org/2210603003/diff/40001/tools/viewer/sk_app/mac...
File tools/viewer/sk_app/mac/WindowContextFactory_mac.h (right):

https://codereview.chromium.org/2210603003/diff/40001/tools/viewer/sk_app/mac...
tools/viewer/sk_app/mac/WindowContextFactory_mac.h:11: 
On 2016/08/04 15:33:15, robertphillips wrote:
> Can we just predeclare SDL_Window ?

I'll get it in a follow-up patch.

[jvanverth1, 2016-08-04 17:14:18]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2016-08-04 17:14:25]

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-08-04 17:16:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-04 17:16:05]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-x86_64-Release-GN-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
  skia_presubmit-Trybot on master.client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[jvanverth1, 2016-08-04 18:35:05]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2016-08-04 18:35:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/2210603003/#ps60001
(title: "Fix patch")

[commit-bot: I haz the power, 2016-08-04 18:35:12]

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-08-04 19:30:33]

Description was changed from

==========
Get Mac viewer working with SDL

Also fixes SkiaSDLExample.

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

to

==========
Get Mac viewer working with SDL

Also fixes SkiaSDLExample.

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

Committed:
https://skia.googlesource.com/skia/+/1ba1d372c2727db56021d0687825320b12d10d4e
==========

[commit-bot: I haz the power, 2016-08-04 19:30:34]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/1ba1d372c2727db56021d0687825320b12d10d4e