Revert "Fixes to alignment issues with regards to mapped vulkan memory."

This reverts commit 9fb6cf4c49.

Reason for revert: breaks fuchsia

Original change's description:
> Fixes to alignment issues with regards to mapped vulkan memory.
> 
> Bug: skia:
> Change-Id: Ida9813fe774580a6d157b8eb8d330488c8e8c4bc
> Reviewed-on: https://skia-review.googlesource.com/109483
> Commit-Queue: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>

TBR=djsollen@google.com,egdaniel@google.com,jvanverth@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: skia:
Change-Id: If1223313cab27737ada401d1f3fe4b7ab849d03f
Reviewed-on: https://skia-review.googlesource.com/110040
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This commit is contained in:
Greg Daniel 2018-02-24 22:41:50 +00:00 committed by Skia Commit-Bot
parent 1267efac7f
commit 88fdee9bde
9 changed files with 19 additions and 81 deletions

View File

@ -31,17 +31,14 @@
* Vulkan textures are really const GrVkImageInfo* * Vulkan textures are really const GrVkImageInfo*
*/ */
struct GrVkAlloc { struct GrVkAlloc {
VkDeviceMemory fMemory = VK_NULL_HANDLE; // can be VK_NULL_HANDLE iff is an RT and is borrowed VkDeviceMemory fMemory; // can be VK_NULL_HANDLE iff Tex is an RT and uses borrow semantics
VkDeviceSize fOffset = 0; VkDeviceSize fOffset;
VkDeviceSize fSize = 0; // this can be indeterminate iff Tex uses borrow semantics VkDeviceSize fSize; // this can be indeterminate iff Tex uses borrow semantics
uint32_t fFlags= 0; uint32_t fFlags;
enum Flag { enum Flag {
kNoncoherent_Flag = 0x1, // memory must be flushed to device after mapping kNoncoherent_Flag = 0x1, // memory must be flushed to device after mapping
}; };
private:
friend class GrVkHeap; // For access to usesSystemHeap
bool fUsesSystemHeap = false;
}; };
struct GrVkImageInfo { struct GrVkImageInfo {

View File

@ -169,21 +169,6 @@ void GrVkBuffer::internalMap(GrVkGpu* gpu, size_t size, bool* createdNewBuffer)
if (fDesc.fDynamic) { if (fDesc.fDynamic) {
const GrVkAlloc& alloc = this->alloc(); const GrVkAlloc& alloc = this->alloc();
SkASSERT(alloc.fSize > 0);
// For Noncoherent buffers we want to make sure the range that we map, both offset and size,
// are aligned to the nonCoherentAtomSize limit. The offset should have been correctly
// aligned by our memory allocator. For size we pad out to make the range also aligned.
if (SkToBool(alloc.fFlags & GrVkAlloc::kNoncoherent_Flag)) {
// Currently we always have the internal offset as 0.
SkASSERT(0 == fOffset);
VkDeviceSize alignment = gpu->physicalDeviceProperties().limits.nonCoherentAtomSize;
SkASSERT(0 == (alloc.fOffset & (alignment - 1)));
// Make size of the map aligned to nonCoherentAtomSize
size = (size + alignment - 1) & ~(alignment - 1);
}
SkASSERT(size + fOffset <= alloc.fSize);
VkResult err = VK_CALL(gpu, MapMemory(gpu->device(), alloc.fMemory, VkResult err = VK_CALL(gpu, MapMemory(gpu->device(), alloc.fMemory,
alloc.fOffset + fOffset, alloc.fOffset + fOffset,
size, 0, &fMapPtr)); size, 0, &fMapPtr));

View File

@ -130,7 +130,6 @@ GrVkGpu::GrVkGpu(GrContext* context, const GrContextOptions& options,
fBackendContext->fFeatures, fBackendContext->fExtensions)); fBackendContext->fFeatures, fBackendContext->fExtensions));
fCaps.reset(SkRef(fVkCaps.get())); fCaps.reset(SkRef(fVkCaps.get()));
VK_CALL(GetPhysicalDeviceProperties(fBackendContext->fPhysicalDevice, &fPhysDevProps));
VK_CALL(GetPhysicalDeviceMemoryProperties(fBackendContext->fPhysicalDevice, &fPhysDevMemProps)); VK_CALL(GetPhysicalDeviceMemoryProperties(fBackendContext->fPhysicalDevice, &fPhysDevMemProps));
const VkCommandPoolCreateInfo cmdPoolInfo = { const VkCommandPoolCreateInfo cmdPoolInfo = {
@ -579,27 +578,12 @@ bool GrVkGpu::uploadTexDataLinear(GrVkTexture* tex, GrSurfaceOrigin texOrigin, i
int texTop = kBottomLeft_GrSurfaceOrigin == texOrigin ? tex->height() - top - height : top; int texTop = kBottomLeft_GrSurfaceOrigin == texOrigin ? tex->height() - top - height : top;
const GrVkAlloc& alloc = tex->alloc(); const GrVkAlloc& alloc = tex->alloc();
VkDeviceSize offset = alloc.fOffset + texTop*layout.rowPitch + left*bpp; VkDeviceSize offset = alloc.fOffset + texTop*layout.rowPitch + left*bpp;
VkDeviceSize offsetDiff = 0;
VkDeviceSize size = height*layout.rowPitch; VkDeviceSize size = height*layout.rowPitch;
// For Noncoherent buffers we want to make sure the range that we map, both offset and size,
// are aligned to the nonCoherentAtomSize limit. We may have to move the initial offset back to
// meet the alignment requirements. So we track how far we move back and then adjust the mapped
// ptr back up so that this is opaque to the caller.
if (SkToBool(alloc.fFlags & GrVkAlloc::kNoncoherent_Flag)) {
VkDeviceSize alignment = this->physicalDeviceProperties().limits.nonCoherentAtomSize;
offsetDiff = offset & (alignment - 1);
offset = offset - offsetDiff;
// Make size of the map aligned to nonCoherentAtomSize
size = (size + alignment - 1) & ~(alignment - 1);
}
SkASSERT(offset >= alloc.fOffset);
SkASSERT(size <= alloc.fOffset + alloc.fSize);
void* mapPtr; void* mapPtr;
err = GR_VK_CALL(interface, MapMemory(fDevice, alloc.fMemory, offset, size, 0, &mapPtr)); err = GR_VK_CALL(interface, MapMemory(fDevice, alloc.fMemory, offset, size, 0, &mapPtr));
if (err) { if (err) {
return false; return false;
} }
mapPtr = reinterpret_cast<char*>(mapPtr) + offsetDiff;
if (kBottomLeft_GrSurfaceOrigin == texOrigin) { if (kBottomLeft_GrSurfaceOrigin == texOrigin) {
// copy into buffer by rows // copy into buffer by rows
@ -1124,30 +1108,13 @@ GrStencilAttachment* GrVkGpu::createStencilAttachmentForRenderTarget(const GrRen
bool copy_testing_data(GrVkGpu* gpu, void* srcData, const GrVkAlloc& alloc, size_t bufferOffset, bool copy_testing_data(GrVkGpu* gpu, void* srcData, const GrVkAlloc& alloc, size_t bufferOffset,
size_t srcRowBytes, size_t dstRowBytes, int h) { size_t srcRowBytes, size_t dstRowBytes, int h) {
// For Noncoherent buffers we want to make sure the range that we map, both offset and size,
// are aligned to the nonCoherentAtomSize limit. We may have to move the initial offset back to
// meet the alignment requirements. So we track how far we move back and then adjust the mapped
// ptr back up so that this is opaque to the caller.
VkDeviceSize mapSize = dstRowBytes * h;
VkDeviceSize mapOffset = alloc.fOffset + bufferOffset;
VkDeviceSize offsetDiff = 0;
if (SkToBool(alloc.fFlags & GrVkAlloc::kNoncoherent_Flag)) {
VkDeviceSize alignment = gpu->physicalDeviceProperties().limits.nonCoherentAtomSize;
offsetDiff = mapOffset & (alignment - 1);
mapOffset = mapOffset - offsetDiff;
// Make size of the map aligned to nonCoherentAtomSize
mapSize = (mapSize + alignment - 1) & ~(alignment - 1);
}
SkASSERT(mapOffset >= alloc.fOffset);
SkASSERT(mapSize + mapOffset <= alloc.fOffset + alloc.fSize);
void* mapPtr; void* mapPtr;
VkResult err = GR_VK_CALL(gpu->vkInterface(), MapMemory(gpu->device(), VkResult err = GR_VK_CALL(gpu->vkInterface(), MapMemory(gpu->device(),
alloc.fMemory, alloc.fMemory,
mapOffset, alloc.fOffset + bufferOffset,
mapSize, dstRowBytes * h,
0, 0,
&mapPtr)); &mapPtr));
mapPtr = reinterpret_cast<char*>(mapPtr) + offsetDiff;
if (err) { if (err) {
return false; return false;
} }
@ -1212,7 +1179,7 @@ GrBackendTexture GrVkGpu::createTestingOnlyBackendTexture(void* srcData, int w,
} }
VkImage image = VK_NULL_HANDLE; VkImage image = VK_NULL_HANDLE;
GrVkAlloc alloc; GrVkAlloc alloc = { VK_NULL_HANDLE, 0, 0, 0 };
VkImageTiling imageTiling = linearTiling ? VK_IMAGE_TILING_LINEAR : VK_IMAGE_TILING_OPTIMAL; VkImageTiling imageTiling = linearTiling ? VK_IMAGE_TILING_LINEAR : VK_IMAGE_TILING_OPTIMAL;
VkImageLayout initialLayout = (VK_IMAGE_TILING_LINEAR == imageTiling) VkImageLayout initialLayout = (VK_IMAGE_TILING_LINEAR == imageTiling)
@ -1257,7 +1224,7 @@ GrBackendTexture GrVkGpu::createTestingOnlyBackendTexture(void* srcData, int w,
} }
// We need to declare these early so that we can delete them at the end outside of the if block. // We need to declare these early so that we can delete them at the end outside of the if block.
GrVkAlloc bufferAlloc; GrVkAlloc bufferAlloc = { VK_NULL_HANDLE, 0, 0, 0 };
VkBuffer buffer = VK_NULL_HANDLE; VkBuffer buffer = VK_NULL_HANDLE;
VkResult err; VkResult err;
@ -2011,8 +1978,8 @@ bool GrVkGpu::onReadPixels(GrSurface* surface, GrSurfaceOrigin origin, int left,
// We need to submit the current command buffer to the Queue and make sure it finishes before // We need to submit the current command buffer to the Queue and make sure it finishes before
// we can copy the data out of the buffer. // we can copy the data out of the buffer.
this->submitCommandBuffer(kForce_SyncQueue); this->submitCommandBuffer(kForce_SyncQueue);
void* mappedMemory = transferBuffer->map();
GrVkMemory::InvalidateMappedAlloc(this, transferBuffer->alloc()); GrVkMemory::InvalidateMappedAlloc(this, transferBuffer->alloc());
void* mappedMemory = transferBuffer->map();
if (copyFromOrigin) { if (copyFromOrigin) {
uint32_t skipRows = region.imageExtent.height - height; uint32_t skipRows = region.imageExtent.height - height;

View File

@ -51,9 +51,6 @@ public:
VkDevice device() const { return fDevice; } VkDevice device() const { return fDevice; }
VkQueue queue() const { return fQueue; } VkQueue queue() const { return fQueue; }
VkCommandPool cmdPool() const { return fCmdPool; } VkCommandPool cmdPool() const { return fCmdPool; }
VkPhysicalDeviceProperties physicalDeviceProperties() const {
return fPhysDevProps;
}
VkPhysicalDeviceMemoryProperties physicalDeviceMemoryProperties() const { VkPhysicalDeviceMemoryProperties physicalDeviceMemoryProperties() const {
return fPhysDevMemProps; return fPhysDevMemProps;
} }
@ -256,7 +253,6 @@ private:
SkSTArray<1, GrVkSemaphore::Resource*> fSemaphoresToWaitOn; SkSTArray<1, GrVkSemaphore::Resource*> fSemaphoresToWaitOn;
SkSTArray<1, GrVkSemaphore::Resource*> fSemaphoresToSignal; SkSTArray<1, GrVkSemaphore::Resource*> fSemaphoresToSignal;
VkPhysicalDeviceProperties fPhysDevProps;
VkPhysicalDeviceMemoryProperties fPhysDevMemProps; VkPhysicalDeviceMemoryProperties fPhysDevMemProps;
std::unique_ptr<GrVkHeap> fHeaps[kHeapCount]; std::unique_ptr<GrVkHeap> fHeaps[kHeapCount];

View File

@ -68,7 +68,6 @@ bool GrVkMemory::AllocAndBindBufferMemory(const GrVkGpu* gpu,
uint32_t typeIndex = 0; uint32_t typeIndex = 0;
uint32_t heapIndex = 0; uint32_t heapIndex = 0;
const VkPhysicalDeviceMemoryProperties& phDevMemProps = gpu->physicalDeviceMemoryProperties(); const VkPhysicalDeviceMemoryProperties& phDevMemProps = gpu->physicalDeviceMemoryProperties();
const VkPhysicalDeviceProperties& phDevProps = gpu->physicalDeviceProperties();
if (dynamic) { if (dynamic) {
// try to get cached and ideally non-coherent memory first // try to get cached and ideally non-coherent memory first
if (!get_valid_memory_type_index(phDevMemProps, if (!get_valid_memory_type_index(phDevMemProps,
@ -88,11 +87,6 @@ bool GrVkMemory::AllocAndBindBufferMemory(const GrVkGpu* gpu,
VkMemoryPropertyFlags mpf = phDevMemProps.memoryTypes[typeIndex].propertyFlags; VkMemoryPropertyFlags mpf = phDevMemProps.memoryTypes[typeIndex].propertyFlags;
alloc->fFlags = mpf & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT ? 0x0 alloc->fFlags = mpf & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT ? 0x0
: GrVkAlloc::kNoncoherent_Flag; : GrVkAlloc::kNoncoherent_Flag;
if (SkToBool(alloc->fFlags & GrVkAlloc::kNoncoherent_Flag)) {
SkASSERT(SkIsPow2(memReqs.alignment));
SkASSERT(SkIsPow2(phDevProps.limits.nonCoherentAtomSize));
memReqs.alignment = SkTMax(memReqs.alignment, phDevProps.limits.nonCoherentAtomSize);
}
} else { } else {
// device-local memory should always be available for static buffers // device-local memory should always be available for static buffers
SkASSERT_RELEASE(get_valid_memory_type_index(phDevMemProps, SkASSERT_RELEASE(get_valid_memory_type_index(phDevMemProps,
@ -299,7 +293,7 @@ void GrVkMemory::FlushMappedAlloc(const GrVkGpu* gpu, const GrVkAlloc& alloc) {
mappedMemoryRange.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE; mappedMemoryRange.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE;
mappedMemoryRange.memory = alloc.fMemory; mappedMemoryRange.memory = alloc.fMemory;
mappedMemoryRange.offset = alloc.fOffset; mappedMemoryRange.offset = alloc.fOffset;
mappedMemoryRange.size = VK_WHOLE_SIZE; // Size of what we mapped mappedMemoryRange.size = alloc.fSize;
GR_VK_CALL(gpu->vkInterface(), FlushMappedMemoryRanges(gpu->device(), GR_VK_CALL(gpu->vkInterface(), FlushMappedMemoryRanges(gpu->device(),
1, &mappedMemoryRange)); 1, &mappedMemoryRange));
} }
@ -312,7 +306,7 @@ void GrVkMemory::InvalidateMappedAlloc(const GrVkGpu* gpu, const GrVkAlloc& allo
mappedMemoryRange.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE; mappedMemoryRange.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE;
mappedMemoryRange.memory = alloc.fMemory; mappedMemoryRange.memory = alloc.fMemory;
mappedMemoryRange.offset = alloc.fOffset; mappedMemoryRange.offset = alloc.fOffset;
mappedMemoryRange.size = VK_WHOLE_SIZE; // Size of what we mapped mappedMemoryRange.size = alloc.fSize;
GR_VK_CALL(gpu->vkInterface(), InvalidateMappedMemoryRanges(gpu->device(), GR_VK_CALL(gpu->vkInterface(), InvalidateMappedMemoryRanges(gpu->device(),
1, &mappedMemoryRange)); 1, &mappedMemoryRange));
} }
@ -525,7 +519,7 @@ bool GrVkHeap::subAlloc(VkDeviceSize size, VkDeviceSize alignment,
VkMemoryAllocateInfo allocInfo = { VkMemoryAllocateInfo allocInfo = {
VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, // sType VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, // sType
nullptr, // pNext nullptr, // pNext
alignedSize, // allocationSize size, // allocationSize
memoryTypeIndex, // memoryTypeIndex memoryTypeIndex, // memoryTypeIndex
}; };
@ -537,8 +531,7 @@ bool GrVkHeap::subAlloc(VkDeviceSize size, VkDeviceSize alignment,
return false; return false;
} }
alloc->fOffset = 0; alloc->fOffset = 0;
alloc->fSize = alignedSize; alloc->fSize = 0; // hint that this is not a subheap allocation
alloc->fUsesSystemHeap = true;
#ifdef SK_DEBUG #ifdef SK_DEBUG
gHeapUsage[VK_MAX_MEMORY_HEAPS] += alignedSize; gHeapUsage[VK_MAX_MEMORY_HEAPS] += alignedSize;
#endif #endif
@ -631,7 +624,7 @@ bool GrVkHeap::singleAlloc(VkDeviceSize size, VkDeviceSize alignment,
bool GrVkHeap::free(const GrVkAlloc& alloc) { bool GrVkHeap::free(const GrVkAlloc& alloc) {
// a size of 0 means we're using the system heap // a size of 0 means we're using the system heap
if (alloc.fUsesSystemHeap) { if (0 == alloc.fSize) {
const GrVkInterface* iface = fGpu->vkInterface(); const GrVkInterface* iface = fGpu->vkInterface();
GR_VK_CALL(iface, FreeMemory(fGpu->device(), alloc.fMemory, nullptr)); GR_VK_CALL(iface, FreeMemory(fGpu->device(), alloc.fMemory, nullptr));
return true; return true;

View File

@ -141,7 +141,6 @@ public:
bool alloc(VkDeviceSize size, VkDeviceSize alignment, uint32_t memoryTypeIndex, bool alloc(VkDeviceSize size, VkDeviceSize alignment, uint32_t memoryTypeIndex,
uint32_t heapIndex, GrVkAlloc* alloc) { uint32_t heapIndex, GrVkAlloc* alloc) {
SkASSERT(size > 0); SkASSERT(size > 0);
alloc->fUsesSystemHeap = false;
return (*this.*fAllocFunc)(size, alignment, memoryTypeIndex, heapIndex, alloc); return (*this.*fAllocFunc)(size, alignment, memoryTypeIndex, heapIndex, alloc);
} }
bool free(const GrVkAlloc& alloc); bool free(const GrVkAlloc& alloc);

View File

@ -159,6 +159,7 @@ void suballoc_test(skiatest::Reporter* reporter, GrContext* context) {
REPORTER_ASSERT(reporter, heap.allocSize() == 128 * 1024 && heap.usedSize() == 0 * 1024); REPORTER_ASSERT(reporter, heap.allocSize() == 128 * 1024 && heap.usedSize() == 0 * 1024);
// heap should not grow here (allocating more than subheap size) // heap should not grow here (allocating more than subheap size)
REPORTER_ASSERT(reporter, heap.alloc(128 * 1024, kAlignment, kMemType, kHeapIndex, &alloc0)); REPORTER_ASSERT(reporter, heap.alloc(128 * 1024, kAlignment, kMemType, kHeapIndex, &alloc0));
REPORTER_ASSERT(reporter, 0 == alloc0.fSize);
REPORTER_ASSERT(reporter, heap.allocSize() == 128 * 1024 && heap.usedSize() == 0 * 1024); REPORTER_ASSERT(reporter, heap.allocSize() == 128 * 1024 && heap.usedSize() == 0 * 1024);
heap.free(alloc0); heap.free(alloc0);
REPORTER_ASSERT(reporter, heap.alloc(24 * 1024, kAlignment, kMemType, kHeapIndex, &alloc0)); REPORTER_ASSERT(reporter, heap.alloc(24 * 1024, kAlignment, kMemType, kHeapIndex, &alloc0));

View File

@ -55,7 +55,7 @@ void wrap_tex_test(skiatest::Reporter* reporter, GrContext* context) {
// alloc is null // alloc is null
{ {
GrVkImageInfo backendCopy = *imageInfo; GrVkImageInfo backendCopy = *imageInfo;
backendCopy.fAlloc = GrVkAlloc(); backendCopy.fAlloc = { VK_NULL_HANDLE, 0, 0, 0 };
GrBackendTexture backendTex = GrBackendTexture(kW, kH, backendCopy); GrBackendTexture backendTex = GrBackendTexture(kW, kH, backendCopy);
tex = gpu->wrapBackendTexture(backendTex, kBorrow_GrWrapOwnership); tex = gpu->wrapBackendTexture(backendTex, kBorrow_GrWrapOwnership);
REPORTER_ASSERT(reporter, !tex); REPORTER_ASSERT(reporter, !tex);
@ -100,7 +100,7 @@ void wrap_rt_test(skiatest::Reporter* reporter, GrContext* context) {
// alloc is null // alloc is null
{ {
GrVkImageInfo backendCopy = *imageInfo; GrVkImageInfo backendCopy = *imageInfo;
backendCopy.fAlloc = GrVkAlloc(); backendCopy.fAlloc = { VK_NULL_HANDLE, 0, 0, 0 };
// can wrap null alloc // can wrap null alloc
GrBackendRenderTarget backendRT(kW, kH, 1, 0, backendCopy); GrBackendRenderTarget backendRT(kW, kH, 1, 0, backendCopy);
rt = gpu->wrapBackendRenderTarget(backendRT); rt = gpu->wrapBackendRenderTarget(backendRT);
@ -138,7 +138,7 @@ void wrap_trt_test(skiatest::Reporter* reporter, GrContext* context) {
// alloc is null // alloc is null
{ {
GrVkImageInfo backendCopy = *imageInfo; GrVkImageInfo backendCopy = *imageInfo;
backendCopy.fAlloc = GrVkAlloc(); backendCopy.fAlloc = { VK_NULL_HANDLE, 0, 0, 0 };
GrBackendTexture backendTex = GrBackendTexture(kW, kH, backendCopy); GrBackendTexture backendTex = GrBackendTexture(kW, kH, backendCopy);
tex = gpu->wrapRenderableBackendTexture(backendTex, 1, kBorrow_GrWrapOwnership); tex = gpu->wrapRenderableBackendTexture(backendTex, 1, kBorrow_GrWrapOwnership);
REPORTER_ASSERT(reporter, !tex); REPORTER_ASSERT(reporter, !tex);

View File

@ -287,7 +287,7 @@ void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType)
GrVkImageInfo info; GrVkImageInfo info;
info.fImage = fImages[i]; info.fImage = fImages[i];
info.fAlloc = GrVkAlloc(); info.fAlloc = { VK_NULL_HANDLE, 0, 0, 0 };
info.fImageLayout = VK_IMAGE_LAYOUT_UNDEFINED; info.fImageLayout = VK_IMAGE_LAYOUT_UNDEFINED;
info.fImageTiling = VK_IMAGE_TILING_OPTIMAL; info.fImageTiling = VK_IMAGE_TILING_OPTIMAL;
info.fFormat = format; info.fFormat = format;