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

Conversation

Qining
Copy link
Contributor

@Qining Qining commented Jun 7, 2018

Three issues fixed:

  1. The opaque sparse memory binding info cached in Buffer and Image
    object should not be fed to MustUnpackReadMap(), as they are actually
    mapping from offsets to info, not a list of info.

  2. IsFullyBound() does not work correctly.

  3. Since the creation of ImageLevel.Data() slice has been moved from
    vkCreateImage to vkBindImageMemory, for sparse binding images, the Data
    slice is empty. For now we just check the size of the Data slice for
    each time a sparse binding is to be submitted and if the Data slice is
    empty, create the shadow memory Data slice for each image level.

Three issues fixed:

1) The opaque sparse memory binding info cached in Buffer and Image
object should not be fed to MustUnpackReadMap(), as they are actually
mapping from offsets to info, not a list of info.

2) IsFullyBound() does not work correctly.

3) Since the creation of ImageLevel.Data() slice has been moved from
vkCreateImage to vkBindImageMemory, for sparse binding images, the Data
slice is empty. For now we just check the size of the Data slice for
each time a sparse binding is to be submitted and if the Data slice is
empty, create the shadow memory Data slice for each image level.
@Qining Qining requested a review from AWoloszyn June 7, 2018 22:37
@Qining
Copy link
Contributor Author

Qining commented Jun 7, 2018

@iburinoc Please take a look, hope this fixes the issue with sparse bindings for now.

@sean-purcell
Copy link
Contributor

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.

@Qining Qining merged commit 26ed8d7 into google:master Jun 8, 2018
@Qining Qining deleted the fix-sparse-binding-crash branch October 23, 2018 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants