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

Support external image buffer for add_image() #524

Closed
JerryShih opened this issue Nov 3, 2016 · 4 comments
Closed

Support external image buffer for add_image() #524

JerryShih opened this issue Nov 3, 2016 · 4 comments

Comments

@JerryShih
Copy link
Contributor

Right now, there are some copy in the path from add_image() interface to the final gpu texture uploading in texture_cache module.
The buffer is serialized(so, here is a copy) through the ipc-channel at [1].
If we are playing a video, it's a big performance impact for every frame's copy-op.

In quantum project, I'm trying to pass raw_buffer_ptr through FFI and serialize the raw_ptr using my custom serializer.

The data enum will like:

pub enum ExternalImage {
    // shmem or memory buffer
    MemoryBuff {
        buff: *const u8,  // the raw_buffer ptr from c++
        size: usize,
    }
}

The serialization/deserialization code is at [2].
With this enum, we could have the new interface "AddExternalImage()" in WR and pass the "buff: *const u8" to gl texture with the minimizing copy.
There will have life-time and sync problem for this raw-buffer. If we can handle the the persistence and thread problem in gecko for this buffer, how about this idea if we try to use WR in gecko or other project?

pub enum ApiMsg {
   ....
  AddExternalImage(ImageKey, u32, u32, ImageFormat, ExternalImage),
}

[1]

self.api_sender.send(msg).unwrap();

[2]
https://gist.github.com/JerryShih/2d4b4738fe0d9d399f90300cdbe6c95d

@glennw
Copy link
Member

glennw commented Nov 3, 2016

@JerryShih We definitely want / need to improve this path to reduce extra copies of data. I think your idea above is possible. I have some other thoughts on possible API though, mentioned below.

In your example above, instead of passing a raw pointer to WR, what if you could register a callback with WR though the public API. This callback would have a signature something like:

fn WRImageRequest(key: ImageKey) -> &[u8]

WR would call this from the compositor thread at the exact instance it needs the image data and then pass that slice of data straight to GL.

This would mean the client managing thread-safe access to the data, but would have the benefit of WR never holding on to a pointer.

One downside of such an API is that we need to work out how to handle device lost situations (e.g. on android), but that's somewhat orthogonal to this discussion.

Thoughts?

@jrmuizel
Copy link
Collaborator

jrmuizel commented Nov 3, 2016

With WRImageRequest(key: ImageKey) the callee wouldn't have a great way to know how long to keep the data alive for so we should probably at least have a matching WRImageRelease() or some such thing.

@glennw
Copy link
Member

glennw commented Nov 3, 2016

@jrmuizel Yup, agreed.

@kvark
Copy link
Member

kvark commented Dec 14, 2016

I believe this is fixed by #554. Please feel free to reopen otherwise!

@kvark kvark closed this as completed Dec 14, 2016
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

4 participants