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

Revert making Raw{Buffer,Image}::bind_memory unsafe #2595

Merged
merged 1 commit into from
Oct 30, 2024
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
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