Add addtional resolve calls to vulkan backend

Add addtional resolve calls to vulkan backend

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

Committed: https://skia.googlesource.com/skia/+/66933552f1723c4a2b248711ab3d43921401e8e6

[egdaniel, 2016-08-05 15:10:05]

Description was changed from

==========
Add addtional resolve calls to vulkan backend

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

to

==========
Add addtional resolve calls to vulkan backend

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

[egdaniel, 2016-08-05 19:07:15]

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

[egdaniel, 2016-08-05 19:07:15]

This CL adds the necessary spots in Vulkan where we need to resolve MSAA RTs.
This does not actually do anything to enable msaa yet.

Copies are an area I haven't addressed yet in this CL. For copies to work we
will need to be able to do copies as draws in vulkan.

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp
File src/gpu/vk/GrVkGpu.cpp (right):

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:371: SkASSERT(target->numColorSamples() > 1);
going back to this. Returning getResolveType as auto for non-msaa cases seems to
fix the problem of us trying to resolve non-msaa RTs

[egdaniel, 2016-08-18 13:13:17]

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

[egdaniel, 2016-08-18 13:13:18]

ptal

[Brian Osman, 2016-08-18 13:28:22]

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp
File src/gpu/vk/GrVkGpu.cpp (right):

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:1470: GrVkRenderTarget* rt =
static_cast<GrVkRenderTarget*>(surface->asRenderTarget());
This is the only place where you're doing the extra checking for resolve type
before calling onResolveRenderTarget. I'm guessing that the other call-sites
won't ever encounter a non-msaa RT? Or would it just be safer to put this logic
in onResolveRenderTarget, so we can always call it blindly?

[egdaniel, 2016-08-18 13:31:39]

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp
File src/gpu/vk/GrVkGpu.cpp (right):

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:1470: GrVkRenderTarget* rt =
static_cast<GrVkRenderTarget*>(surface->asRenderTarget());
On 2016/08/18 13:28:22, Brian Osman wrote:
> This is the only place where you're doing the extra checking for resolve type
> before calling onResolveRenderTarget. I'm guessing that the other call-sites
> won't ever encounter a non-msaa RT? Or would it just be safer to put this
logic
> in onResolveRenderTarget, so we can always call it blindly?

Yeah I can move it up there. I was actually just copying how our GL side is set
up.

[egdaniel, 2016-08-18 13:36:23]

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp
File src/gpu/vk/GrVkGpu.cpp (right):

https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:1470: GrVkRenderTarget* rt =
static_cast<GrVkRenderTarget*>(surface->asRenderTarget());
On 2016/08/18 13:31:39, egdaniel wrote:
> On 2016/08/18 13:28:22, Brian Osman wrote:
> > This is the only place where you're doing the extra checking for resolve
type
> > before calling onResolveRenderTarget. I'm guessing that the other call-sites
> > won't ever encounter a non-msaa RT? Or would it just be safer to put this
> logic
> > in onResolveRenderTarget, so we can always call it blindly?
> 
> Yeah I can move it up there. I was actually just copying how our GL side is
set
> up.

Actually I see the reasoning now. onResolveRenderTarget is a virtual that
currently has no return type. onReadPixels is the only place where it matters if
we were able to actually resolve or not (returning a bool itself). Thus the
extra check there.

[egdaniel, 2016-08-23 14:04:17]

On 2016/08/18 13:36:23, egdaniel wrote:
> https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp
> File src/gpu/vk/GrVkGpu.cpp (right):
> 
>
https://codereview.chromium.org/2215363003/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
> src/gpu/vk/GrVkGpu.cpp:1470: GrVkRenderTarget* rt =
> static_cast<GrVkRenderTarget*>(surface->asRenderTarget());
> On 2016/08/18 13:31:39, egdaniel wrote:
> > On 2016/08/18 13:28:22, Brian Osman wrote:
> > > This is the only place where you're doing the extra checking for resolve
> type
> > > before calling onResolveRenderTarget. I'm guessing that the other
call-sites
> > > won't ever encounter a non-msaa RT? Or would it just be safer to put this
> > logic
> > > in onResolveRenderTarget, so we can always call it blindly?
> > 
> > Yeah I can move it up there. I was actually just copying how our GL side is
> set
> > up.
> 
> Actually I see the reasoning now. onResolveRenderTarget is a virtual that
> currently has no return type. onReadPixels is the only place where it matters
if
> we were able to actually resolve or not (returning a bool itself). Thus the
> extra check there.

re-ping

[Brian Osman, 2016-08-23 14:08:43]

lgtm

[egdaniel, 2016-08-24 13:40:12]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2016-08-24 13:40:17]

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-24 14:22:21]

Description was changed from

==========
Add addtional resolve calls to vulkan backend

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

to

==========
Add addtional resolve calls to vulkan backend

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

Committed:
https://skia.googlesource.com/skia/+/66933552f1723c4a2b248711ab3d43921401e8e6
==========

[commit-bot: I haz the power, 2016-08-24 14:22:23]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/66933552f1723c4a2b248711ab3d43921401e8e6