[impeller] [vulkan] Support textures backed by vk::Images
#35163
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
|
||
| TextureVK::~TextureVK() { | ||
| if (image_) { | ||
| vmaDestroyImage(allocator_, image_, allocation_); |
There was a problem hiding this comment.
IMO (in a later patch is fine) we should create RAII wrappers for VMA handles.
|
|
||
| // |Texture| | ||
| ~TextureVK() override; | ||
|
|
||
| private: | ||
| ContextVK& context_; | ||
| const VmaAllocator& allocator_; | ||
| VkImage image_; |
There was a problem hiding this comment.
Shouldn't these be unique images?
There was a problem hiding this comment.
Also, zero out these variable explicitly in case we add more ctors.
There was a problem hiding this comment.
I thought the image was just a "holder" to the vma managed allocation, and in the dtor i clean it up. Should we still have a UniqueImage?
| return image_; | ||
| } | ||
|
|
||
| ISize TextureVK::GetSize() const { |
There was a problem hiding this comment.
I wonder where this isn't true. Perhaps we should just move this to the base class?
There was a problem hiding this comment.
agreed, I think during creation we ensure that the size is the same as the descriptor size.
| // allocations, but setting them to be host visible for now. | ||
| alloc_create_info.flags = | ||
| VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT | | ||
| VMA_ALLOCATION_CREATE_MAPPED_BIT; |
There was a problem hiding this comment.
IMO we should manage memory mappings explicitly. Thats more efficient and the API already allows for that. All you need is the VMA map-unmap calls later before and after the memcpy.
|
will address the follow-ups in future PRs. |
* 7a536ee Roll Dart SDK from 0ab07c889f74 to 16cbbc9d5e4b (1 revision) (flutter/engine#35177) * e1e4a28 [Impeller] Enforce shader resource limits in impellerc (flutter/engine#35120) * d6931cc Roll Fuchsia Mac SDK from CUZLMQ1lB... to b8WQvI4f0... (flutter/engine#35179) * c1bb5d7 Roll Dart SDK from 16cbbc9d5e4b to aada0a67f81e (1 revision) (flutter/engine#35180) * d45c4c4 ColorFilterLayer change SkColorFilter to DLColorFilter (flutter/engine#34744) * 1a678d2 Roll Fuchsia Linux SDK from BRTz21cLl... to kURZcohuz... (flutter/engine#35182) * cdccc60 Roll Dart SDK from aada0a67f81e to 344a7d12b413 (1 revision) (flutter/engine#35181) * 2c7b4f6 Update setAssetDirectory service extension to fail if provided path is invalid (flutter/engine#35178) * ce3397f fix analysis error (flutter/engine#35187) * eaeae8e Make it possible to obtain `FontWeight` integer value (flutter/engine#35183) * 1f9d87d Analyze all dart code on CI (flutter/engine#35147) * 1cf7023 [impeller] [vulkan] Support textures backed by `vk::Image`s (flutter/engine#35163) * 57f43bd Roll Skia from 098c234c05f7 to f1245dcd35f8 (23 revisions) (flutter/engine#35189)
…09063) * 7a536ee Roll Dart SDK from 0ab07c889f74 to 16cbbc9d5e4b (1 revision) (flutter/engine#35177) * e1e4a28 [Impeller] Enforce shader resource limits in impellerc (flutter/engine#35120) * d6931cc Roll Fuchsia Mac SDK from CUZLMQ1lB... to b8WQvI4f0... (flutter/engine#35179) * c1bb5d7 Roll Dart SDK from 16cbbc9d5e4b to aada0a67f81e (1 revision) (flutter/engine#35180) * d45c4c4 ColorFilterLayer change SkColorFilter to DLColorFilter (flutter/engine#34744) * 1a678d2 Roll Fuchsia Linux SDK from BRTz21cLl... to kURZcohuz... (flutter/engine#35182) * cdccc60 Roll Dart SDK from aada0a67f81e to 344a7d12b413 (1 revision) (flutter/engine#35181) * 2c7b4f6 Update setAssetDirectory service extension to fail if provided path is invalid (flutter/engine#35178) * ce3397f fix analysis error (flutter/engine#35187) * eaeae8e Make it possible to obtain `FontWeight` integer value (flutter/engine#35183) * 1f9d87d Analyze all dart code on CI (flutter/engine#35147) * 1cf7023 [impeller] [vulkan] Support textures backed by `vk::Image`s (flutter/engine#35163) * 57f43bd Roll Skia from 098c234c05f7 to f1245dcd35f8 (23 revisions) (flutter/engine#35189)
No description provided.