First pass at VulkanViewer

First pass at VulkanViewer

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1848833005

Committed: https://skia.googlesource.com/skia/+/9f37246d6856d40f916e8f1fff5b43cfdc42e567

[jvanverth1, 2016-04-01 20:29:59]

Description was changed from

==========
First pass at VulkanViewer

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

to

==========
First pass at VulkanViewer

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[jvanverth1, 2016-04-04 20:15:39]

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

[jvanverth1, 2016-04-04 20:15:40]

So here's my mostly working code for the viewer. There's still a fair amount I
want to clean up (it still doesn't display GMs or SKPs, for example), but it's
mostly there. PTAL and let me know how it looks so far.

[bsalomon, 2016-04-05 14:24:44]

Looks like a nice start!

https://codereview.chromium.org/1848833005/diff/60001/include/gpu/vk/GrVkBack...
File include/gpu/vk/GrVkBackendContext.h (left):

https://codereview.chromium.org/1848833005/diff/60001/include/gpu/vk/GrVkBack...
include/gpu/vk/GrVkBackendContext.h:52: SkAutoTUnref<const GrVkInterface>
fInterface;
was this accidental? I miss the space.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
File tools/vulkan/InputHandler.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:23: bool isKeyDown(unsigned char key) {
should the getters be const?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:37: bool isMouseDown(unsigned int& h, unsigned int&
v) {
To be consistent with our style h and v should be pointers not refs.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:47: bool isMousePressed(unsigned int& h, unsigned
int& v) {
ditto

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:13: #include "../../src/gpu/vk/GrVkMemory.h"
You could probably update the gyp file to have src/gpu be on the include path so
that you could just say vk/GrVkMemory.h. Then if we move this file to a new
location in the future you don't have to update the ..s

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:377: delete[] fSurfaces;   //*** will this
unref properly?
I believe it will since it will call ~sk_skp

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:19: class VulkanTestContext {
Is this class specifically handling the window-backed case? Seems centered
around having a real back buffer, presenting, etc. Wondering if it should be
VulkanWindowContext or something. Or do you imagine using it for offscreen
contexts as well?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanView...
File tools/vulkan/VulkanViewer.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanView...
tools/vulkan/VulkanViewer.cpp:2: * Copyright 2016 Google Inc.
Would it make since to put everything related to the viewer in it's own dir
(e.g. tools/vulkan/viewer) to make it really clear what is framework and what is
app-specific?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h
File tools/vulkan/Window.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:33: virtual bool attach(BackEndTypes attachType, int
msaaSampleCount, AttachmentInfo*) = 0;
Is the window unusable until attach is called?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:34: void release();
Not totally obvious that this is related to attach until seeing the impl. Maybe
detach?

[egdaniel, 2016-04-05 16:50:28]

https://codereview.chromium.org/1848833005/diff/60001/src/gpu/vk/GrVkMemory.cpp
File src/gpu/vk/GrVkMemory.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/src/gpu/vk/GrVkMemory.c...
src/gpu/vk/GrVkMemory.cpp:121: } else if (VK_IMAGE_LAYOUT_PRESENT_SRC_KHR ==
layout) {
We should probably comment on why this is the stage we are waiting at (even if
it really just arbitrary...) since it definitely isn't intuitive.

Actually I wonder if we actually even want to put these in here. Inside of
Ganesh we are neverusing LAYOUT_PRESENT, and the few places that do in our
windowing code can just hard code. Especially since when we use this stage as a
srcStageMask, it is actually being chosen because of a connected semaphore.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:71: SkASSERT(fPresentQueueIndex <
queueCount);
do we want to assert that QueueIndex != -1 or >= 0?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:74: VkResult res =
GR_VK_CALL(fBackendContext->fInterface,
this line is very hard to read what is happening. Maybe move the GR_VK_CALL down
a line and things might fit/align better?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:102:
GetPhysicalDeviceSurfaceCapabilitiesKHR(fBackendContext->fPhysicalDevice,
100 chars

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:113: &surfaceFormatCount,
is it possible for this to return 0 surfaceFormatCount here? if it does will
that hurt anything later on?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:123: fSurface,
align these with fBackendContext

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:133: fSurface,
align

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:134: &presentModeCount,
again question on is a count of 0 bad?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:144: fSurface,
align

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:174: //if (ctx_.extent.width == extent.width
&& ctx_.extent.height == extent.height)
what is ctx referring to in this call?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:184: //VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
why are these commented out?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:189: VK_COMPOSITE_ALPHA_INHERIT_BIT_KHR));
align

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:210: swapchainCreateInfo.imageFormat =
surfaceFormats[0].format;       // for now, use the first one
is this something we'll search through at some point to find an sRGB surface, or
linear, or single channel, etc. if we want that? Any idea what the first format
generally is

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:325: fBackbuffers = new
BackbufferInfo[fImageCount + 1];
comment on why the +1 here?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:8: #ifndef VULKANTESTCONTEXT_DEFINED
we don't usually capitalize every letter in the class name here

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:71: SkAutoTUnref<const GrVkBackendContext>
fBackendContext;
use sk_sp?

https://codereview.chromium.org/1848833005/diff/80001/tools/vulkan/win/Vulkan...
File tools/vulkan/win/VulkanTestContext_win.cpp (right):

https://codereview.chromium.org/1848833005/diff/80001/tools/vulkan/win/Vulkan...
tools/vulkan/win/VulkanTestContext_win.cpp:32: nullptr, &surface));
align this last line with fBackendContext?

[jvanverth1, 2016-04-05 18:16:49]

https://codereview.chromium.org/1848833005/diff/60001/include/gpu/vk/GrVkBack...
File include/gpu/vk/GrVkBackendContext.h (left):

https://codereview.chromium.org/1848833005/diff/60001/include/gpu/vk/GrVkBack...
include/gpu/vk/GrVkBackendContext.h:52: SkAutoTUnref<const GrVkInterface>
fInterface;
On 2016/04/05 14:24:43, bsalomon wrote:
> was this accidental? I miss the space.

Done.

https://codereview.chromium.org/1848833005/diff/60001/src/gpu/vk/GrVkMemory.cpp
File src/gpu/vk/GrVkMemory.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/src/gpu/vk/GrVkMemory.c...
src/gpu/vk/GrVkMemory.cpp:121: } else if (VK_IMAGE_LAYOUT_PRESENT_SRC_KHR ==
layout) {
On 2016/04/05 16:50:28, egdaniel wrote:
> We should probably comment on why this is the stage we are waiting at (even if
> it really just arbitrary...) since it definitely isn't intuitive.
> 
> Actually I wonder if we actually even want to put these in here. Inside of
> Ganesh we are neverusing LAYOUT_PRESENT, and the few places that do in our
> windowing code can just hard code. Especially since when we use this stage as
a
> srcStageMask, it is actually being chosen because of a connected semaphore.

Removed.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
File tools/vulkan/InputHandler.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:23: bool isKeyDown(unsigned char key) {
On 2016/04/05 14:24:43, bsalomon wrote:
> should the getters be const?

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:37: bool isMouseDown(unsigned int& h, unsigned int&
v) {
On 2016/04/05 14:24:43, bsalomon wrote:
> To be consistent with our style h and v should be pointers not refs.

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/InputHandl...
tools/vulkan/InputHandler.h:47: bool isMousePressed(unsigned int& h, unsigned
int& v) {
On 2016/04/05 14:24:43, bsalomon wrote:
> ditto

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:13: #include "../../src/gpu/vk/GrVkMemory.h"
On 2016/04/05 14:24:43, bsalomon wrote:
> You could probably update the gyp file to have src/gpu be on the include path
so
> that you could just say vk/GrVkMemory.h. Then if we move this file to a new
> location in the future you don't have to update the ..s

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:71: SkASSERT(fPresentQueueIndex <
queueCount);
On 2016/04/05 16:50:28, egdaniel wrote:
> do we want to assert that QueueIndex != -1 or >= 0?

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:74: VkResult res =
GR_VK_CALL(fBackendContext->fInterface,
On 2016/04/05 16:50:28, egdaniel wrote:
> this line is very hard to read what is happening. Maybe move the GR_VK_CALL
down
> a line and things might fit/align better?

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:102:
GetPhysicalDeviceSurfaceCapabilitiesKHR(fBackendContext->fPhysicalDevice,
On 2016/04/05 16:50:28, egdaniel wrote:
> 100 chars

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:113: &surfaceFormatCount,
On 2016/04/05 16:50:28, egdaniel wrote:
> is it possible for this to return 0 surfaceFormatCount here? if it does will
> that hurt anything later on?

It has to support at least one surface format. So if it does return 0 we've got
bigger problems (like the driver is busted).

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:123: fSurface,
On 2016/04/05 16:50:28, egdaniel wrote:
> align these with fBackendContext

I hate Visual Studio.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:133: fSurface,
On 2016/04/05 16:50:28, egdaniel wrote:
> align

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:134: &presentModeCount,
On 2016/04/05 16:50:28, egdaniel wrote:
> again question on is a count of 0 bad?

Same as above.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:144: fSurface,
On 2016/04/05 16:50:28, egdaniel wrote:
> align

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:174: //if (ctx_.extent.width == extent.width
&& ctx_.extent.height == extent.height)
On 2016/04/05 16:50:28, egdaniel wrote:
> what is ctx referring to in this call?

Removed

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:184: //VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
On 2016/04/05 16:50:28, egdaniel wrote:
> why are these commented out?

I was trying to track down why things weren't working. Restored.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:189: VK_COMPOSITE_ALPHA_INHERIT_BIT_KHR));
On 2016/04/05 16:50:28, egdaniel wrote:
> align

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:210: swapchainCreateInfo.imageFormat =
surfaceFormats[0].format;       // for now, use the first one
On 2016/04/05 16:50:28, egdaniel wrote:
> is this something we'll search through at some point to find an sRGB surface,
or
> linear, or single channel, etc. if we want that? Any idea what the first
format
> generally is

Yes, this is where we'd search for sRGB support -- it's in the colorspace for
the format. Entry 0 on our NVIDIA cards seems to be non-sRGB UNORM. I'd expect
that 0 would be platform-dependent, but for now choosing it seems to be a
reasonable default.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:325: fBackbuffers = new
BackbufferInfo[fImageCount + 1];
On 2016/04/05 16:50:28, egdaniel wrote:
> comment on why the +1 here?

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.cpp:377: delete[] fSurfaces;   //*** will this
unref properly?
On 2016/04/05 14:24:43, bsalomon wrote:
> I believe it will since it will call ~sk_skp

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
File tools/vulkan/VulkanTestContext.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:8: #ifndef VULKANTESTCONTEXT_DEFINED
On 2016/04/05 16:50:28, egdaniel wrote:
> we don't usually capitalize every letter in the class name here

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:19: class VulkanTestContext {
On 2016/04/05 14:24:43, bsalomon wrote:
> Is this class specifically handling the window-backed case? Seems centered
> around having a real back buffer, presenting, etc. Wondering if it should be
> VulkanWindowContext or something. Or do you imagine using it for offscreen
> contexts as well?

I was thinking it would be -- don't we create a window and do swapping in
nanobench?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanTest...
tools/vulkan/VulkanTestContext.h:71: SkAutoTUnref<const GrVkBackendContext>
fBackendContext;
On 2016/04/05 16:50:28, egdaniel wrote:
> use sk_sp?

I agree, but I'd like to do that as a separate change -- it looks like changing
this might cause other changes to propagate outside of this patch.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanView...
File tools/vulkan/VulkanViewer.cpp (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/VulkanView...
tools/vulkan/VulkanViewer.cpp:2: * Copyright 2016 Google Inc.
On 2016/04/05 14:24:43, bsalomon wrote:
> Would it make since to put everything related to the viewer in it's own dir
> (e.g. tools/vulkan/viewer) to make it really clear what is framework and what
is
> app-specific?

Done.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h
File tools/vulkan/Window.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:33: virtual bool attach(BackEndTypes attachType, int
msaaSampleCount, AttachmentInfo*) = 0;
On 2016/04/05 14:24:44, bsalomon wrote:
> Is the window unusable until attach is called?

I'm not sure I understand. It's unusable for rendering GL or Vulkan.

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:34: void release();
On 2016/04/05 14:24:44, bsalomon wrote:
> Not totally obvious that this is related to attach until seeing the impl.
Maybe
> detach?

Done.

[bsalomon, 2016-04-05 18:44:41]

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h
File tools/vulkan/Window.h (right):

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:23: struct AttachmentInfo {
Should sample count go into attachment info?

https://codereview.chromium.org/1848833005/diff/60001/tools/vulkan/Window.h#n...
tools/vulkan/Window.h:33: virtual bool attach(BackEndTypes attachType, int
msaaSampleCount, AttachmentInfo*) = 0;
On 2016/04/05 18:16:49, jvanverth1 wrote:
> On 2016/04/05 14:24:44, bsalomon wrote:
> > Is the window unusable until attach is called?
> 
> I'm not sure I understand. It's unusable for rendering GL or Vulkan.

nm, I was just making sure I understood how the Window class was used.

[bsalomon, 2016-04-05 18:47:57]

lgtm

https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp
File gyp/vulkanviewer.gyp (right):

https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp#n...
gyp/vulkanviewer.gyp:30: '<!@(python find.py ../tools/vulkan "*.cpp")',
I should have thought of this earlier but do you want to separate out the viewer
from the framework in terms of gyp targets?

[jvanverth1, 2016-04-05 19:00:39]

https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp
File gyp/vulkanviewer.gyp (right):

https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp#n...
gyp/vulkanviewer.gyp:30: '<!@(python find.py ../tools/vulkan "*.cpp")',
On 2016/04/05 18:47:57, bsalomon wrote:
> I should have thought of this earlier but do you want to separate out the
viewer
> from the framework in terms of gyp targets?

Sure, how would I do that?

[bsalomon, 2016-04-05 19:44:16]

On 2016/04/05 19:00:39, jvanverth1 wrote:
> https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp
> File gyp/vulkanviewer.gyp (right):
> 
>
https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp#n...
> gyp/vulkanviewer.gyp:30: '<!@(python find.py ../tools/vulkan "*.cpp")',
> On 2016/04/05 18:47:57, bsalomon wrote:
> > I should have thought of this earlier but do you want to separate out the
> viewer
> > from the framework in terms of gyp targets?
> 
> Sure, how would I do that?

You would just make a new gyp file that only had the framework stuff in a target
with type static_library, instead of executable and then make vulkanviewer
target depend on it via its dependencies list. You'd probably have to move the
files so that the viewer files weren't in a subdirectory of the framework or
manually list the files instead of using find.py.

[jvanverth1, 2016-04-05 19:59:50]

On 2016/04/05 19:44:16, bsalomon wrote:
> On 2016/04/05 19:00:39, jvanverth1 wrote:
> > https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp
> > File gyp/vulkanviewer.gyp (right):
> > 
> >
>
https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp#n...
> > gyp/vulkanviewer.gyp:30: '<!@(python find.py ../tools/vulkan "*.cpp")',
> > On 2016/04/05 18:47:57, bsalomon wrote:
> > > I should have thought of this earlier but do you want to separate out the
> > viewer
> > > from the framework in terms of gyp targets?
> > 
> > Sure, how would I do that?
> 
> You would just make a new gyp file that only had the framework stuff in a
target
> with type static_library, instead of executable and then make vulkanviewer
> target depend on it via its dependencies list. You'd probably have to move the
> files so that the viewer files weren't in a subdirectory of the framework or
> manually list the files instead of using find.py.

Trying to do this my include paths got a little out of control, so I'll probably
need some help. Unless there's objection I'd like to commit this so Derek can
start looking at it, and then fix it up.

[bsalomon, 2016-04-06 00:06:40]

On 2016/04/05 19:59:50, jvanverth1 wrote:
> On 2016/04/05 19:44:16, bsalomon wrote:
> > On 2016/04/05 19:00:39, jvanverth1 wrote:
> > >
https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp
> > > File gyp/vulkanviewer.gyp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1848833005/diff/100001/gyp/vulkanviewer.gyp#n...
> > > gyp/vulkanviewer.gyp:30: '<!@(python find.py ../tools/vulkan "*.cpp")',
> > > On 2016/04/05 18:47:57, bsalomon wrote:
> > > > I should have thought of this earlier but do you want to separate out
the
> > > viewer
> > > > from the framework in terms of gyp targets?
> > > 
> > > Sure, how would I do that?
> > 
> > You would just make a new gyp file that only had the framework stuff in a
> target
> > with type static_library, instead of executable and then make vulkanviewer
> > target depend on it via its dependencies list. You'd probably have to move
the
> > files so that the viewer files weren't in a subdirectory of the framework or
> > manually list the files instead of using find.py.
> 
> Trying to do this my include paths got a little out of control, so I'll
probably
> need some help. Unless there's objection I'd like to commit this so Derek can
> start looking at it, and then fix it up.

sgtm, lgtm

[jvanverth1, 2016-04-06 12:59:07]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2016-04-06 12:59:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1848833005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1848833005/100001

[commit-bot: I haz the power, 2016-04-06 13:09:03]

Description was changed from

==========
First pass at VulkanViewer

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
First pass at VulkanViewer

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/9f37246d6856d40f916e8f1fff5b43cfdc42e567
==========

[commit-bot: I haz the power, 2016-04-06 13:09:04]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/9f37246d6856d40f916e8f1fff5b43cfdc42e567