Add discard support to Vulkan

Add discard support to Vulkan

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

Committed: https://skia.googlesource.com/skia/+/37535c9ecef67a56cad293535f8626b1d60f7c08

[egdaniel, 2016-06-29 15:53:38]

Description was changed from

==========
Add discard support to Vulkan

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

to

==========
Add discard support to Vulkan

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

[egdaniel, 2016-06-29 15:54:43]

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

[egdaniel, 2016-06-29 15:54:44]

This does discards for the color attachments. I'll make a different change for
testing them on stencil.

[jvanverth1, 2016-06-29 16:00:30]

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
File src/gpu/vk/GrVkGpuCommandBuffer.cpp (right):

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:138: GrVkRenderPass::LoadStoreOps
vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD,
Shouldn't this and the resolve be LOAD_OP_DONT_CARE as well?

[egdaniel, 2016-06-29 16:11:30]

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
File src/gpu/vk/GrVkGpuCommandBuffer.cpp (right):

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:138: GrVkRenderPass::LoadStoreOps
vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD,
On 2016/06/29 16:00:30, jvanverth1 wrote:
> Shouldn't this and the resolve be LOAD_OP_DONT_CARE as well?

In general, when we call discard on a RT do we mean it for both color and
stencil?

In a follow on CL I'm actually going to experiment with making stencil DONT_CARE
always for both load and store (there are some unlucky situations where this
could cause a bug, but we'll see how bad it is/isn't).

For resolve it doesn't matter right now what we put here since it is not used.

[bsalomon, 2016-06-29 17:45:25]

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
File src/gpu/vk/GrVkGpuCommandBuffer.cpp (right):

https://codereview.chromium.org/2104353002/diff/1/src/gpu/vk/GrVkGpuCommandBu...
src/gpu/vk/GrVkGpuCommandBuffer.cpp:138: GrVkRenderPass::LoadStoreOps
vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD,
On 2016/06/29 16:11:30, egdaniel wrote:
> On 2016/06/29 16:00:30, jvanverth1 wrote:
> > Shouldn't this and the resolve be LOAD_OP_DONT_CARE as well?
> 
> In general, when we call discard on a RT do we mean it for both color and
> stencil?
> 

both, I guess? The current discard stuff is not useful in the GL backend so
redefine it as you see fit.

> In a follow on CL I'm actually going to experiment with making stencil
DONT_CARE
> always for both load and store (there are some unlucky situations where this
> could cause a bug, but we'll see how bad it is/isn't).
> 
> For resolve it doesn't matter right now what we put here since it is not used.

[bsalomon, 2016-06-30 13:47:31]

lgtm w/ question

https://codereview.chromium.org/2104353002/diff/40001/src/gpu/GrGpu.cpp
File src/gpu/GrGpu.cpp (right):

https://codereview.chromium.org/2104353002/diff/40001/src/gpu/GrGpu.cpp#newco...
src/gpu/GrGpu.cpp:181: if (isRT && texels.empty()) {
I imagine that this is not going to be a long term way of the initial discard.
Should we have a comment here about removing this once we auto insert discards
post-MDB?

[egdaniel, 2016-06-30 14:42:32]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-06-30 14:42:33]

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

[commit-bot: I haz the power, 2016-06-30 14:42:38]

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

[egdaniel, 2016-06-30 14:56:33]

The CQ bit was unchecked by egdaniel@google.com

[egdaniel, 2016-06-30 15:22:29]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2016-06-30 15:22:33]

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-06-30 15:23:33]

Description was changed from

==========
Add discard support to Vulkan

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

to

==========
Add discard support to Vulkan

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

Committed:
https://skia.googlesource.com/skia/+/37535c9ecef67a56cad293535f8626b1d60f7c08
==========

[commit-bot: I haz the power, 2016-06-30 15:23:34]

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

[commit-bot: I haz the power, 2016-06-30 15:23:37]

CQ bit was unchecked.