Skip to content

Commit 58459fd

Browse files
bors[bot]kvark
bors[bot]
and
kvark
committed
Merge #2178
2178: [mtl] minimize creation of render passes r=grovesNL a=kvark PR checklist: - [x] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [ ] tested examples with the following backends: The logic is changed in the following way: - a framebuffer still keeps a hold of a render pass descriptor, but now behind a mutex - starting a render pass mutates that locked descriptor in place - if a command buffer is deferred, only then we copy the whole descriptor out - careful treatment of image blits/clears is done to re-use the RP descriptor more often Note: doesn't seem to affect Dota framerate considerably Co-authored-by: Dzmitry Malyshau <[email protected]>
2 parents 6cb2a80 + 7ee40d4 commit 58459fd

File tree

3 files changed

+99
-56
lines changed

3 files changed

+99
-56
lines changed

‎src/backend/metal/src/command.rs

+94-54
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,11 @@ impl CommandSink {
777777
CommandSink::Deferred { ref mut passes, ref mut is_encoding } => {
778778
*is_encoding = keep_open;
779779
passes.push(soft::Pass::Render {
780-
desc: descriptor.to_owned(),
780+
//Note: the original descriptor belongs to the framebuffer,
781+
// and will me mutated afterwards.
782+
desc: unsafe {
783+
msg_send![descriptor, copy]
784+
},
781785
commands: init_commands.map(soft::RenderCommand::own).collect(),
782786
});
783787
}
@@ -1596,6 +1600,7 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
15961600

15971601
for subresource_range in subresource_ranges {
15981602
let sub = subresource_range.borrow();
1603+
let descriptor = metal::RenderPassDescriptor::new();
15991604

16001605
let num_layers = (sub.layers.end - sub.layers.start) as u64;
16011606
let layers = if CLEAR_IMAGE_ARRAY {
@@ -1623,9 +1628,54 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
16231628
&*image.raw
16241629
};
16251630

1631+
let clear_color_attachment = sub.aspects.contains(Aspects::COLOR);
1632+
if image.format_desc.aspects.contains(Aspects::COLOR) {
1633+
let attachment = descriptor
1634+
.color_attachments()
1635+
.object_at(0)
1636+
.unwrap();
1637+
attachment.set_texture(Some(texture));
1638+
attachment.set_store_action(metal::MTLStoreAction::Store);
1639+
if clear_color_attachment {
1640+
attachment.set_load_action(metal::MTLLoadAction::Clear);
1641+
attachment.set_clear_color(clear_color.clone());
1642+
} else {
1643+
attachment.set_load_action(metal::MTLLoadAction::Load);
1644+
}
1645+
}
1646+
1647+
let clear_depth_attachment = sub.aspects.contains(Aspects::DEPTH);
1648+
if image.format_desc.aspects.contains(Aspects::DEPTH) {
1649+
let attachment = descriptor
1650+
.depth_attachment()
1651+
.unwrap();
1652+
attachment.set_texture(Some(texture));
1653+
attachment.set_store_action(metal::MTLStoreAction::Store);
1654+
if clear_depth_attachment {
1655+
attachment.set_load_action(metal::MTLLoadAction::Clear);
1656+
attachment.set_clear_depth(depth_stencil.depth as _);
1657+
} else {
1658+
attachment.set_load_action(metal::MTLLoadAction::Load);
1659+
}
1660+
}
1661+
1662+
let clear_stencil_attachment = sub.aspects.contains(Aspects::STENCIL);
1663+
if image.format_desc.aspects.contains(Aspects::STENCIL) {
1664+
let attachment = descriptor
1665+
.stencil_attachment()
1666+
.unwrap();
1667+
attachment.set_texture(Some(texture));
1668+
attachment.set_store_action(metal::MTLStoreAction::Store);
1669+
if clear_stencil_attachment {
1670+
attachment.set_load_action(metal::MTLLoadAction::Clear);
1671+
attachment.set_clear_stencil(depth_stencil.stencil);
1672+
} else {
1673+
attachment.set_load_action(metal::MTLLoadAction::Load);
1674+
}
1675+
}
1676+
16261677
for layer in layers {
16271678
for level in sub.levels.clone() {
1628-
let descriptor = metal::RenderPassDescriptor::new();
16291679
if image.extent.depth > 1 {
16301680
assert_eq!(sub.layers.end, 1);
16311681
let depth = image.extent.at_level(level).depth as u64;
@@ -1634,62 +1684,33 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
16341684
descriptor.set_render_target_array_length(num_layers);
16351685
};
16361686

1637-
let clear_color_attachment = sub.aspects.contains(Aspects::COLOR);
1638-
if clear_color_attachment || image.format_desc.aspects.contains(Aspects::COLOR) {
1687+
if clear_color_attachment {
16391688
let attachment = descriptor
16401689
.color_attachments()
16411690
.object_at(0)
16421691
.unwrap();
1643-
attachment.set_texture(Some(texture));
16441692
attachment.set_level(level as _);
1645-
attachment.set_store_action(metal::MTLStoreAction::Store);
16461693
if !CLEAR_IMAGE_ARRAY {
16471694
attachment.set_slice(layer as _);
16481695
}
1649-
if clear_color_attachment {
1650-
attachment.set_load_action(metal::MTLLoadAction::Clear);
1651-
attachment.set_clear_color(clear_color.clone());
1652-
} else {
1653-
attachment.set_load_action(metal::MTLLoadAction::Load);
1654-
}
16551696
}
1656-
1657-
let clear_depth_attachment = sub.aspects.contains(Aspects::DEPTH);
1658-
if clear_depth_attachment || image.format_desc.aspects.contains(Aspects::DEPTH) {
1697+
if clear_depth_attachment {
16591698
let attachment = descriptor
16601699
.depth_attachment()
16611700
.unwrap();
1662-
attachment.set_texture(Some(texture));
16631701
attachment.set_level(level as _);
1664-
attachment.set_store_action(metal::MTLStoreAction::Store);
16651702
if !CLEAR_IMAGE_ARRAY {
16661703
attachment.set_slice(layer as _);
16671704
}
1668-
if clear_depth_attachment {
1669-
attachment.set_load_action(metal::MTLLoadAction::Clear);
1670-
attachment.set_clear_depth(depth_stencil.depth as _);
1671-
} else {
1672-
attachment.set_load_action(metal::MTLLoadAction::Load);
1673-
}
16741705
}
1675-
1676-
let clear_stencil_attachment = sub.aspects.contains(Aspects::STENCIL);
1677-
if clear_stencil_attachment || image.format_desc.aspects.contains(Aspects::STENCIL) {
1706+
if clear_stencil_attachment {
16781707
let attachment = descriptor
16791708
.stencil_attachment()
16801709
.unwrap();
1681-
attachment.set_texture(Some(texture));
16821710
attachment.set_level(level as _);
1683-
attachment.set_store_action(metal::MTLStoreAction::Store);
16841711
if !CLEAR_IMAGE_ARRAY {
16851712
attachment.set_slice(layer as _);
16861713
}
1687-
if clear_stencil_attachment {
1688-
attachment.set_load_action(metal::MTLLoadAction::Clear);
1689-
attachment.set_clear_stencil(depth_stencil.stencil);
1690-
} else {
1691-
attachment.set_load_action(metal::MTLLoadAction::Load);
1692-
}
16931714
}
16941715

16951716
sink.as_mut()
@@ -2009,6 +2030,27 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
20092030
}
20102031
}
20112032

2033+
let descriptor = metal::RenderPassDescriptor::new();
2034+
if src.format_desc.aspects.contains(Aspects::COLOR) {
2035+
descriptor
2036+
.color_attachments()
2037+
.object_at(0)
2038+
.unwrap()
2039+
.set_texture(Some(&dst.raw));
2040+
}
2041+
if src.format_desc.aspects.contains(Aspects::DEPTH) {
2042+
descriptor
2043+
.depth_attachment()
2044+
.unwrap()
2045+
.set_texture(Some(&dst.raw));
2046+
}
2047+
if src.format_desc.aspects.contains(Aspects::STENCIL) {
2048+
descriptor
2049+
.stencil_attachment()
2050+
.unwrap()
2051+
.set_texture(Some(&dst.raw));
2052+
}
2053+
20122054
let mut inner = self.inner.borrow_mut();
20132055
// Note: we don't bother to restore any render states here, since we are currently
20142056
// outside of a render pass, and the state will be reset automatically once
@@ -2052,7 +2094,7 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
20522094
};
20532095

20542096
for ((aspects, level), list) in vertices.drain() {
2055-
let ext = &dst.extent;
2097+
let ext = dst.extent.at_level(level);
20562098

20572099
let extra = [
20582100
//Note: flipping Y coordinate of the destination here
@@ -2087,29 +2129,25 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
20872129
},
20882130
];
20892131

2090-
let descriptor = metal::RenderPassDescriptor::new();
20912132
descriptor.set_render_target_array_length(ext.depth as _);
20922133
if aspects.contains(Aspects::COLOR) {
2093-
let attachment = descriptor
2134+
descriptor
20942135
.color_attachments()
20952136
.object_at(0)
2096-
.unwrap();
2097-
attachment.set_texture(Some(&dst.raw));
2098-
attachment.set_level(level as _);
2137+
.unwrap()
2138+
.set_level(level as _);
20992139
}
21002140
if aspects.contains(Aspects::DEPTH) {
2101-
let attachment = descriptor
2141+
descriptor
21022142
.depth_attachment()
2103-
.unwrap();
2104-
attachment.set_texture(Some(&dst.raw));
2105-
attachment.set_level(level as _);
2143+
.unwrap()
2144+
.set_level(level as _);
21062145
}
21072146
if aspects.contains(Aspects::STENCIL) {
2108-
let attachment = descriptor
2147+
descriptor
21092148
.stencil_attachment()
2110-
.unwrap();
2111-
attachment.set_texture(Some(&dst.raw));
2112-
attachment.set_level(level as _);
2149+
.unwrap()
2150+
.set_level(level as _);
21132151
}
21142152

21152153
let commands = prelude
@@ -2118,7 +2156,7 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
21182156
.chain(&extra)
21192157
.cloned();
21202158

2121-
inner.sink().begin_render_pass(false, descriptor, commands);
2159+
inner.sink().begin_render_pass(false, &descriptor, commands);
21222160
}
21232161
}
21242162

@@ -2283,11 +2321,13 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
22832321
T: IntoIterator,
22842322
T::Item: Borrow<com::ClearValueRaw>,
22852323
{
2286-
let _ap = AutoreleasePool::new();
22872324
// FIXME: subpasses
2288-
let descriptor: metal::RenderPassDescriptor = unsafe {
2289-
msg_send![framebuffer.descriptor, copy]
2290-
};
2325+
let _ap = AutoreleasePool::new();
2326+
2327+
// we are going to modify the RP descriptor here, so
2328+
// locking to avoid data races.
2329+
let descriptor = framebuffer.descriptor.lock().unwrap();
2330+
22912331
let mut num_colors = 0;
22922332
let mut full_aspects = Aspects::empty();
22932333
let mut inner = self.inner.borrow_mut();
@@ -2346,7 +2386,7 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
23462386
let init_commands = self.state.make_render_commands(full_aspects);
23472387
inner
23482388
.sink()
2349-
.begin_render_pass(true, &descriptor, init_commands);
2389+
.begin_render_pass(true, &*descriptor, init_commands);
23502390
}
23512391

23522392
fn next_subpass(&mut self, _contents: com::SubpassContents) {

‎src/backend/metal/src/device.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,10 @@ impl hal::Device<Backend> for Device {
11691169
}
11701170
}
11711171

1172-
Ok(n::Framebuffer { descriptor, inner })
1172+
Ok(n::Framebuffer {
1173+
descriptor: Mutex::new(descriptor),
1174+
inner,
1175+
})
11731176
}
11741177

11751178
fn create_shader_module(&self, raw_data: &[u8]) -> Result<n::ShaderModule, ShaderError> {

‎src/backend/metal/src/native.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub struct FramebufferInner {
5757

5858
#[derive(Debug)]
5959
pub struct Framebuffer {
60-
pub(crate) descriptor: metal::RenderPassDescriptor,
60+
pub(crate) descriptor: Mutex<metal::RenderPassDescriptor>,
6161
pub(crate) inner: FramebufferInner,
6262
}
6363

0 commit comments

Comments
 (0)