Rework render target allocation #351
Conversation
zakorgy
force-pushed
the
render-target-alloc
branch
from
b4103c2 to
098a20c
Compare
webrender/src/device/gfx/device.rs
Outdated
| @@ -352,6 +352,7 @@ pub struct Device<B: hal::Backend> { | |||
| #[cfg(debug_assertions)] | |||
| shader_is_ready: bool, | |||
| rebind_descriptors: bool, | |||
| linear_memory: LinearMemoryBlock<B>, | |||
webrender/src/device/gfx/device.rs
Outdated
| @@ -1918,6 +1928,7 @@ impl<B: hal::Backend> Device<B> { | |||
| filter: TextureFilter, | |||
| render_target: Option<RenderTargetInfo>, | |||
| layer_count: i32, | |||
| texture_usage: TextureUsage, | |||
There was a problem hiding this comment.
Don't we already know if it's a render target based on the render_target parameter? Does a new parameter help?
There was a problem hiding this comment.
Yes we know that this is a render target from that parameter, but there is a TextureCache texture which is also a render target, but can persist across multiple frames and we can't use the linear allocation for that:
https://github.com/szeged/webrender/blob/master/webrender/src/renderer.rs#L3462
There was a problem hiding this comment.
in this case, can we extend RenderTargetInfo ? perhaps, something as simple as a persistent: bool field would work best.
zakorgy
changed the title
zakorgy
force-pushed
the
render-target-alloc
branch
from
40859ab to
ee93717
Compare
webrender/src/device/gfx/device.rs
Outdated
| @@ -1983,6 +1990,12 @@ impl<B: hal::Backend> Device<B> { | |||
| usage_base | hal::image::Usage::COLOR_ATTACHMENT, | |||
| ), | |||
| }; | |||
|
|
|||
| let (memory, id) = match render_target { | |||
| Some(RenderTargetInfo { has_depth: _, persistent}) if !persistent => (Some(&mut self.render_target_memory), Some(texture.id)), | |||
There was a problem hiding this comment.
could just do RenderTargetInfo { persistent: false, .. }
webrender/src/device/gfx/image.rs
Outdated
| } | ||
| } | ||
|
|
||
| struct LinearMemoryBlock<B: hal::Backend> { |
There was a problem hiding this comment.
isn't all of that implemented by rendy-memory?
webrender/src/device/gfx/image.rs
Outdated
| memory_block: MemoryBlock<B>, | ||
| alignment: u64, | ||
| type_mask: u64, | ||
| free_chunks: Vec<MemoryRange>, |
There was a problem hiding this comment.
My understanding is that the whole point of a linear allocator ("Linear" in the LinearMemoryBlock) that we don't need to track any free chunks, otherwise it's becoming a general-purpose allocator.
There was a problem hiding this comment.
That's true, and that was the case before I introduced the tracking of free chunks(which is required to reuse memory between passes), I just forget to change the name of the allocator and the block. I will change it to avoid further confusion.
This is the first step toward #348 . We allocate a large chunk of linear memory which we can use for render targets between consecutive frames.
Right now the PR just allows us to reuse that chunk between frames, the next step would be reuse it between render passes.This change is