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

Adds support for making custom memory allocations and images #252

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ewoudje
Copy link

@ewoudje ewoudje commented Feb 11, 2025

The use I'm using it for is to have external images using a Win32 handle. Because this is very platform specific and niche I instead made a way to import self-made images and memory.

Specifically i exposed the vulkan device, the Allocation struct, allocate_memory and free_memory.
I added 2 functions to the vulkan context for importing memory and textures.
And added select_memory_type as a utility function.

I also added a way to add extensions when creating the context using a new struct PlatformSpecificContextDesc.
Which is used by the new function init_with_platform_desc. The original init function is still the same so it doesn't break anything.

@kvark
Copy link
Owner

kvark commented Feb 13, 2025

Nice! I was working on the exact same thing today in another project and thinking that we need this in Blade.
Let me have a good look at it.

@kvark
Copy link
Owner

kvark commented Feb 17, 2025

I would approach this differently. Instead of exposing the memory infrastructure and device creation facilities, we could allow buffer and texture creation to support external file descriptors. For buffers specifically, the API is very nice:

#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Memory {
    Device,
    Shared,
    Upload,
    External { import_fd: i32 }, // our new variant
}

More generally, we'd even go with import_fd: Option<if32> such as None corresponds to memory that is exported from Blade but not imported from the outside.

For textures, it's a bit less clean since there is no enum like Memory used there. We could do something like this:

struct ExternalMemory {
  import_fd: Option<i32>,
}
struct TextureDesc {
  ...
  external: Option<ExternalMemory>,
}

Note that all the sharing is done via a file descriptor in this case. Besides buffers and textures, there is one more thing that is possibly complicated: synchronization. We need a way to import and export a semaphore.

@ewoudje
Copy link
Author

ewoudje commented Feb 17, 2025

I would approach this differently. Instead of exposing the memory infrastructure and device creation facilities, we could allow buffer and texture creation to support external file descriptors. For buffers specifically, the API is very nice:

#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Memory {
    Device,
    Shared,
    Upload,
    External { import_fd: i32 }, // our new variant
}

More generally, we'd even go with import_fd: Option<if32> such as None corresponds to memory that is exported from Blade but not imported from the outside.

For textures, it's a bit less clean since there is no enum like Memory used there. We could do something like this:

struct ExternalMemory {
  import_fd: Option<i32>,
}
struct TextureDesc {
  ...
  external: Option<ExternalMemory>,
}

Note that all the sharing is done via a file descriptor in this case. Besides buffers and textures, there is one more thing that is possibly complicated: synchronization. We need a way to import and export a semaphore.

How about having in struct TextureDesc a field for the enum Memory?
It can then simply be fed into let allocation = self.allocate_memory(requirements, crate::Memory::Device); replacing the hardcoded device memory part.

@ewoudje
Copy link
Author

ewoudje commented Feb 17, 2025

It seems github closed it because i discarded previous commits

I hope the latest commit is more on track?
For the Memory:External should i add a enum that defines the external source instead of just a isize that corresponds to fd or handle?
Since the source could also be from directx, metal, etc

@ewoudje ewoudje reopened this Feb 17, 2025
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.

2 participants