Skip to content

Commit

Permalink
Revert making Raw{Buffer,Image}::bind_memory unsafe (#2595)
Browse files Browse the repository at this point in the history
  • Loading branch information
marc0246 authored Oct 30, 2024
1 parent 726c4be commit a44a778
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 57 deletions.
4 changes: 2 additions & 2 deletions examples/gl-interop/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,9 @@ mod linux {
.export_fd(ExternalMemoryHandleType::OpaqueFd)
.unwrap();

// SAFETY: we just created this raw image and hasn't bound any memory to it.
let image = Arc::new(
unsafe { raw_image.bind_memory([ResourceMemory::new_dedicated(image_memory)]) }
raw_image
.bind_memory([ResourceMemory::new_dedicated(image_memory)])
.map_err(|(err, _, _)| err)
.unwrap(),
);
Expand Down
3 changes: 1 addition & 2 deletions vulkano/src/buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,7 @@ impl Buffer {
.map_err(AllocateBufferError::AllocateMemory)?;
let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) };

// SAFETY: we just created this raw buffer and hasn't bound any memory to it.
let buffer = unsafe { raw_buffer.bind_memory(allocation) }.map_err(|(err, _, _)| {
let buffer = raw_buffer.bind_memory(allocation).map_err(|(err, _, _)| {
err.map(AllocateBufferError::BindMemory)
.map_validation(|err| err.add_context("RawBuffer::bind_memory"))
})?;
Expand Down
28 changes: 14 additions & 14 deletions vulkano/src/buffer/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ impl RawBuffer {
///
/// - `handle` must be a valid Vulkan object handle created from `device`.
/// - `create_info` must match the info used to create the object.
/// - If the buffer has memory bound to it, `bind_memory` must not be called on the returned
/// `RawBuffer`.
#[inline]
pub unsafe fn from_handle(
device: Arc<Device>,
Expand All @@ -119,6 +121,8 @@ impl RawBuffer {
///
/// - `handle` must be a valid Vulkan object handle created from `device`.
/// - `create_info` must match the info used to create the object.
/// - If the buffer has memory bound to it, `bind_memory` must not be called on the returned
/// `RawBuffer`.
/// - Caller must ensure that the handle will not be destroyed for the lifetime of returned
/// `RawBuffer`.
#[inline]
Expand Down Expand Up @@ -241,11 +245,7 @@ impl RawBuffer {
}

/// Binds device memory to this buffer.
///
/// # Safety
///
/// - The buffer must not already have memory bound to it.
pub unsafe fn bind_memory(
pub fn bind_memory(
self,
allocation: ResourceMemory,
) -> Result<Buffer, (Validated<VulkanError>, RawBuffer, ResourceMemory)> {
Expand All @@ -257,15 +257,6 @@ impl RawBuffer {
.map_err(|(err, buffer, allocation)| (err.into(), buffer, allocation))
}

/// Assume this buffer has memory bound to it.
///
/// # Safety
///
/// - The buffer must have memory bound to it.
pub unsafe fn assume_bound(self) -> Buffer {
Buffer::from_raw(self, BufferMemory::External)
}

fn validate_bind_memory(
&self,
allocation: &ResourceMemory,
Expand Down Expand Up @@ -520,6 +511,15 @@ impl RawBuffer {
Ok(Buffer::from_raw(self, BufferMemory::Normal(allocation)))
}

/// Assume this buffer has memory bound to it.
///
/// # Safety
///
/// - The buffer must have memory bound to it.
pub unsafe fn assume_bound(self) -> Buffer {
Buffer::from_raw(self, BufferMemory::External)
}

/// Returns the memory requirements for this buffer.
pub fn memory_requirements(&self) -> &MemoryRequirements {
&self.memory_requirements
Expand Down
2 changes: 1 addition & 1 deletion vulkano/src/image/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl Image {
let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) };

// SAFETY: we just created this raw image and hasn't bound any memory to it.
let image = unsafe { raw_image.bind_memory([allocation]) }.map_err(|(err, _, _)| {
let image = raw_image.bind_memory([allocation]).map_err(|(err, _, _)| {
err.map(AllocateImageError::BindMemory)
.map_validation(|err| err.add_context("RawImage::bind_memory"))
})?;
Expand Down
76 changes: 38 additions & 38 deletions vulkano/src/image/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ impl RawImage {
///
/// - `handle` must be a valid Vulkan object handle created from `device`.
/// - `create_info` must match the info used to create the object.
/// - If the image has memory bound to it, `bind_memory` must not be called on the returned
/// `RawImage`.
#[inline]
pub unsafe fn from_handle(
device: Arc<Device>,
Expand All @@ -145,6 +147,8 @@ impl RawImage {
///
/// - `handle` must be a valid Vulkan object handle created from `device`.
/// - `create_info` must match the info used to create the object.
/// - If the image has memory bound to it, `bind_memory` must not be called on the returned
/// `RawImage`.
/// - Caller must ensure the handle will not be destroyed for the lifetime of returned
/// `RawImage`.
#[inline]
Expand Down Expand Up @@ -475,11 +479,7 @@ impl RawImage {
/// - If `self.flags()` contains `ImageCreateFlags::DISJOINT`, and `self.tiling()` is
/// `ImageTiling::DrmFormatModifier`, then `allocations` must contain exactly
/// `self.drm_format_modifier().unwrap().1` elements.
///
/// # Safety
///
/// - The image must not already have memory bound to it.
pub unsafe fn bind_memory(
pub fn bind_memory(
self,
allocations: impl IntoIterator<Item = ResourceMemory>,
) -> Result<
Expand Down Expand Up @@ -833,39 +833,6 @@ impl RawImage {
Ok(())
}

/// Assume that this image already has memory backing it.
///
/// # Safety
///
/// - The image must be backed by suitable memory allocations.
pub unsafe fn assume_bound(self) -> Image {
let usage = self
.usage
.difference(ImageUsage::TRANSFER_SRC | ImageUsage::TRANSFER_DST);

let layout = if usage.intersects(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT)
&& usage
.difference(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT)
.is_empty()
{
ImageLayout::ShaderReadOnlyOptimal
} else if usage.intersects(ImageUsage::COLOR_ATTACHMENT)
&& usage.difference(ImageUsage::COLOR_ATTACHMENT).is_empty()
{
ImageLayout::ColorAttachmentOptimal
} else if usage.intersects(ImageUsage::DEPTH_STENCIL_ATTACHMENT)
&& usage
.difference(ImageUsage::DEPTH_STENCIL_ATTACHMENT)
.is_empty()
{
ImageLayout::DepthStencilAttachmentOptimal
} else {
ImageLayout::General
};

Image::from_raw(self, ImageMemory::External, layout)
}

/// # Safety
///
/// - If `self.flags()` does not contain `ImageCreateFlags::DISJOINT`, then `allocations` must
Expand Down Expand Up @@ -1011,6 +978,39 @@ impl RawImage {
))
}

/// Assume that this image already has memory backing it.
///
/// # Safety
///
/// - The image must be backed by suitable memory allocations.
pub unsafe fn assume_bound(self) -> Image {
let usage = self
.usage
.difference(ImageUsage::TRANSFER_SRC | ImageUsage::TRANSFER_DST);

let layout = if usage.intersects(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT)
&& usage
.difference(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT)
.is_empty()
{
ImageLayout::ShaderReadOnlyOptimal
} else if usage.intersects(ImageUsage::COLOR_ATTACHMENT)
&& usage.difference(ImageUsage::COLOR_ATTACHMENT).is_empty()
{
ImageLayout::ColorAttachmentOptimal
} else if usage.intersects(ImageUsage::DEPTH_STENCIL_ATTACHMENT)
&& usage
.difference(ImageUsage::DEPTH_STENCIL_ATTACHMENT)
.is_empty()
{
ImageLayout::DepthStencilAttachmentOptimal
} else {
ImageLayout::General
};

Image::from_raw(self, ImageMemory::External, layout)
}

/// Returns the memory requirements for this image.
///
/// - If the image is a swapchain image, this returns a slice with a length of 0.
Expand Down

0 comments on commit a44a778

Please sign in to comment.