Don't add the resolve attachment to vulkan render passes.

Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

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

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

[egdaniel, 2016-08-17 20:38:35]

Description was changed from

==========
Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

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

to

==========
Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

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

[egdaniel, 2016-08-26 13:49:13]

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

[egdaniel, 2016-08-26 13:49:13]

Next part of vulkan msaa. Main thing left after this is to get msaa copies
working in vulkan

[jvanverth1, 2016-08-26 15:03:19]

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
File src/gpu/vk/GrVkGpuCommandBuffer.cpp (right):

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:104: // we dont' attach the resolve to the
framebuffer so no need to change its layout.
don't

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:105: if (GrVkImage* msaaImage =
fRenderTarget->msaaImage()) {
How about 
GrVkImage* layoutImage = fRenderTarget->msaaImage() ? fRenderTarget->msaaImage()
                                                    : fRenderTarget;
layoutImage->setImageLayout(...

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
File src/gpu/vk/GrVkRenderTarget.cpp (right):

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:242: nullptr, stencilView);
This is the only place we use Framebuffer::Create(). Can we get rid of this
param?

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:257: if (colorSamples > 0) {
Do we need this code?

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:293: //
commandBuffer.addResource(this->resolveAttachmentView());
Again, do we need this code?

[egdaniel, 2016-08-26 15:48:46]

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
File src/gpu/vk/GrVkGpuCommandBuffer.cpp (right):

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:104: // we dont' attach the resolve to the
framebuffer so no need to change its layout.
On 2016/08/26 15:03:19, jvanverth1 wrote:
> don't

Done.

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkGpuComma...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:105: if (GrVkImage* msaaImage =
fRenderTarget->msaaImage()) {
On 2016/08/26 15:03:19, jvanverth1 wrote:
> How about 
> GrVkImage* layoutImage = fRenderTarget->msaaImage() ?
fRenderTarget->msaaImage()
>                                                     : fRenderTarget;
> layoutImage->setImageLayout(...

Done.

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
File src/gpu/vk/GrVkRenderTarget.cpp (right):

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:242: nullptr, stencilView);
On 2016/08/26 15:03:19, jvanverth1 wrote:
> This is the only place we use Framebuffer::Create(). Can we get rid of this
> param?

So in my head I was thinking we should save this code, and similar parts below,
for once we start doing resolves at the end of render passes. However, as I
think about it, I don't want to do that until we do the whole render target,
framebuffer split/refactorization in Ganesh as a whole. And at that point this
code is going to look different anyways. So I'll delete all this then and in
rest of code

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:257: if (colorSamples > 0) {
On 2016/08/26 15:03:19, jvanverth1 wrote:
> Do we need this code?

Done.

https://codereview.chromium.org/2256843002/diff/40001/src/gpu/vk/GrVkRenderTa...
src/gpu/vk/GrVkRenderTarget.cpp:293: //
commandBuffer.addResource(this->resolveAttachmentView());
On 2016/08/26 15:03:19, jvanverth1 wrote:
> Again, do we need this code?

Done.

[jvanverth1, 2016-08-26 17:37:30]

lgtm

[egdaniel, 2016-08-26 17:43:08]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2016-08-26 17:43:21]

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-26 18:05:15]

Description was changed from

==========
Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

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

to

==========
Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

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

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

[commit-bot: I haz the power, 2016-08-26 18:05:16]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/ce3bfb1ed155880585b2d0bb0a8d3e43306e23f2