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

Manual buffer management #59

Open
nickbabcock opened this issue Apr 28, 2019 · 3 comments
Open

Manual buffer management #59

nickbabcock opened this issue Apr 28, 2019 · 3 comments

Comments

@nickbabcock
Copy link
Owner

Pfim 0.7.0 introduced IImageAllocator so now code can look like this

IImageAllocator alloc = //
var config = new PfimConfig(allocator: alloc);
using (var image = Pfim.FromStream(stream, config))
{
}

// Can't access image.Data

In #47, @adamkvd mentioned that it would be nice to have an allocator but be able to take ownership of the pixel data. Something along the lines of:

IImageAllocator alloc = //
var config = new PfimConfig(allocator: alloc);
byte[] pixelData;
int dataLen;
using (var image = Pfim.FromStream(stream, config))
{
    pixelData = image.Data;
    dataLen = image.DataLen;
    image.Detach();
}

// do something with pixelData and dataLen.

Before something like this is implemented, I'm curious if a small, straightforward sample can demonstrate the benefits of this override.

@adamkvd
Copy link

adamkvd commented Apr 29, 2019

Before something like this is implemented, I'm curious if a small, straightforward sample can demonstrate the benefits of this override.

We use Pfim for loading textures from DDS files, but we also support loading textures from other image formats, so Pfim is "one library among others". The common denominator is an engine-specific structure that temporarily holds image data. Because of a multi-threaded scenario, this data is only used after Pfim loading has finished. So the use case would be:

  • Main thread schedules image data load.
  • Worker thread picks up scheduled item, loads image data using shared memory pool for required allocations where possible. This may or may not involve Pfim.
  • Worker thread puts loaded image data in the result queue.
  • Main thread picks up image data, uploads it to the GPU, returns image data buffer to the shared memory pool.

To access the loaded data in a world without Detach, I currently see two options on how to process the image data:

  • Carry around the Pfim image, so the data remains valid. This is not great, because there is no place for a Pfim object in our data structure that doesn't really care which library loaded the data, or what needs to be done to shut it down afterwards. Even when hiding it behind an IDisposable, it's a leaked implementation detail of a library that could otherwise remain conveniently scoped to one method.
  • Don't dispose the Pfim image, so the data remains valid. That's not great either, because it's counter-intuitive to not dispose an IDisposable after use, and also prone to custom allocator leaks depending on the implementation of both allocator and Pfim image implementation.

Explicitly taking ownership of the data could be a way around this. Not having Detach is not a blocking issue for us, but having it would remove some design constraints we might otherwise have to work around.

@nickbabcock
Copy link
Owner Author

I've been mulling this over to see if I can modify one of the samples to demonstrate this use case without seeming too contrived, but then I remembered #45 and was curious how you would expect interactions too work. For instance, if the Mipmap data was stored in the same array then it wouldn't complicate this feature (as there still would be only one buffer to manually manage), but if Mipmap was split off then it may open the flood gates for what can be expected of the user to manually manage. 🤔

@adamkvd
Copy link

adamkvd commented May 2, 2019

I'd expect all mipmaps to be stored in a single buffer, preferably in sequence from largest to smallest. When choosing not to decode DDS data, that's to my knowledge already how the (compressed) data will be laid out in the buffer, so the default behavior of decoding the DDS data into uncompressed data would not introduce something entirely new.

For uncompressed data and a known mipmap / image order, it shouldn't be too much of an issue to calculate the data offsets inside the buffer on the user side when needed, but Pfim could at some point provide explicit data layout information for the provided buffer as well.

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

No branches or pull requests

2 participants