Add mipmap loading to Vulkan.

Add mipmap loading to Vulkan.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1925303002

Committed: https://skia.googlesource.com/skia/+/900bd4a0463bc6471ef07a77120b413bd8f472b2

[jvanverth1, 2016-04-28 19:27:10]

Description was changed from

==========
Add mipmap loading to Vulkan.
==========

to

==========
Add mipmap loading to Vulkan.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[jvanverth1, 2016-04-28 19:56:57]

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

[bsalomon, 2016-04-29 13:29:42]

seems ok to me but I defer to Greg

[jvanverth1, 2016-04-29 13:44:15]

A couple comments and one update:

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkImage.h
File src/gpu/vk/GrVkImage.h (right):

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkImage.h#...
src/gpu/vk/GrVkImage.h:34: GrBackendObjectOwnership fOwnership;
I added this here rather than the texture because the texture is just given a
resource and doesn't know whether it's borrowed or not.

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkTexture.h
File src/gpu/vk/GrVkTexture.h (right):

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkTexture....
src/gpu/vk/GrVkTexture.h:59: GrBackendObjectOwnership fTextureOwnership;
Removed based on offline comments.

[egdaniel, 2016-04-29 14:12:28]

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

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:329: : top;
can this now fit on the line above?

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:395: &width,
align these?

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:492: &regions[0]);
why not just regions.begin()?

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:550: imageDesc.fLevels = linearTiling ? 1 :
texels.count();
how about we just decide to use linear only if texels.count() ==1

[jvanverth1, 2016-04-29 17:36:32]

Comments addressed.

In doing this, I realized that we may need to allocate subresources when using
onWriteTexels with mipmaps, so added code to handle that case. I also fixed some
bugs with marking mipmaps dirty.

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

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:329: : top;
On 2016/04/29 14:12:28, egdaniel wrote:
> can this now fit on the line above?

Done.

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:395: &width,
On 2016/04/29 14:12:28, egdaniel wrote:
> align these?

Done.

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:492: &regions[0]);
On 2016/04/29 14:12:27, egdaniel wrote:
> why not just regions.begin()?

Done.

https://codereview.chromium.org/1925303002/diff/20001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:550: imageDesc.fLevels = linearTiling ? 1 :
texels.count();
On 2016/04/29 14:12:27, egdaniel wrote:
> how about we just decide to use linear only if texels.count() ==1

Linear tiling and a mipmap is invalid input, so returns nullptr in this case.

[egdaniel, 2016-04-29 18:10:28]

lgtm with just some general questions

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

https://codereview.chromium.org/1925303002/diff/60001/src/gpu/GrGpu.cpp#newco...
src/gpu/GrGpu.cpp:431: if (texture && 1 == mipLevels) {
just to clarify for myself here. The reason we make mipLevels == 1 as dirty is
because if we actually had someone pass in a whole mip chain to us we would
write all those levels and assume it is fine right?

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

https://codereview.chromium.org/1925303002/diff/60001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:1362: this->didWriteToSurface(dst, &dstRect);
is this something that GL does as well when we do copies?

[jvanverth1, 2016-04-29 18:16:10]

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

https://codereview.chromium.org/1925303002/diff/60001/src/gpu/GrGpu.cpp#newco...
src/gpu/GrGpu.cpp:431: if (texture && 1 == mipLevels) {
On 2016/04/29 18:10:28, egdaniel wrote:
> just to clarify for myself here. The reason we make mipLevels == 1 as dirty is
> because if we actually had someone pass in a whole mip chain to us we would
> write all those levels and assume it is fine right?

Yes, that's right.

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

https://codereview.chromium.org/1925303002/diff/60001/src/gpu/vk/GrVkGpu.cpp#...
src/gpu/vk/GrVkGpu.cpp:1362: this->didWriteToSurface(dst, &dstRect);
On 2016/04/29 18:10:28, egdaniel wrote:
> is this something that GL does as well when we do copies?

Yup, and in flushRenderTarget(). I don't think we have an equivalent of that.

[jvanverth1, 2016-04-29 18:26:52]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2016-04-29 18:27:01]

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

[commit-bot: I haz the power, 2016-04-29 20:53:14]

Description was changed from

==========
Add mipmap loading to Vulkan.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add mipmap loading to Vulkan.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/900bd4a0463bc6471ef07a77120b413bd8f472b2
==========

[commit-bot: I haz the power, 2016-04-29 20:53:15]

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