Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crashes with Vulkan sparse bindings #1966

Merged
merged 1 commit into from
Jun 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gapis/api/vulkan/api/buffer.api
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
VkMemoryRequirements MemoryRequirements
ref!DedicatedRequirementsKHR DedicatedRequirementsKHR
}

@threadSafety("system")
@indirect("VkDevice")
@override
Expand Down
1 change: 0 additions & 1 deletion gapis/api/vulkan/api/copy_clear_commands.api
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ sub void dovkCmdCopyBufferToImage(ref!vkCmdCopyBufferToImageArgs args) {
// When multiple layers are specified in the buffer image copy region,
// Vulkan assumes the data of all the layers are placed continuously in
// the source buffer memory.
// TODO: (qining) Handle aspect mask
for j in (0 .. region.imageSubresource.layerCount) {
layerIndex := region.imageSubresource.baseArrayLayer + j
bufferLayerOffset := (as!u64(j) * layerSize) + as!u64(region.bufferOffset)
Expand Down
1 change: 1 addition & 0 deletions gapis/api/vulkan/api/image.api
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
}

@internal class SparseBoundImageLevelInfo {
// mapping from memory offset to sparse block info
map!(u64, ref!SparseBoundImageBlockInfo) Blocks
}

Expand Down
63 changes: 63 additions & 0 deletions gapis/api/vulkan/api/queued_command_tracking.api
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,76 @@ sub void execPendingCommands(VkQueue queue) {
bind.flags)
}
}
// Creates the shadow image data memory here if has not been done so.
// This is exactly the same to vkBindImageMemory except for linear
// and preinitialized images, we also create individual pool here.
// TODO: Redirect to bound memories for the data of linear preinitialized
// images.
for _, _, aspectBit in unpackImageAspectFlags(image.ImageAspect).Bits {
aspect := image.Aspects[aspectBit]
for lay in (0 .. image.Info.ArrayLayers) {
for lev in (0 .. image.Info.MipLevels) {
level := aspect.Layers[lay].Levels[lev]
if len(level.Data) == 0 {
elementAndTexelBlockSize := getElementAndTexelBlockSize(image.Info.Format)
depthElementSize := getDepthElementSize(image.Info.Format, false)
// Roundup the width and height in the number of blocks.
widthInBlocks := roundUpTo(level.Width, elementAndTexelBlockSize.TexelBlockSize.Width)
heightInBlocks := roundUpTo(level.Height, elementAndTexelBlockSize.TexelBlockSize.Height)
elementSize := switch (aspectBit) {
case VK_IMAGE_ASPECT_COLOR_BIT:
elementAndTexelBlockSize.ElementSize
case VK_IMAGE_ASPECT_DEPTH_BIT:
depthElementSize
case VK_IMAGE_ASPECT_STENCIL_BIT:
// stencil element is always 1 byte wide
as!u32(1)
}
size := widthInBlocks * heightInBlocks * level.Depth * elementSize
level.Data = make!u8(size)
}
}
}
}
}

for _ , img , binds in cmd.SparseBinds.ImageBinds {
if (!img in Images) { vkErrorInvalidImage(img) }
for _ , _ , bind in binds.SparseImageMemoryBinds {
addSparseImageMemoryBinding(img, bind)
}
image := Images[img]
// Creates the shadow image data memory here if has not been done so.
// This is exactly the same to vkBindImageMemory except for linear
// and preinitialized images, we also create individual pool here.
// TODO: Redirect to bound memories for the data of linear preinitialized
// images.
for _, _, aspectBit in unpackImageAspectFlags(image.ImageAspect).Bits {
aspect := image.Aspects[aspectBit]
for lay in (0 .. image.Info.ArrayLayers) {
for lev in (0 .. image.Info.MipLevels) {
level := aspect.Layers[lay].Levels[lev]
if len(level.Data) == 0 {
elementAndTexelBlockSize := getElementAndTexelBlockSize(image.Info.Format)
depthElementSize := getDepthElementSize(image.Info.Format, false)
// Roundup the width and height in the number of blocks.
widthInBlocks := roundUpTo(level.Width, elementAndTexelBlockSize.TexelBlockSize.Width)
heightInBlocks := roundUpTo(level.Height, elementAndTexelBlockSize.TexelBlockSize.Height)
elementSize := switch (aspectBit) {
case VK_IMAGE_ASPECT_COLOR_BIT:
elementAndTexelBlockSize.ElementSize
case VK_IMAGE_ASPECT_DEPTH_BIT:
depthElementSize
case VK_IMAGE_ASPECT_STENCIL_BIT:
// stencil element is always 1 byte wide
as!u32(1)
}
size := widthInBlocks * heightInBlocks * level.Depth * elementSize
level.Data = make!u8(size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to see what this looks like if/when we actually switch our pools over to actual byte slices. This will likely not be what we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to 2 steps

  1. Get the texture data from replay side, so that nothing rely on the image level data pool any more
  2. Move to use the device memory data pool.

Or as you said, we can move to the pool first, for the cases that the calculated size is smaller than the required size.

}
}
}
}
}

postBindSparse(cmd.SparseBinds)
Expand Down
89 changes: 59 additions & 30 deletions gapis/api/vulkan/state_rebuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package vulkan

import (
"context"
"sort"

"github.com/google/gapid/core/log"
"github.com/google/gapid/core/math/interval"
Expand Down Expand Up @@ -1085,6 +1086,11 @@ func (sb *stateBuilder) createBuffer(buffer BufferObjectʳ) {
}
}

bufSparseBindings := make([]VkSparseMemoryBind, 0, buffer.SparseMemoryBindings().Len())
for _, bd := range buffer.SparseMemoryBindings().All() {
bufSparseBindings = append(bufSparseBindings, bd)
}

sb.write(sb.cb.VkQueueBindSparse(
sparseQueue.VulkanHandle(),
1,
Expand All @@ -1100,7 +1106,7 @@ func (sb *stateBuilder) createBuffer(buffer BufferObjectʳ) {
buffer.VulkanHandle(), // buffer
uint32(buffer.SparseMemoryBindings().Len()), // bindCount
NewVkSparseMemoryBindᶜᵖ( // pBinds
sb.MustUnpackReadMap(buffer.SparseMemoryBindings().All()).Ptr(),
sb.MustAllocReadData(bufSparseBindings).Ptr(),
),
)).Ptr()),
0, // imageOpaqueBindCount
Expand Down Expand Up @@ -1309,46 +1315,64 @@ func (sb *stateBuilder) imageWholeSubresourceRange(img ImageObjectʳ) VkImageSub
}

// IsFullyBound returns true if the resource range from offset with size is
// fully covered in the bindings.
// fully covered in the bindings. If the size ofd the resource range is 0,
// returns false.
func IsFullyBound(offset VkDeviceSize, size VkDeviceSize,
bindings U64ːVkSparseMemoryBindᵐ) bool {
resourceOffsets := bindings.Keys()

oneAfterReqRange := -1
for i := range resourceOffsets {
if resourceOffsets[i] > uint64(offset+size) {
oneAfterReqRange = i
break
}
}
if oneAfterReqRange == -1 || oneAfterReqRange == 0 {
if size == VkDeviceSize(0) {
return false
}
i := oneAfterReqRange - 1

end := offset + size
for i > 0 && end > offset {
resOffset := resourceOffsets[i]
if resOffset+uint64(bindings.Get(resOffset).Size()) >= uint64(end) {
end = VkDeviceSize(resOffset)
i--
resourceOffsets := bindings.Keys()
coveredCounter := 0
addAtBoundaries := map[uint64]int{}
boundaries := make([]uint64, 0, len(resourceOffsets)*2)
for o, b := range bindings.All() {
// Skip the zero sized binds (they are actually invalid)
bindSize := uint64(b.Size())
bindBegin := uint64(o)
bindEnd := bindBegin + bindSize
if bindSize == uint64(0) {
continue
}
return false
// Get the number of binds that cover the start of the requested range.
if bindBegin <= uint64(offset) && bindEnd > uint64(offset) {
coveredCounter++
}
// Count the number of bind begin boundaries in the requested range.
if bindBegin > uint64(offset) && bindBegin < uint64(offset+size) {
boundaries = append(boundaries, bindBegin)
if _, ok := addAtBoundaries[bindBegin]; !ok {
addAtBoundaries[bindBegin] = 0
}
addAtBoundaries[bindBegin]++
}
// Count the number of bind end boundaries in the reqested range
if bindEnd < uint64(offset+size) && bindEnd > uint64(offset) {
boundaries = append(boundaries, bindEnd)
if _, ok := addAtBoundaries[bindEnd]; !ok {
addAtBoundaries[bindEnd] = 0
}
addAtBoundaries[bindEnd]--
}
}
sort.Slice(boundaries, func(i, j int) bool {
return boundaries[i] < boundaries[j]
})

if end <= offset {
return true
// No bind covers the begin of the requested range.
if coveredCounter == 0 {
return false
}

if i == 0 {
resOffset := resourceOffsets[0]
if resOffset <= uint64(offset) &&
resOffset+uint64(bindings.Get(resOffset).Size()) >= uint64(end) {
return true
// Scan through all the boundaries in the requested range. If the coveredCounter
// drops to 0, it means a hole in the requested range.
for _, b := range boundaries {
coveredCounter += addAtBoundaries[b]
if coveredCounter == 0 {
return false
}
}
return false
return true
}

func (sb *stateBuilder) createImage(img ImageObjectʳ, imgPrimer *imagePrimer) {
Expand Down Expand Up @@ -1463,6 +1487,11 @@ func (sb *stateBuilder) createImage(img ImageObjectʳ, imgPrimer *imagePrimer) {
}
}

opaqueSparseBindings := make([]VkSparseMemoryBind, 0, img.OpaqueSparseMemoryBindings().Len())
for _, obd := range img.OpaqueSparseMemoryBindings().All() {
opaqueSparseBindings = append(opaqueSparseBindings, obd)
}

sb.write(sb.cb.VkQueueBindSparse(
sparseQueue.VulkanHandle(),
1,
Expand All @@ -1480,7 +1509,7 @@ func (sb *stateBuilder) createImage(img ImageObjectʳ, imgPrimer *imagePrimer) {
img.VulkanHandle(), // image
uint32(img.OpaqueSparseMemoryBindings().Len()), // bindCount
NewVkSparseMemoryBindᶜᵖ( // pBinds
sb.MustUnpackReadMap(img.OpaqueSparseMemoryBindings().All()).Ptr(),
sb.MustAllocReadData(opaqueSparseBindings).Ptr(),
),
)).Ptr()),
0, // imageBindCount
Expand Down