From 5340e094dd7de17b2060d99eb011ad7f00e8c55c Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 26 Jun 2018 09:53:54 -0400 Subject: [PATCH 1/5] [mtl] descriptor management rework to get the real pool semantics --- src/backend/metal/src/command.rs | 344 +++++++++++++++---------------- src/backend/metal/src/device.rs | 144 +++++++------ src/backend/metal/src/native.rs | 309 ++++++++++++++++----------- src/hal/src/pso/descriptor.rs | 1 + 4 files changed, 438 insertions(+), 360 deletions(-) diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index eed0ddb2d30..24dcf2b1b10 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -597,6 +597,9 @@ enum CommandSink { }, } +//TODO: scope-based command submission, to avoid doing any work (including just storing the command lists) +// if they are going to be discarded (e.g. by `pre_render_commands`) + impl CommandSink { /// Issue provided (state-setting) commands only when there is already /// a render pass being actively encoded. @@ -2468,7 +2471,7 @@ impl com::RawCommandBuffer for CommandBuffer { fn bind_graphics_descriptor_sets<'a, I, J>( &mut self, - layout: &native::PipelineLayout, + pipe_layout: &native::PipelineLayout, first_set: usize, sets: I, offsets: J, @@ -2481,118 +2484,109 @@ impl com::RawCommandBuffer for CommandBuffer { use spirv_cross::{msl, spirv}; let mut offset_iter = offsets.into_iter(); + let mut dynamic_offsets = SmallVec::<[u64; 16]>::new(); for (set_index, desc_set) in sets.into_iter().enumerate() { match *desc_set.borrow() { - native::DescriptorSet::Emulated(ref desc_inner) => { - use native::DescriptorSetBinding::*; - - let set = desc_inner.lock().unwrap(); - let mut commands = Vec::with_capacity( - set.bindings - .iter() - .map(|values| values.as_ref().map_or(0, |v| v.count())) - .sum() - ); - let bindings = set.bindings - .iter() - .enumerate() - .filter_map(|(binding, values)| values.as_ref().map(|v| (binding as u32, v))); - - for (binding, values) in bindings { - let desc_layout = set.layout.iter().find(|x| x.binding == binding).unwrap(); - - let mut bind_stages = SmallVec::<[_; 2]>::new(); - if desc_layout.stage_flags.contains(pso::ShaderStageFlags::VERTEX) { + native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { + let mut commands = Vec::new(); + let pool = pool.read().unwrap(); + let mut sampler_base = sampler_range.start as usize; + let mut texture_base = texture_range.start as usize; + let mut buffer_base = buffer_range.start as usize; + + for layout in layouts.iter() { + let sm_range = sampler_base .. sampler_base + layout.count; + let tx_range = texture_base .. texture_base + layout.count; + let bf_range = buffer_base .. buffer_base + layout.count; + native::DescriptorPool::count_bindings(layout.ty, layout.count, + &mut sampler_base, &mut texture_base, &mut buffer_base); + + if buffer_base != bf_range.start { + dynamic_offsets.clear(); + for bref in &pool.buffers[bf_range.clone()] { + if bref.base.is_some() && bref.dynamic { + dynamic_offsets.push(*offset_iter + .next() + .expect("No dynamic offset provided!") + .borrow() as u64 + ); + } + } + } + + // collect the binding stages + let bind_vs = if layout.stage_flags.contains(pso::ShaderStageFlags::VERTEX) { let loc = msl::ResourceBindingLocation { stage: spirv::ExecutionModel::Vertex, desc_set: (first_set + set_index) as _, - binding: binding as _, + binding: layout.binding as _, }; - bind_stages.push((pso::Stage::Vertex, loc, &mut self.state.resources_vs)); - } - if desc_layout.stage_flags.contains(pso::ShaderStageFlags::FRAGMENT) { + Some((pso::Stage::Vertex, loc, &mut self.state.resources_vs)) + } else { + None + }; + let bind_fs = if layout.stage_flags.contains(pso::ShaderStageFlags::FRAGMENT) { let loc = msl::ResourceBindingLocation { stage: spirv::ExecutionModel::Fragment, desc_set: (first_set + set_index) as _, - binding: binding as _, + binding: layout.binding as _, }; - bind_stages.push((pso::Stage::Fragment, loc, &mut self.state.resources_fs)); - } + Some((pso::Stage::Fragment, loc, &mut self.state.resources_fs)) + } else { + None + }; - match values { - Sampler(ref samplers) => { - for &mut (stage, ref loc, ref mut resources) in &mut bind_stages { - let start = layout.res_overrides[loc].sampler_id as usize; - resources.add_samplers(start, samplers.as_slice()); - commands.extend(samplers.iter().enumerate().map(|(i, sampler)| { - soft::RenderCommand::BindSampler { - stage, - index: start + i, - sampler: *sampler, - } - })); - } - } - Image(ref images) => { - for &mut (stage, ref loc, ref mut resources) in &mut bind_stages { - let start = layout.res_overrides[loc].texture_id as usize; - resources.add_textures(start, images.as_slice()); - commands.extend(images.iter().enumerate().map(|(i, texture)| { - soft::RenderCommand::BindTexture { - stage, - index: start + i, - texture: texture.map(|(root, _)| root), - } - })); - } + for (stage, loc, resources) in bind_vs.into_iter().chain(bind_fs) { + if sampler_base != sm_range.start { + debug_assert_eq!(sampler_base, sm_range.end); + let samplers = &pool.samplers[sm_range.clone()]; + let start = pipe_layout.res_overrides[&loc].sampler_id as usize; + resources.add_samplers(start, samplers); + commands.extend(samplers.iter().enumerate().map(|(i, &sampler)| { + soft::RenderCommand::BindSampler { + stage, + index: start + i, + sampler, + } + })); } - Combined(ref combos) => { - for &mut (stage, ref loc, ref mut resources) in &mut bind_stages { - let start_tx = layout.res_overrides[loc].texture_id as usize; - let start_sm = layout.res_overrides[loc].sampler_id as usize; - for (i, (texture, sampler)) in combos.iter().enumerate() { - resources.add_textures(start_tx + i, &[texture.clone()]); - resources.add_samplers(start_sm + i, &[sampler.clone()]); - commands.push(soft::RenderCommand::BindTexture { - stage, - index: start_tx + i, - texture: texture.map(|(root, _)| root), - }); - commands.push(soft::RenderCommand::BindSampler { - stage, - index: start_sm + i, - sampler: *sampler, - }); + if texture_base != tx_range.start { + debug_assert_eq!(texture_base, tx_range.end); + let textures = &pool.textures[tx_range.clone()]; + let start = pipe_layout.res_overrides[&loc].texture_id as usize; + resources.add_textures(start, textures); + commands.extend(textures.iter().enumerate().map(|(i, texture)| { + soft::RenderCommand::BindTexture { + stage, + index: start + i, + texture: texture.map(|(tex, _)| tex), } - } + })); } - Buffer(ref buffers) => { + if buffer_base != bf_range.start { + debug_assert_eq!(buffer_base, bf_range.end); + let buffers = &pool.buffers[bf_range.clone()]; + let start = pipe_layout.res_overrides[&loc].buffer_id as usize; + let mut dynamic_index = 0; for (i, bref) in buffers.iter().enumerate() { let (buffer, offset) = match bref.base { Some((buffer, mut offset)) => { if bref.dynamic { - offset += *offset_iter - .next() - .expect("No dynamic offset provided!") - .borrow() as u64; + offset += dynamic_offsets[dynamic_index]; + dynamic_index += 1; } + resources.add_buffer(start + i, buffer, offset as _); (Some(buffer), offset) } None => (None, 0), }; - for &mut (stage, ref loc, ref mut resources) in &mut bind_stages { - let start = layout.res_overrides[loc].buffer_id as usize; - if let Some(buffer) = buffer { - resources.add_buffer(start + i, buffer, offset as _); - } - commands.push(soft::RenderCommand::BindBuffer { - stage, - index: start + i, - buffer, - offset, - }); - } + commands.push(soft::RenderCommand::BindBuffer { + stage, + index: start + i, + buffer, + offset, + }); } } } @@ -2610,7 +2604,7 @@ impl com::RawCommandBuffer for CommandBuffer { desc_set: (first_set + set_index) as _, binding: 0, }; - let slot = layout.res_overrides[&loc].buffer_id; + let slot = pipe_layout.res_overrides[&loc].buffer_id; self.state.resources_vs.add_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _); Some(soft::RenderCommand::BindBuffer { stage: pso::Stage::Vertex, @@ -2627,7 +2621,7 @@ impl com::RawCommandBuffer for CommandBuffer { desc_set: (first_set + set_index) as _, binding: 0, }; - let slot = layout.res_overrides[&loc].buffer_id; + let slot = pipe_layout.res_overrides[&loc].buffer_id; self.state.resources_fs.add_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _); Some(soft::RenderCommand::BindBuffer { stage: pso::Stage::Fragment, @@ -2662,7 +2656,7 @@ impl com::RawCommandBuffer for CommandBuffer { fn bind_compute_descriptor_sets<'a, I, J>( &mut self, - layout: &native::PipelineLayout, + pipe_layout: &native::PipelineLayout, first_set: usize, sets: I, offsets: J, @@ -2675,109 +2669,105 @@ impl com::RawCommandBuffer for CommandBuffer { use spirv_cross::{msl, spirv}; let mut offset_iter = offsets.into_iter(); - let mut inner = self.inner.borrow_mut(); + let mut dynamic_offsets = SmallVec::<[u64; 16]>::new(); for (set_index, desc_set) in sets.into_iter().enumerate() { let resources = &mut self.state.resources_cs; - let location_cs = msl::ResourceBindingLocation { + let res_override = &pipe_layout.res_overrides[&msl::ResourceBindingLocation { stage: spirv::ExecutionModel::GlCompute, desc_set: (first_set + set_index) as _, binding: 0, - }; + }]; match *desc_set.borrow() { - native::DescriptorSet::Emulated(ref desc_inner) => { - use native::DescriptorSetBinding::*; - - let set = desc_inner.lock().unwrap(); - let mut commands = Vec::with_capacity( - set.bindings - .iter() - .map(|values| values.as_ref().map_or(0, |v| v.count())) - .sum() - ); - let bindings = set.bindings - .iter() - .enumerate() - .filter_map(|(binding, values)| values.as_ref().map(|v| (binding as u32, v))); - - for (binding, values) in bindings { - let desc_layout = set.layout.iter().find(|x| x.binding == binding).unwrap(); - - if desc_layout.stage_flags.contains(pso::ShaderStageFlags::COMPUTE) { - let res = &layout.res_overrides[&msl::ResourceBindingLocation { - binding: binding as _, - .. location_cs - }]; - match values { - Sampler(ref samplers) => { - let start = res.sampler_id as usize; - resources.add_samplers(start, samplers.as_slice()); - commands.extend(samplers.iter().enumerate().map(|(i, sampler)| { - soft::ComputeCommand::BindSampler { - index: start + i, - sampler: *sampler, - } - })); + native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { + let mut commands = Vec::new(); + let pool = pool.read().unwrap(); + let mut sampler_base = sampler_range.start as usize; + let mut texture_base = texture_range.start as usize; + let mut buffer_base = buffer_range.start as usize; + + for layout in layouts.iter() { + let sm_range = sampler_base .. sampler_base + layout.count; + let tx_range = texture_base .. texture_base + layout.count; + let bf_range = buffer_base .. buffer_base + layout.count; + native::DescriptorPool::count_bindings(layout.ty, layout.count, + &mut sampler_base, &mut texture_base, &mut buffer_base); + + if buffer_base != bf_range.start { + dynamic_offsets.clear(); + for bref in &pool.buffers[bf_range.clone()] { + if bref.base.is_some() && bref.dynamic { + dynamic_offsets.push(*offset_iter + .next() + .expect("No dynamic offset provided!") + .borrow() as u64 + ); } - Image(ref images) => { - let start = res.texture_id as usize; - resources.add_textures(start, images.as_slice()); - commands.extend(images.iter().enumerate().map(|(i, texture)| { - soft::ComputeCommand::BindTexture { - index: start + i, - texture: texture.map(|(root, _)| root), - } - })); + } + } + + if sampler_base != sm_range.start { + debug_assert_eq!(sampler_base, sm_range.end); + let samplers = &pool.samplers[sm_range]; + let start = res_override.sampler_id as usize; + resources.add_samplers(start, samplers); + commands.extend(samplers.iter().enumerate().map(|(i, &sampler)| { + soft::ComputeCommand::BindSampler { + index: start + i, + sampler, } - Combined(ref combos) => { - for (i, (texture, sampler)) in combos.iter().enumerate() { - let id_tx = res.texture_id as usize + i; - let id_sm = res.sampler_id as usize + i; - resources.add_textures(id_tx, &[texture.clone()]); - resources.add_samplers(id_sm, &[sampler.clone()]); - commands.push(soft::ComputeCommand::BindTexture { - index: id_tx, - texture: texture.map(|(root, _)| root), - }); - commands.push(soft::ComputeCommand::BindSampler { - index: id_sm, - sampler: *sampler, - }); - } + })); + } + if texture_base != tx_range.start { + debug_assert_eq!(texture_base, tx_range.end); + let textures = &pool.textures[tx_range]; + let start = res_override.texture_id as usize; + resources.add_textures(start, textures); + commands.extend(textures.iter().enumerate().map(|(i, texture)| { + soft::ComputeCommand::BindTexture { + index: start + i, + texture: texture.map(|(tex, _)| tex), } - Buffer(ref buffers) => { - let start = res.buffer_id as usize; - for (i, bref) in buffers.iter().enumerate() { - let (buffer, offset) = match bref.base { - Some((buffer, mut offset)) => { - if bref.dynamic { - offset += *offset_iter - .next() - .expect("No dynamic offset provided!") - .borrow() as u64; - } - resources.add_buffer(start + i, buffer, offset as _); - (Some(buffer), offset) - }, - None => (None, 0), - }; - commands.push(soft::ComputeCommand::BindBuffer { - index: start + i, - buffer, - offset, - }); + })); + } + if buffer_base != bf_range.start { + debug_assert_eq!(buffer_base, bf_range.end); + let buffers = &pool.buffers[bf_range]; + let start = res_override.buffer_id as usize; + let mut dynamic_index = 0; + for (i, bref) in buffers.iter().enumerate() { + let (buffer, offset) = match bref.base { + Some((buffer, mut offset)) => { + if bref.dynamic { + offset += dynamic_offsets[dynamic_index]; + dynamic_index += 1; + } + resources.add_buffer(start + i, buffer, offset as _); + (Some(buffer), offset) } - } + None => (None, 0), + }; + commands.push(soft::ComputeCommand::BindBuffer { + index: start + i, + buffer, + offset, + }); } } } - inner.sink().pre_compute_commands(commands); + self.inner + .borrow_mut() + .sink() + .pre_compute_commands(commands); } native::DescriptorSet::ArgumentBuffer { ref raw, offset, stage_flags, .. } => { if stage_flags.contains(pso::ShaderStageFlags::COMPUTE) { - let slot = layout.res_overrides[&location_cs].buffer_id; - resources.add_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _); + resources.add_buffer( + res_override.buffer_id as _, + BufferPtr(raw.as_ptr()), + offset as _, + ); } } } diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 62b1ea5fd97..a8db0c2e9fb 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -9,7 +9,7 @@ use std::borrow::Borrow; use std::collections::hash_map::{Entry, HashMap}; use std::ops::Range; use std::path::Path; -use std::sync::{Arc, Condvar, Mutex}; +use std::sync::{Arc, Condvar, Mutex, RwLock}; use std::{cmp, mem, slice, time}; use hal::{self, error, image, pass, format, mapping, memory, buffer, pso, query, window}; @@ -731,7 +731,7 @@ impl hal::Device for Device { for (set_index, set_layout) in set_layouts.into_iter().enumerate() { match set_layout.borrow() { &n::DescriptorSetLayout::Emulated(ref set_bindings, _) => { - for set_binding in set_bindings { + for set_binding in set_bindings.iter() { for &mut(stage_bit, stage, ref mut counters) in stage_infos.iter_mut() { if !set_binding.stage_flags.contains(stage_bit) { continue @@ -1350,37 +1350,45 @@ impl hal::Device for Device { I: IntoIterator, I::Item: Borrow, { - if !self.private_caps.argument_buffers { - return n::DescriptorPool::Emulated; - } - let mut num_samplers = 0; let mut num_textures = 0; - let mut num_uniforms = 0; - - let arguments = descriptor_ranges.into_iter().map(|desc| { - let desc = desc.borrow(); - let offset_ref = match desc.ty { - pso::DescriptorType::Sampler => &mut num_samplers, - pso::DescriptorType::SampledImage => &mut num_textures, - pso::DescriptorType::UniformBuffer | pso::DescriptorType::StorageBuffer => &mut num_uniforms, - _ => unimplemented!() - }; - let index = *offset_ref; - *offset_ref += desc.count; - Self::describe_argument(desc.ty, index as _, desc.count) - }).collect::>(); + let mut num_buffers = 0; - let device = self.shared.device.lock().unwrap(); - let arg_array = metal::Array::from_owned_slice(&arguments); - let encoder = device.new_argument_encoder(&arg_array); + if self.private_caps.argument_buffers { + let mut arguments = Vec::new(); + for desc_range in descriptor_ranges { + let desc = desc_range.borrow(); + let offset_ref = match desc.ty { + pso::DescriptorType::Sampler => &mut num_samplers, + pso::DescriptorType::SampledImage => &mut num_textures, + pso::DescriptorType::UniformBuffer | pso::DescriptorType::StorageBuffer => &mut num_buffers, + _ => unimplemented!() + }; + let index = *offset_ref; + *offset_ref += desc.count; + let arg_desc = Self::describe_argument(desc.ty, index as _, desc.count); + arguments.push(arg_desc); + } - let total_size = encoder.encoded_length(); - let raw = device.new_buffer(total_size, MTLResourceOptions::empty()); + let device = self.shared.device.lock().unwrap(); + let arg_array = metal::Array::from_owned_slice(&arguments); + let encoder = device.new_argument_encoder(&arg_array); - n::DescriptorPool::ArgumentBuffer { - raw, - range_allocator: RangeAllocator::new(0..total_size), + let total_size = encoder.encoded_length(); + let raw = device.new_buffer(total_size, MTLResourceOptions::empty()); + + n::DescriptorPool::ArgumentBuffer { + raw, + range_allocator: RangeAllocator::new(0..total_size), + } + } else { + for desc_range in descriptor_ranges { + let desc = desc_range.borrow(); + n::DescriptorPool::count_bindings(desc.ty, desc.count, + &mut num_samplers, &mut num_textures, &mut num_buffers); + } + let inner = n::DescriptorPoolInner::new(num_samplers, num_textures, num_buffers); + n::DescriptorPool::Emulated(Arc::new(RwLock::new(inner))) } } @@ -1408,11 +1416,15 @@ impl hal::Device for Device { n::DescriptorSetLayout::ArgumentBuffer(encoder, stage_flags) } else { + //TODO: if we always process the layout bindings in the order of binding points, + // the spill logic becomes trivial. Problem is - keeping track of immutable samplers. n::DescriptorSetLayout::Emulated( - binding_iter - .into_iter() - .map(|b| b.borrow().clone()) - .collect(), + Arc::new( + binding_iter + .into_iter() + .map(|b| b.borrow().clone()) + .collect() + ), immutable_sampler_iter .into_iter() .map(|is| is.borrow().0.clone()) @@ -1429,52 +1441,60 @@ impl hal::Device for Device { { for write in write_iter { match *write.set { - n::DescriptorSet::Emulated(ref inner) => { - let mut set = inner.lock().unwrap(); + n::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { let mut array_offset = write.array_offset; let mut binding = write.binding; + let mut pool = pool.write().unwrap(); for descriptor in write.descriptors { - while array_offset >= set.layout.iter() - .find(|layout| layout.binding == binding) - .expect("invalid descriptor set binding index") - .count - { + let mut layout; + let mut sampler_index; + let mut texture_index; + let mut buffer_index; + loop { + sampler_index = sampler_range.start as usize + array_offset; + texture_index = texture_range.start as usize + array_offset; + buffer_index = buffer_range.start as usize + array_offset; + //TODO: can pre-compute this + layout = layouts.iter() + .find(|layout| { + if layout.binding == binding { + true + } else { + n::DescriptorPool::count_bindings(layout.ty, layout.count, + &mut sampler_index, &mut texture_index, &mut buffer_index); + false + } + }) + .expect("invalid descriptor set binding index"); + if array_offset < layout.count { + break; + } array_offset = 0; binding += 1; } - match (descriptor.borrow(), set.bindings[binding as usize].as_mut().unwrap()) { - (&pso::Descriptor::Sampler(sampler), &mut n::DescriptorSetBinding::Sampler(ref mut vec)) => { - vec[array_offset] = Some(SamplerPtr(sampler.0.as_ptr())); - } - (&pso::Descriptor::Image(image, layout), &mut n::DescriptorSetBinding::Image(ref mut vec)) => { - vec[array_offset] = Some((TexturePtr(image.raw.as_ptr()), layout)); + match *descriptor.borrow() { + pso::Descriptor::Sampler(sampler) => { + pool.samplers[sampler_index] = Some(SamplerPtr(sampler.0.as_ptr())); } - (&pso::Descriptor::Image(image, layout), &mut n::DescriptorSetBinding::Combined(ref mut vec)) => { - vec[array_offset].0 = Some((TexturePtr(image.raw.as_ptr()), layout)); + pso::Descriptor::Image(image, layout) => { + pool.textures[texture_index] = Some((TexturePtr(image.raw.as_ptr()), layout)); } - (&pso::Descriptor::CombinedImageSampler(image, layout, sampler), &mut n::DescriptorSetBinding::Combined(ref mut vec)) => { - vec[array_offset] = (Some((TexturePtr(image.raw.as_ptr()), layout)), Some(SamplerPtr(sampler.0.as_ptr()))); + pso::Descriptor::CombinedImageSampler(image, layout, sampler) => { + pool.samplers[sampler_index] = Some(SamplerPtr(sampler.0.as_ptr())); + pool.textures[texture_index] = Some((TexturePtr(image.raw.as_ptr()), layout)); } - (&pso::Descriptor::UniformTexelBuffer(view), &mut n::DescriptorSetBinding::Image(ref mut vec)) | - (&pso::Descriptor::StorageTexelBuffer(view), &mut n::DescriptorSetBinding::Image(ref mut vec)) => { - vec[array_offset] = Some((TexturePtr(view.raw.as_ptr()), image::Layout::General)); + pso::Descriptor::UniformTexelBuffer(view) | + pso::Descriptor::StorageTexelBuffer(view) => { + pool.textures[texture_index] = Some((TexturePtr(view.raw.as_ptr()), image::Layout::General)); } - (&pso::Descriptor::Buffer(buffer, ref range), &mut n::DescriptorSetBinding::Buffer(ref mut vec)) => { + pso::Descriptor::Buffer(buffer, ref range) => { let buf_length = buffer.raw.length(); let start = range.start.unwrap_or(0); let end = range.end.unwrap_or(buf_length); assert!(end <= buf_length); - vec[array_offset].base = Some((BufferPtr(buffer.raw.as_ptr()), start)); - } - (&pso::Descriptor::Sampler(..), _) | - (&pso::Descriptor::Image(..), _) | - (&pso::Descriptor::CombinedImageSampler(..), _) | - (&pso::Descriptor::Buffer(..), _) | - (&pso::Descriptor::UniformTexelBuffer(..), _) | - (&pso::Descriptor::StorageTexelBuffer(..), _) => { - panic!("mismatched descriptor set type") + pool.buffers[buffer_index].base = Some((BufferPtr(buffer.raw.as_ptr()), start)); } } } diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 5328ac59d5e..67cd49721ee 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -5,7 +5,7 @@ use window::SwapchainImage; use std::collections::HashMap; use std::ops::Range; use std::os::raw::{c_void, c_long}; -use std::sync::{Arc, Condvar, Mutex}; +use std::sync::{Arc, Condvar, Mutex, RwLock}; use hal::{self, image, pso}; use hal::backend::FastHashMap; @@ -230,7 +230,7 @@ unsafe impl Sync for Buffer {} #[derive(Debug)] pub enum DescriptorPool { - Emulated, + Emulated(Arc>), ArgumentBuffer { raw: metal::Buffer, range_allocator: RangeAllocator, @@ -240,82 +240,141 @@ pub enum DescriptorPool { unsafe impl Send for DescriptorPool {} unsafe impl Sync for DescriptorPool {} +#[derive(Clone, Debug)] +pub struct BufferBinding { + pub base: Option<(BufferPtr, u64)>, + pub dynamic: bool, +} + +#[derive(Debug)] +pub struct DescriptorPoolInner { + pub samplers: Vec>, + sampler_alloc: RangeAllocator, + pub textures: Vec>, + texture_alloc: RangeAllocator, + pub buffers: Vec, + buffer_alloc: RangeAllocator, +} + +impl DescriptorPoolInner { + pub fn new(num_samplers: usize, num_textures: usize, num_buffers: usize) -> Self { + DescriptorPoolInner { + samplers: vec![None; num_samplers], + sampler_alloc: RangeAllocator::new(0 .. num_samplers as pso::DescriptorBinding), + textures: vec![None; num_textures], + texture_alloc: RangeAllocator::new(0 .. num_textures as pso::DescriptorBinding), + buffers: vec![BufferBinding { base: None, dynamic: false }; num_buffers], + buffer_alloc: RangeAllocator::new(0 .. num_buffers as pso::DescriptorBinding), + } + } +} + +impl DescriptorPool { + pub(crate) fn count_bindings( + desc_type: pso::DescriptorType, + desc_count: usize, + num_samplers: &mut usize, + num_textures: &mut usize, + num_buffers: &mut usize, + ) { + match desc_type { + pso::DescriptorType::Sampler => { + *num_samplers += desc_count; + } + pso::DescriptorType::CombinedImageSampler => { + *num_samplers += desc_count; + *num_textures += desc_count; + } + pso::DescriptorType::SampledImage | + pso::DescriptorType::StorageImage | + pso::DescriptorType::UniformTexelBuffer | + pso::DescriptorType::StorageTexelBuffer | + pso::DescriptorType::InputAttachment => { + *num_textures += desc_count; + } + pso::DescriptorType::UniformBuffer | + pso::DescriptorType::StorageBuffer | + pso::DescriptorType::UniformBufferDynamic | + pso::DescriptorType::StorageBufferDynamic => { + *num_buffers += desc_count; + } + }; + } +} + impl hal::DescriptorPool for DescriptorPool { - fn allocate_set(&mut self, layout: &DescriptorSetLayout) -> Result { + fn allocate_set(&mut self, set_layout: &DescriptorSetLayout) -> Result { match *self { - DescriptorPool::Emulated => { - let (layout_bindings, immutable_samplers) = match layout { + DescriptorPool::Emulated(ref pool_inner) => { + let (layout_bindings, immutable_samplers) = match set_layout { &DescriptorSetLayout::Emulated(ref bindings, ref samplers) => (bindings, samplers), _ => return Err(pso::AllocationError::IncompatibleLayout), }; - let mut sampler_offset = 0; - - // Assume some reasonable starting capacity - let mut bindings = Vec::with_capacity(layout_bindings.len()); + // step[1]: count the total number of descriptors needed + let mut total_samplers = 0; + let mut total_textures = 0; + let mut total_buffers = 0; for layout in layout_bindings.iter() { - let binding = match layout.ty { - pso::DescriptorType::Sampler => { - DescriptorSetBinding::Sampler(if layout.immutable_samplers { - let slice = &immutable_samplers[sampler_offset.. sampler_offset + layout.count]; - sampler_offset += layout.count; - slice - .iter() - .map(|s| Some(SamplerPtr(s.as_ptr()))) - .collect() - } else { - vec![None; layout.count] - }) - } - pso::DescriptorType::CombinedImageSampler => { - DescriptorSetBinding::Combined(if layout.immutable_samplers { - let slice = &immutable_samplers[sampler_offset.. sampler_offset + layout.count]; - sampler_offset += layout.count; - slice - .iter() - .map(|s| (None, Some(SamplerPtr(s.as_ptr())))) - .collect() - } else { - vec![(None, None); layout.count] - }) - } - pso::DescriptorType::SampledImage | - pso::DescriptorType::StorageImage | - pso::DescriptorType::UniformTexelBuffer | - pso::DescriptorType::StorageTexelBuffer | - pso::DescriptorType::InputAttachment => { - DescriptorSetBinding::Image(vec![None; layout.count]) - } - pso::DescriptorType::UniformBuffer | - pso::DescriptorType::StorageBuffer => { - DescriptorSetBinding::Buffer(vec![BufferBinding { base: None, dynamic: false }; layout.count]) - } - pso::DescriptorType::UniformBufferDynamic | - pso::DescriptorType::StorageBufferDynamic => { - DescriptorSetBinding::Buffer(vec![BufferBinding { base: None, dynamic: true }; layout.count]) - } - }; - - let layout_binding = layout.binding as usize; + Self::count_bindings(layout.ty, layout.count, + &mut total_samplers, &mut total_textures, &mut total_buffers); + } - if bindings.len() <= layout_binding { - bindings.resize(layout_binding + 1, None); + // step[2]: try to allocate the ranges from the pool + let mut inner = pool_inner.write().unwrap(); + let sampler_range = match inner.sampler_alloc.allocate_range(total_samplers as _) { + Some(range) => range, + None => { + warn!("Not enough samplers for {}", total_samplers); + return Err(pso::AllocationError::FragmentedPool); + } + }; + let texture_range = match inner.texture_alloc.allocate_range(total_textures as _) { + Some(range) => range, + None => { + inner.sampler_alloc.free_range(sampler_range); + warn!("Not enough images for {}", total_textures); + return Err(pso::AllocationError::FragmentedPool); + } + }; + let buffer_range = match inner.buffer_alloc.allocate_range(total_buffers as _) { + Some(range) => range, + None => { + inner.sampler_alloc.free_range(sampler_range); + inner.texture_alloc.free_range(texture_range); + warn!("Not enough buffers for {}", total_buffers); + return Err(pso::AllocationError::FragmentedPool); } + }; - bindings[layout_binding] = Some(binding); + // step[3]: fill out immutable samplers + let mut immutable_sampler_offset = 0; + let mut sampler_offset = sampler_range.start as usize; + for layout in layout_bindings.iter() { + if layout.immutable_samplers { + for (sampler, immutable) in inner + .samplers[sampler_offset .. sampler_offset + layout.count] + .iter_mut() + .zip(&immutable_samplers[immutable_sampler_offset..]) + { + *sampler = Some(SamplerPtr(immutable.as_ptr())) + } + immutable_sampler_offset += layout.count; + } + let (mut tx_temp, mut bf_temp) = (0, 0); + Self::count_bindings(layout.ty, layout.count, &mut sampler_offset, &mut tx_temp, &mut bf_temp); } - // The set may be held onto for a long time, so attempt to shrink to avoid large overallocations - bindings.shrink_to_fit(); - - let inner = DescriptorSetInner { - layout: layout_bindings.to_vec(), - bindings, - }; - Ok(DescriptorSet::Emulated(Arc::new(Mutex::new(inner)))) + Ok(DescriptorSet::Emulated { + pool: Arc::clone(pool_inner), + layouts: Arc::clone(layout_bindings), + sampler_range, + texture_range, + buffer_range, + }) } DescriptorPool::ArgumentBuffer { ref raw, ref mut range_allocator, } => { - let (encoder, stage_flags) = match layout { + let (encoder, stage_flags) = match set_layout { &DescriptorSetLayout::ArgumentBuffer(ref encoder, stages) => (encoder, stages), _ => return Err(pso::AllocationError::IncompatibleLayout), }; @@ -333,37 +392,72 @@ impl hal::DescriptorPool for DescriptorPool { fn free_sets(&mut self, descriptor_sets: &[DescriptorSet]) { match self { - DescriptorPool::Emulated => { - return; // Does nothing! No metal allocation happened here. - }, - DescriptorPool::ArgumentBuffer { - ref mut range_allocator, - .. - } => { + DescriptorPool::Emulated(pool_inner) => { + let mut inner = pool_inner.write().unwrap(); + for descriptor_set in descriptor_sets { + match *descriptor_set { + DescriptorSet::Emulated { ref sampler_range, ref texture_range, ref buffer_range, .. } => { + if sampler_range.start != sampler_range.end { + inner.sampler_alloc.free_range(sampler_range.clone()); + } + for sampler in &mut inner.samplers[sampler_range.start as usize .. sampler_range.end as usize] { + *sampler = None; + } + if texture_range.start != texture_range.end { + inner.texture_alloc.free_range(texture_range.clone()); + } + for image in &mut inner.textures[texture_range.start as usize .. texture_range.end as usize] { + *image = None; + } + if buffer_range.start != buffer_range.end { + inner.buffer_alloc.free_range(buffer_range.clone()); + } + for buffer in &mut inner.buffers[buffer_range.start as usize .. buffer_range.end as usize] { + buffer.base = None; + } + } + DescriptorSet::ArgumentBuffer{..} => { + panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!") + } + } + } + } + DescriptorPool::ArgumentBuffer { ref mut range_allocator, .. } => { for descriptor_set in descriptor_sets { match descriptor_set { - DescriptorSet::Emulated(..) => panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!"), - DescriptorSet::ArgumentBuffer { - offset, - encoder, - .. - } => { + DescriptorSet::Emulated{..} => { + panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!") + } + DescriptorSet::ArgumentBuffer { offset, encoder, .. } => { let handle_range = (*offset)..offset + encoder.encoded_length(); range_allocator.free_range(handle_range); - }, + } } } - }, + } } } fn reset(&mut self) { - match self { - DescriptorPool::Emulated => {/* No action necessary */} - DescriptorPool::ArgumentBuffer { - range_allocator, - .. - } => { + match *self { + DescriptorPool::Emulated(ref pool_inner) => { + let mut inner = pool_inner.write().unwrap(); + + inner.sampler_alloc.reset(); + inner.texture_alloc.reset(); + inner.buffer_alloc.reset(); + + for sampler in &mut inner.samplers { + *sampler = None; + } + for texture in &mut inner.textures { + *texture = None; + } + for buffer in &mut inner.buffers { + buffer.base = None; + } + } + DescriptorPool::ArgumentBuffer { ref mut range_allocator, .. } => { range_allocator.reset(); } } @@ -372,7 +466,7 @@ impl hal::DescriptorPool for DescriptorPool { #[derive(Debug)] pub enum DescriptorSetLayout { - Emulated(Vec, Vec), + Emulated(Arc>, Vec), ArgumentBuffer(metal::ArgumentEncoder, pso::ShaderStageFlags), } unsafe impl Send for DescriptorSetLayout {} @@ -380,50 +474,23 @@ unsafe impl Sync for DescriptorSetLayout {} #[derive(Clone, Debug)] pub enum DescriptorSet { - Emulated(Arc>), + Emulated { + pool: Arc>, + layouts: Arc>, + sampler_range: Range, + texture_range: Range, + buffer_range: Range + }, ArgumentBuffer { raw: metal::Buffer, offset: NSUInteger, encoder: metal::ArgumentEncoder, stage_flags: pso::ShaderStageFlags, - } + }, } unsafe impl Send for DescriptorSet {} unsafe impl Sync for DescriptorSet {} -#[derive(Debug)] -pub struct DescriptorSetInner { - pub(crate) layout: Vec, // TODO: maybe don't clone? - // The index of `bindings` is `pso::DescriptorBinding` - pub(crate) bindings: Vec>, -} -unsafe impl Send for DescriptorSetInner {} - -#[derive(Clone, Debug)] -pub struct BufferBinding { - pub base: Option<(BufferPtr, u64)>, - pub dynamic: bool, -} - -#[derive(Clone, Debug)] -pub enum DescriptorSetBinding { - Sampler(Vec>), - Image(Vec>), - Combined(Vec<(Option<(TexturePtr, image::Layout)>, Option)>), - Buffer(Vec), - //InputAttachment(Vec<(TexturePtr, image::Layout)>), -} - -impl DescriptorSetBinding { - pub(crate) fn count(&self) -> usize { - match *self { - DescriptorSetBinding::Sampler(ref vec) => vec.len(), - DescriptorSetBinding::Image(ref vec) => vec.len(), - DescriptorSetBinding::Combined(ref vec) => 2 * vec.len(), - DescriptorSetBinding::Buffer(ref vec) => vec.len(), - } - } -} #[derive(Debug)] pub struct Memory { diff --git a/src/hal/src/pso/descriptor.rs b/src/hal/src/pso/descriptor.rs index db533a7e7bd..0ad051c54e9 100644 --- a/src/hal/src/pso/descriptor.rs +++ b/src/hal/src/pso/descriptor.rs @@ -138,6 +138,7 @@ pub trait DescriptorPool: Send + Sync + fmt::Debug { .collect() } + //TODO: change the API to use an iterator /// Free the descriptor sets given, after calling this all descriptor sets in `descriptor_sets` /// will be invalid. fn free_sets(&mut self, descriptor_sets: &[B::DescriptorSet]); From 61a2cef38454410a93ff3ddce72b34a2905588d1 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 26 Jun 2018 23:14:53 -0400 Subject: [PATCH 2/5] [mtl] only issue resource commands for changed bindings --- src/backend/metal/src/command.rs | 185 +++++++++++++++++++------------ src/backend/metal/src/lib.rs | 6 +- 2 files changed, 115 insertions(+), 76 deletions(-) diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 24dcf2b1b10..2ab8e9cc094 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -558,28 +558,59 @@ impl StageResources { self.push_constants_buffer_id = None; } - fn add_buffer(&mut self, slot: usize, buffer: BufferPtr, offset: buffer::Offset) { - while self.buffers.len() <= slot { - self.buffers.push(None) + fn set_buffer(&mut self, slot: usize, buffer: BufferPtr, offset: buffer::Offset) -> bool { + if self.buffers.len() <= slot { + self.buffers.resize(slot + 1, None); + } + let value = Some((buffer, offset)); + if self.buffers[slot] != value { + self.buffers[slot] = value; + true + } else { + false } - self.buffers[slot] = Some((buffer.to_owned(), offset)); } - fn add_textures(&mut self, start: usize, textures: &[Option<(TexturePtr, Layout)>]) { - while self.textures.len() < start + textures.len() { - self.textures.push(None) + fn set_textures( + &mut self, start: usize, textures: &[Option<(TexturePtr, Layout)>], mut update: F + ) where + F: FnMut(usize, Option) + { + if self.textures.len() < start + textures.len() { + self.textures.resize(start + textures.len(), None); } - for (out, tex) in self.textures[start..].iter_mut().zip(textures.iter()) { - *out = tex.map(|(t, _)| t); + for (i, (out, maybe)) in self + .textures[start..] + .iter_mut() + .zip(textures) + .enumerate() + { + let value = maybe.map(|(t, _)| t); + if *out != value { + *out = value; + update(start + i, value); + } } } - fn add_samplers(&mut self, start: usize, samplers: &[Option]) { - while self.samplers.len() < start + samplers.len() { - self.samplers.push(None) + fn set_samplers( + &mut self, start: usize, samplers: &[Option], mut update: F + ) where + F: FnMut(usize, Option) + { + if self.samplers.len() < start + samplers.len() { + self.samplers.resize(start + samplers.len(), None); } - for (out, sampler) in self.samplers[start..].iter_mut().zip(samplers.iter()) { - *out = *sampler; + for (i, (out, &value)) in self + .samplers[start..] + .iter_mut() + .zip(samplers) + .enumerate() + { + if *out != value { + *out = value; + update(start + i, value); + } } } } @@ -2540,29 +2571,23 @@ impl com::RawCommandBuffer for CommandBuffer { for (stage, loc, resources) in bind_vs.into_iter().chain(bind_fs) { if sampler_base != sm_range.start { debug_assert_eq!(sampler_base, sm_range.end); - let samplers = &pool.samplers[sm_range.clone()]; - let start = pipe_layout.res_overrides[&loc].sampler_id as usize; - resources.add_samplers(start, samplers); - commands.extend(samplers.iter().enumerate().map(|(i, &sampler)| { - soft::RenderCommand::BindSampler { - stage, - index: start + i, - sampler, - } - })); + resources.set_samplers( + pipe_layout.res_overrides[&loc].sampler_id as usize, + &pool.samplers[sm_range.clone()], + |index, sampler| { + commands.push(soft::RenderCommand::BindSampler { stage, index, sampler }); + }, + ); } if texture_base != tx_range.start { debug_assert_eq!(texture_base, tx_range.end); - let textures = &pool.textures[tx_range.clone()]; - let start = pipe_layout.res_overrides[&loc].texture_id as usize; - resources.add_textures(start, textures); - commands.extend(textures.iter().enumerate().map(|(i, texture)| { - soft::RenderCommand::BindTexture { - stage, - index: start + i, - texture: texture.map(|(tex, _)| tex), - } - })); + resources.set_textures( + pipe_layout.res_overrides[&loc].texture_id as usize, + &pool.textures[tx_range.clone()], + |index, texture| { + commands.push(soft::RenderCommand::BindTexture { stage, index, texture }); + }, + ); } if buffer_base != bf_range.start { debug_assert_eq!(buffer_base, bf_range.end); @@ -2576,7 +2601,9 @@ impl com::RawCommandBuffer for CommandBuffer { offset += dynamic_offsets[dynamic_index]; dynamic_index += 1; } - resources.add_buffer(start + i, buffer, offset as _); + if !resources.set_buffer(start + i, buffer, offset as _) { + continue + } (Some(buffer), offset) } None => (None, 0), @@ -2605,13 +2632,16 @@ impl com::RawCommandBuffer for CommandBuffer { binding: 0, }; let slot = pipe_layout.res_overrides[&loc].buffer_id; - self.state.resources_vs.add_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _); - Some(soft::RenderCommand::BindBuffer { - stage: pso::Stage::Vertex, - index: slot as _, - buffer: Some(BufferPtr(raw.as_ptr())), - offset, - }) + if self.state.resources_vs.set_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _) { + Some(soft::RenderCommand::BindBuffer { + stage: pso::Stage::Vertex, + index: slot as _, + buffer: Some(BufferPtr(raw.as_ptr())), + offset, + }) + } else { + None + } } else { None }; @@ -2622,13 +2652,16 @@ impl com::RawCommandBuffer for CommandBuffer { binding: 0, }; let slot = pipe_layout.res_overrides[&loc].buffer_id; - self.state.resources_fs.add_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _); - Some(soft::RenderCommand::BindBuffer { - stage: pso::Stage::Fragment, - index: slot as _, - buffer: Some(BufferPtr(raw.as_ptr())), - offset, - }) + if self.state.resources_fs.set_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _) { + Some(soft::RenderCommand::BindBuffer { + stage: pso::Stage::Fragment, + index: slot as _, + buffer: Some(BufferPtr(raw.as_ptr())), + offset, + }) + } else { + None + } } else { None }; @@ -2708,27 +2741,23 @@ impl com::RawCommandBuffer for CommandBuffer { if sampler_base != sm_range.start { debug_assert_eq!(sampler_base, sm_range.end); - let samplers = &pool.samplers[sm_range]; - let start = res_override.sampler_id as usize; - resources.add_samplers(start, samplers); - commands.extend(samplers.iter().enumerate().map(|(i, &sampler)| { - soft::ComputeCommand::BindSampler { - index: start + i, - sampler, - } - })); + resources.set_samplers( + res_override.sampler_id as usize, + &pool.samplers[sm_range], + |index, sampler| { + commands.push(soft::ComputeCommand::BindSampler { index, sampler }); + }, + ); } if texture_base != tx_range.start { debug_assert_eq!(texture_base, tx_range.end); - let textures = &pool.textures[tx_range]; - let start = res_override.texture_id as usize; - resources.add_textures(start, textures); - commands.extend(textures.iter().enumerate().map(|(i, texture)| { - soft::ComputeCommand::BindTexture { - index: start + i, - texture: texture.map(|(tex, _)| tex), - } - })); + resources.set_textures( + res_override.texture_id as usize, + &pool.textures[tx_range], + |index, texture| { + commands.push(soft::ComputeCommand::BindTexture { index, texture }); + }, + ); } if buffer_base != bf_range.start { debug_assert_eq!(buffer_base, bf_range.end); @@ -2742,7 +2771,9 @@ impl com::RawCommandBuffer for CommandBuffer { offset += dynamic_offsets[dynamic_index]; dynamic_index += 1; } - resources.add_buffer(start + i, buffer, offset as _); + if !resources.set_buffer(start + i, buffer, offset as _) { + continue + } (Some(buffer), offset) } None => (None, 0), @@ -2763,11 +2794,19 @@ impl com::RawCommandBuffer for CommandBuffer { } native::DescriptorSet::ArgumentBuffer { ref raw, offset, stage_flags, .. } => { if stage_flags.contains(pso::ShaderStageFlags::COMPUTE) { - resources.add_buffer( - res_override.buffer_id as _, - BufferPtr(raw.as_ptr()), - offset as _, - ); + let index = res_override.buffer_id as usize; + let buffer = BufferPtr(raw.as_ptr()); + if resources.set_buffer(index, buffer, offset as _) { + let com = soft::ComputeCommand::BindBuffer { + index, + buffer: Some(buffer), + offset, + }; + self.inner + .borrow_mut() + .sink() + .pre_compute_commands(iter::once(com)); + } } } } diff --git a/src/backend/metal/src/lib.rs b/src/backend/metal/src/lib.rs index 84e55fb640f..af3553e3ba0 100644 --- a/src/backend/metal/src/lib.rs +++ b/src/backend/metal/src/lib.rs @@ -256,7 +256,7 @@ fn validate_line_width(width: f32) { assert_eq!(width, 1.0); } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct BufferPtr(*mut metal::MTLBuffer); impl BufferPtr { @@ -273,7 +273,7 @@ impl BufferPtr { } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct TexturePtr(*mut metal::MTLTexture); impl TexturePtr { @@ -290,7 +290,7 @@ impl TexturePtr { } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct SamplerPtr(*mut metal::MTLSamplerState); impl SamplerPtr { From 9bcc9f405e87d66fa38bc3bd8c8d04f16b6167cb Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 26 Jun 2018 23:45:48 -0400 Subject: [PATCH 3/5] [mtl] temporary scope for pre-pass commands --- src/backend/metal/src/command.rs | 263 ++++++++++++++++--------------- 1 file changed, 132 insertions(+), 131 deletions(-) diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 2ab8e9cc094..0809f822f08 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -628,30 +628,64 @@ enum CommandSink { }, } -//TODO: scope-based command submission, to avoid doing any work (including just storing the command lists) -// if they are going to be discarded (e.g. by `pre_render_commands`) +/// A helper temporary object that consumes state-setting commands only +/// applicable to a render pass currently encoded. +enum PreRender<'a> { + Immediate(&'a metal::RenderCommandEncoder), + Deferred(&'a mut Vec>), + Void, +} + +impl<'a> PreRender<'a> { + fn is_void(&self) -> bool { + match *self { + PreRender::Void => true, + _ => false, + } + } + + fn issue<'b>(&mut self, command: soft::RenderCommand<&'b soft::Own>) { + match *self { + PreRender::Immediate(encoder) => exec_render(encoder, command), + PreRender::Deferred(ref mut list) => list.push(command.own()), + PreRender::Void => (), + } + } +} + +/// A helper temporary object that consumes state-setting commands only +/// applicable to a compute pass currently encoded. +enum PreCompute<'a> { + Immediate(&'a metal::ComputeCommandEncoder), + Deferred(&'a mut Vec>), + Void, +} + +impl<'a> PreCompute<'a> { + fn issue<'b>(&mut self, command: soft::ComputeCommand<&'b soft::Own>) { + match *self { + PreCompute::Immediate(encoder) => exec_compute(encoder, command), + PreCompute::Deferred(ref mut list) => list.push(command.own()), + PreCompute::Void => (), + } + } +} impl CommandSink { - /// Issue provided (state-setting) commands only when there is already - /// a render pass being actively encoded. - /// The caller is expected to change the cached state accordingly, so these commands - /// are going to be issued when a next pass starts, if not at this very moment. - fn pre_render_commands<'a, I>(&mut self, commands: I) - where - I: IntoIterator>, - { + /// Start issuing pre-render commands. Those can be rejected, so the caller is responsible + /// for updating the state cache accordingly, so that it's set upon the start of a next pass. + fn pre_render(&mut self) -> PreRender { match *self { CommandSink::Immediate { encoder_state: EncoderState::Render(ref encoder), .. } => { - for command in commands { - exec_render(encoder, command); - } + PreRender::Immediate(encoder) } CommandSink::Deferred { ref mut passes, is_encoding: true } => { - if let Some(&mut soft::Pass::Render { commands: ref mut list, .. }) = passes.last_mut() { - list.extend(commands.into_iter().map(soft::RenderCommand::own)); + match passes.last_mut() { + Some(&mut soft::Pass::Render { commands: ref mut list, .. }) => PreRender::Deferred(list), + _ => PreRender::Void, } } - _ => {} + _ => PreRender::Void } } @@ -724,26 +758,20 @@ impl CommandSink { } } - /// Issue provided (state-setting) commands only when there is already - /// a compute pass being actively encoded. - /// The caller is expected to change the cached state accordingly, so these commands - /// are going to be issued when a next pass starts, if not at this very moment. - fn pre_compute_commands<'a, I>(&mut self, commands: I) - where - I: IntoIterator>, - { + /// Start issuing pre-compute commands. Those can be rejected, so the caller is responsible + /// for updating the state cache accordingly, so that it's set upon the start of a next pass. + fn pre_compute(&mut self) -> PreCompute { match *self { CommandSink::Immediate { encoder_state: EncoderState::Compute(ref encoder), .. } => { - for command in commands { - exec_compute(encoder, command); - } + PreCompute::Immediate(encoder) } CommandSink::Deferred { ref mut passes, is_encoding: true } => { - if let Some(&mut soft::Pass::Compute(ref mut list)) = passes.last_mut() { - list.extend(commands.into_iter().map(soft::ComputeCommand::own)); + match passes.last_mut() { + Some(&mut soft::Pass::Compute(ref mut list)) => PreCompute::Deferred(list), + _ => PreCompute::Void, } } - _ => {} + _ => PreCompute::Void } } @@ -2216,11 +2244,13 @@ impl com::RawCommandBuffer for CommandBuffer { let mask = self.state.set_vertex_buffers(); if mask != 0 { - let commands = self.state.iter_vertex_buffers(mask); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(commands); + let mut inner = self.inner.borrow_mut(); + let mut pre = inner.sink().pre_render(); + if !pre.is_void() { + for com in self.state.iter_vertex_buffers(mask) { + pre.issue(com); + } + } } } @@ -2245,7 +2275,8 @@ impl com::RawCommandBuffer for CommandBuffer { self.inner .borrow_mut() .sink() - .pre_render_commands(iter::once(com)); + .pre_render() + .issue(com); } fn set_scissors(&mut self, first_scissor: u32, rects: T) @@ -2268,7 +2299,8 @@ impl com::RawCommandBuffer for CommandBuffer { self.inner .borrow_mut() .sink() - .pre_render_commands(iter::once(com)); + .pre_render() + .issue(com); } fn set_blend_constants(&mut self, color: pso::ColorValue) { @@ -2276,7 +2308,8 @@ impl com::RawCommandBuffer for CommandBuffer { self.inner .borrow_mut() .sink() - .pre_render_commands(iter::once(com)); + .pre_render() + .issue(com); } fn set_depth_bounds(&mut self, _: Range) { @@ -2292,7 +2325,8 @@ impl com::RawCommandBuffer for CommandBuffer { self.inner .borrow_mut() .sink() - .pre_render_commands(iter::once(com)); + .pre_render() + .issue(com); } fn set_stencil_reference(&mut self, faces: pso::Face, value: pso::StencilValue) { @@ -2305,11 +2339,11 @@ impl com::RawCommandBuffer for CommandBuffer { }; let com = self.state.set_stencil_reference_values(front, back); - self.inner .borrow_mut() .sink() - .pre_render_commands(iter::once(com)); + .pre_render() + .issue(com); } fn set_stencil_read_mask(&mut self, faces: pso::Face, value: pso::StencilValue) { @@ -2321,11 +2355,13 @@ impl com::RawCommandBuffer for CommandBuffer { _ => (value, value), }; - let com = self.state.set_stencil_mask_values(&self.shared, Some((front, back)), None, None); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(com); + if let Some(com) = self.state.set_stencil_mask_values(&self.shared, Some((front, back)), None, None) { + self.inner + .borrow_mut() + .sink() + .pre_render() + .issue(com); + } } fn set_stencil_write_mask(&mut self, faces: pso::Face, value: pso::StencilValue) { @@ -2337,11 +2373,13 @@ impl com::RawCommandBuffer for CommandBuffer { _ => (value, value), }; - let com = self.state.set_stencil_mask_values(&self.shared, None, Some((front, back)), None); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(com); + if let Some(com) = self.state.set_stencil_mask_values(&self.shared, None, Some((front, back)), None) { + self.inner + .borrow_mut() + .sink() + .pre_render() + .issue(com); + } } fn begin_render_pass( @@ -2446,26 +2484,27 @@ impl com::RawCommandBuffer for CommandBuffer { let vertex_mask = self.state.set_vertex_buffers(); let mut inner = self.inner.borrow_mut(); - let mut commands = SmallVec::<[soft::RenderCommand<_>; 5]>::new(); - commands.push(soft::RenderCommand::BindPipeline( + let mut pre = inner.sink().pre_render(); + pre.issue(soft::RenderCommand::BindPipeline( &*pipeline.raw, pipeline.rasterizer_state.clone(), )); if let Some(ref vp) = pipeline.baked_states.viewport { - commands.push(self.state.set_viewport(vp, &self.shared.disabilities)); + pre.issue(self.state.set_viewport(vp, &self.shared.disabilities)); } if let Some(ref rect) = pipeline.baked_states.scissor { - commands.push(self.state.set_scissor(rect)); + pre.issue(self.state.set_scissor(rect)); } if let Some(ref color) = pipeline.baked_states.blend_color { - commands.push(self.state.set_blend_color(color)); + pre.issue(self.state.set_blend_color(color)); } // re-bind vertex buffers if vertex_mask != 0 { - let vertex_commands = self.state.iter_vertex_buffers(vertex_mask); - inner.sink().pre_render_commands(vertex_commands); + for command in self.state.iter_vertex_buffers(vertex_mask) { + pre.issue(command); + } } let ds = &pipeline.depth_stencil_state; @@ -2475,29 +2514,27 @@ impl com::RawCommandBuffer for CommandBuffer { let front_ref = ds.stencil.front_reference.static_or(self.state.stencil.front_reference); let back_ref = ds.stencil.back_reference.static_or(self.state.stencil.back_reference); if ds.stencil.front_reference.is_static() || ds.stencil.back_reference.is_static() { - commands.push(self.state.set_stencil_reference_values(front_ref, back_ref)); + pre.issue(self.state.set_stencil_reference_values(front_ref, back_ref)); } - let command = match ds.depth_stencil_static { - Some(ref raw) => Some(self.state.set_depth_stencil_desc(&desc, raw)), + match ds.depth_stencil_static { + Some(ref raw) => pre.issue(self.state.set_depth_stencil_desc(&desc, raw)), None => { let front_r = ds.stencil.front_read_mask.static_or(self.state.stencil.front_read_mask); let back_r = ds.stencil.back_read_mask.static_or(self.state.stencil.back_read_mask); let front_w = ds.stencil.front_write_mask.static_or(self.state.stencil.front_write_mask); let back_w = ds.stencil.back_write_mask.static_or(self.state.stencil.back_write_mask); - self.state.set_stencil_mask_values( + if let Some(com) = self.state.set_stencil_mask_values( &self.shared, Some((front_r, back_r)), Some((front_w, back_w)), ds.depth_stencil_desc_raw.as_ref().map(Borrow::borrow), - ) + ) { + pre.issue(com); + } } }; - - commands.extend(command); } - - inner.sink().pre_render_commands(commands); } fn bind_graphics_descriptor_sets<'a, I, J>( @@ -2516,11 +2553,12 @@ impl com::RawCommandBuffer for CommandBuffer { let mut offset_iter = offsets.into_iter(); let mut dynamic_offsets = SmallVec::<[u64; 16]>::new(); + let mut inner = self.inner.borrow_mut(); + let mut pre = inner.sink().pre_render(); for (set_index, desc_set) in sets.into_iter().enumerate() { match *desc_set.borrow() { native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { - let mut commands = Vec::new(); let pool = pool.read().unwrap(); let mut sampler_base = sampler_range.start as usize; let mut texture_base = texture_range.start as usize; @@ -2575,7 +2613,7 @@ impl com::RawCommandBuffer for CommandBuffer { pipe_layout.res_overrides[&loc].sampler_id as usize, &pool.samplers[sm_range.clone()], |index, sampler| { - commands.push(soft::RenderCommand::BindSampler { stage, index, sampler }); + pre.issue(soft::RenderCommand::BindSampler { stage, index, sampler }); }, ); } @@ -2585,7 +2623,7 @@ impl com::RawCommandBuffer for CommandBuffer { pipe_layout.res_overrides[&loc].texture_id as usize, &pool.textures[tx_range.clone()], |index, texture| { - commands.push(soft::RenderCommand::BindTexture { stage, index, texture }); + pre.issue(soft::RenderCommand::BindTexture { stage, index, texture }); }, ); } @@ -2608,7 +2646,7 @@ impl com::RawCommandBuffer for CommandBuffer { } None => (None, 0), }; - commands.push(soft::RenderCommand::BindBuffer { + pre.issue(soft::RenderCommand::BindBuffer { stage, index: start + i, buffer, @@ -2618,14 +2656,9 @@ impl com::RawCommandBuffer for CommandBuffer { } } } - - self.inner - .borrow_mut() - .sink() - .pre_render_commands(commands); } native::DescriptorSet::ArgumentBuffer { ref raw, offset, stage_flags, .. } => { - let com_vs = if stage_flags.contains(pso::ShaderStageFlags::VERTEX) { + if stage_flags.contains(pso::ShaderStageFlags::VERTEX) { let loc = msl::ResourceBindingLocation { stage: spirv::ExecutionModel::Vertex, desc_set: (first_set + set_index) as _, @@ -2633,19 +2666,15 @@ impl com::RawCommandBuffer for CommandBuffer { }; let slot = pipe_layout.res_overrides[&loc].buffer_id; if self.state.resources_vs.set_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _) { - Some(soft::RenderCommand::BindBuffer { + pre.issue(soft::RenderCommand::BindBuffer { stage: pso::Stage::Vertex, index: slot as _, buffer: Some(BufferPtr(raw.as_ptr())), offset, - }) - } else { - None + }); } - } else { - None - }; - let com_fs = if stage_flags.contains(pso::ShaderStageFlags::FRAGMENT) { + } + if stage_flags.contains(pso::ShaderStageFlags::FRAGMENT) { let loc = msl::ResourceBindingLocation { stage: spirv::ExecutionModel::Fragment, desc_set: (first_set + set_index) as _, @@ -2653,23 +2682,14 @@ impl com::RawCommandBuffer for CommandBuffer { }; let slot = pipe_layout.res_overrides[&loc].buffer_id; if self.state.resources_fs.set_buffer(slot as _, BufferPtr(raw.as_ptr()), offset as _) { - Some(soft::RenderCommand::BindBuffer { + pre.issue(soft::RenderCommand::BindBuffer { stage: pso::Stage::Fragment, index: slot as _, buffer: Some(BufferPtr(raw.as_ptr())), offset, - }) - } else { - None + }); } - } else { - None - }; - let commands = com_vs.into_iter().chain(com_fs); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(commands); + } } } } @@ -2680,11 +2700,11 @@ impl com::RawCommandBuffer for CommandBuffer { self.state.work_group_size = pipeline.work_group_size; let command = soft::ComputeCommand::BindPipeline(&*pipeline.raw); - self.inner .borrow_mut() .sink() - .pre_compute_commands(iter::once(command)); + .pre_compute() + .issue(command); } fn bind_compute_descriptor_sets<'a, I, J>( @@ -2703,6 +2723,8 @@ impl com::RawCommandBuffer for CommandBuffer { let mut offset_iter = offsets.into_iter(); let mut dynamic_offsets = SmallVec::<[u64; 16]>::new(); + let mut inner = self.inner.borrow_mut(); + let mut pre = inner.sink().pre_compute(); for (set_index, desc_set) in sets.into_iter().enumerate() { let resources = &mut self.state.resources_cs; @@ -2713,7 +2735,6 @@ impl com::RawCommandBuffer for CommandBuffer { }]; match *desc_set.borrow() { native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { - let mut commands = Vec::new(); let pool = pool.read().unwrap(); let mut sampler_base = sampler_range.start as usize; let mut texture_base = texture_range.start as usize; @@ -2745,7 +2766,7 @@ impl com::RawCommandBuffer for CommandBuffer { res_override.sampler_id as usize, &pool.samplers[sm_range], |index, sampler| { - commands.push(soft::ComputeCommand::BindSampler { index, sampler }); + pre.issue(soft::ComputeCommand::BindSampler { index, sampler }); }, ); } @@ -2755,7 +2776,7 @@ impl com::RawCommandBuffer for CommandBuffer { res_override.texture_id as usize, &pool.textures[tx_range], |index, texture| { - commands.push(soft::ComputeCommand::BindTexture { index, texture }); + pre.issue(soft::ComputeCommand::BindTexture { index, texture }); }, ); } @@ -2778,7 +2799,7 @@ impl com::RawCommandBuffer for CommandBuffer { } None => (None, 0), }; - commands.push(soft::ComputeCommand::BindBuffer { + pre.issue(soft::ComputeCommand::BindBuffer { index: start + i, buffer, offset, @@ -2786,26 +2807,17 @@ impl com::RawCommandBuffer for CommandBuffer { } } } - - self.inner - .borrow_mut() - .sink() - .pre_compute_commands(commands); } native::DescriptorSet::ArgumentBuffer { ref raw, offset, stage_flags, .. } => { if stage_flags.contains(pso::ShaderStageFlags::COMPUTE) { let index = res_override.buffer_id as usize; let buffer = BufferPtr(raw.as_ptr()); if resources.set_buffer(index, buffer, offset as _) { - let com = soft::ComputeCommand::BindBuffer { + pre.issue(soft::ComputeCommand::BindBuffer { index, buffer: Some(buffer), offset, - }; - self.inner - .borrow_mut() - .sink() - .pre_compute_commands(iter::once(com)); + }); } } } @@ -3148,22 +3160,14 @@ impl com::RawCommandBuffer for CommandBuffer { let id = self.shared.push_constants_buffer_id; if stages.intersects(pso::ShaderStageFlags::GRAPHICS) { - // Note: it's a waste to heap allocate the bytes here in case - // of no active render pass. + let mut inner = self.inner.borrow_mut(); + let mut pre = inner.sink().pre_render(); // Note: the whole range is re-uploaded, which may be inefficient if stages.contains(pso::ShaderStageFlags::VERTEX) { - let com = self.state.push_vs_constants(id); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(iter::once(com)); + pre.issue(self.state.push_vs_constants(id)); } if stages.contains(pso::ShaderStageFlags::FRAGMENT) { - let com = self.state.push_ps_constants(id); - self.inner - .borrow_mut() - .sink() - .pre_render_commands(iter::once(com)); + pre.issue(self.state.push_ps_constants(id)); } } } @@ -3177,15 +3181,12 @@ impl com::RawCommandBuffer for CommandBuffer { self.state.update_push_constants(offset, constants); let id = self.shared.push_constants_buffer_id; - // Note: it's a waste to heap allocate the bytes here in case - // of no active render pass. // Note: the whole range is re-uploaded, which may be inefficient - let command = self.state.push_cs_constants(id); - self.inner .borrow_mut() .sink() - .pre_compute_commands(iter::once(command)); + .pre_compute() + .issue(self.state.push_cs_constants(id)); } fn execute_commands( From cb4c48a7e7d382d40005b9ce3bee0fa9b95fcdf4 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Wed, 27 Jun 2018 09:37:04 -0400 Subject: [PATCH 4/5] Avoid allocation in free_sets --- src/backend/dx11/src/lib.rs | 5 ++++- src/backend/dx12/src/native.rs | 5 ++++- src/backend/empty/src/lib.rs | 5 ++++- src/backend/gl/src/native.rs | 5 ++++- src/backend/metal/src/native.rs | 9 ++++++--- src/backend/vulkan/src/native.rs | 7 +++++-- src/hal/src/device.rs | 10 ++++++++-- src/hal/src/pso/descriptor.rs | 8 ++++---- 8 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index 12ed22454d5..755eae70115 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -1525,7 +1525,10 @@ impl hal::DescriptorPool for DescriptorPool { Ok(DescriptorSet::new()) } - fn free_sets(&mut self, descriptor_sets: &[DescriptorSet]) { + fn free_sets(&mut self, _descriptor_sets: I) + where + I: IntoIterator + { unimplemented!() } diff --git a/src/backend/dx12/src/native.rs b/src/backend/dx12/src/native.rs index 032701ab366..89bf0315c97 100644 --- a/src/backend/dx12/src/native.rs +++ b/src/backend/dx12/src/native.rs @@ -494,7 +494,10 @@ impl HalDescriptorPool for DescriptorPool { }) } - fn free_sets(&mut self, descriptor_sets: &[DescriptorSet]) { + fn free_sets(&mut self, descriptor_sets: I) + where + I: IntoIterator + { for descriptor_set in descriptor_sets { for binding_info in &descriptor_set.binding_infos { if let Some(ref view_range) = binding_info.view_range { diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 4e2d0618f55..1ce539cce82 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -755,7 +755,10 @@ impl command::RawCommandBuffer for RawCommandBuffer { #[derive(Debug)] pub struct DescriptorPool; impl pso::DescriptorPool for DescriptorPool { - fn free_sets(&mut self, _descriptor_sets: &[()]) { + fn free_sets(&mut self, _descriptor_sets: I) + where + I: IntoIterator + { unimplemented!() } diff --git a/src/backend/gl/src/native.rs b/src/backend/gl/src/native.rs index c483d96bba7..de8e1c90ff4 100644 --- a/src/backend/gl/src/native.rs +++ b/src/backend/gl/src/native.rs @@ -185,7 +185,10 @@ impl pso::DescriptorPool for DescriptorPool { })).collect() } - fn free_sets(&mut self, _descriptor_sets: &[DescriptorSet]) { + fn free_sets(&mut self, _descriptor_sets: I) + where + I: IntoIterator + { // Poof! Does nothing, because OpenGL doesn't have a meaningful concept of a `DescriptorSet`. } diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 67cd49721ee..74c01907966 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -390,12 +390,15 @@ impl hal::DescriptorPool for DescriptorPool { } } - fn free_sets(&mut self, descriptor_sets: &[DescriptorSet]) { + fn free_sets(&mut self, descriptor_sets: I) + where + I: IntoIterator + { match self { DescriptorPool::Emulated(pool_inner) => { let mut inner = pool_inner.write().unwrap(); for descriptor_set in descriptor_sets { - match *descriptor_set { + match descriptor_set { DescriptorSet::Emulated { ref sampler_range, ref texture_range, ref buffer_range, .. } => { if sampler_range.start != sampler_range.end { inner.sampler_alloc.free_range(sampler_range.clone()); @@ -429,7 +432,7 @@ impl hal::DescriptorPool for DescriptorPool { panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!") } DescriptorSet::ArgumentBuffer { offset, encoder, .. } => { - let handle_range = (*offset)..offset + encoder.encoded_length(); + let handle_range = offset .. offset + encoder.encoded_length(); range_allocator.free_range(handle_range); } } diff --git a/src/backend/vulkan/src/native.rs b/src/backend/vulkan/src/native.rs index cb939e3c31f..210730037b0 100644 --- a/src/backend/vulkan/src/native.rs +++ b/src/backend/vulkan/src/native.rs @@ -140,9 +140,12 @@ impl pso::DescriptorPool for DescriptorPool { } } - fn free_sets(&mut self, descriptor_sets: &[DescriptorSet]) { + fn free_sets(&mut self, descriptor_sets: I) + where + I: IntoIterator + { self.set_free_vec.clear(); - self.set_free_vec.extend(descriptor_sets.iter().map(|d| d.raw)); + self.set_free_vec.extend(descriptor_sets.into_iter().map(|d| d.raw)); unsafe { self.device.0.free_descriptor_sets(self.raw, &self.set_free_vec); } diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 95852519346..0dec94c2304 100644 --- a/src/hal/src/device.rs +++ b/src/hal/src/device.rs @@ -221,7 +221,10 @@ pub trait Device: Any + Send + Sync { I: IntoIterator, I::Item: Borrow>, { - descs.into_iter().map(|desc| self.create_graphics_pipeline(desc.borrow())).collect() + descs + .into_iter() + .map(|desc| self.create_graphics_pipeline(desc.borrow())) + .collect() } /// Destroys a graphics pipeline. @@ -246,7 +249,10 @@ pub trait Device: Any + Send + Sync { I: IntoIterator, I::Item: Borrow>, { - descs.into_iter().map(|desc| self.create_compute_pipeline(desc.borrow())).collect() + descs + .into_iter() + .map(|desc| self.create_compute_pipeline(desc.borrow())) + .collect() } /// Destroys a compute pipeline. diff --git a/src/hal/src/pso/descriptor.rs b/src/hal/src/pso/descriptor.rs index 0ad051c54e9..a084068fb57 100644 --- a/src/hal/src/pso/descriptor.rs +++ b/src/hal/src/pso/descriptor.rs @@ -138,10 +138,10 @@ pub trait DescriptorPool: Send + Sync + fmt::Debug { .collect() } - //TODO: change the API to use an iterator - /// Free the descriptor sets given, after calling this all descriptor sets in `descriptor_sets` - /// will be invalid. - fn free_sets(&mut self, descriptor_sets: &[B::DescriptorSet]); + /// Free the given descriptor sets provided as an iterator. + fn free_sets(&mut self, descriptor_sets: I) + where + I: IntoIterator; /// Resets a descriptor pool, releasing all resources from all the descriptor sets /// allocated from it and freeing the descriptor sets. Invalidates all descriptor From 1f9b40e4be0efe00fd1ebaed19f096b8831e68ea Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Wed, 27 Jun 2018 12:37:35 -0400 Subject: [PATCH 5/5] [mtl] fix range de-allocation of an empty during descriptor allocation fail --- src/backend/auxil/range_alloc.rs | 3 +- src/backend/metal/src/native.rs | 70 ++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/backend/auxil/range_alloc.rs b/src/backend/auxil/range_alloc.rs index 5169f4172e6..b6f95ef0b32 100644 --- a/src/backend/auxil/range_alloc.rs +++ b/src/backend/auxil/range_alloc.rs @@ -19,11 +19,12 @@ where pub fn new(range: Range) -> Self { RangeAllocator { initial_range: range.clone(), - free_ranges: vec![range.clone()], + free_ranges: vec![range], } } pub fn allocate_range(&mut self, length: T) -> Option> { + assert_ne!(length + length, length); let mut best_fit: Option<(usize, Range)> = None; for (index, range) in self.free_ranges.iter().cloned().enumerate() { let range_length = range.end - range.start; diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 74c01907966..e7313cc02ae 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -322,29 +322,47 @@ impl hal::DescriptorPool for DescriptorPool { // step[2]: try to allocate the ranges from the pool let mut inner = pool_inner.write().unwrap(); - let sampler_range = match inner.sampler_alloc.allocate_range(total_samplers as _) { - Some(range) => range, - None => { - warn!("Not enough samplers for {}", total_samplers); - return Err(pso::AllocationError::FragmentedPool); + let sampler_range = if total_samplers != 0 { + match inner.sampler_alloc.allocate_range(total_samplers as _) { + Some(range) => range, + None => { + warn!("Not enough samplers for {}", total_samplers); + return Err(pso::AllocationError::FragmentedPool); + } } + } else { + 0 .. 0 }; - let texture_range = match inner.texture_alloc.allocate_range(total_textures as _) { - Some(range) => range, - None => { - inner.sampler_alloc.free_range(sampler_range); - warn!("Not enough images for {}", total_textures); - return Err(pso::AllocationError::FragmentedPool); + let texture_range = if total_textures != 0 { + match inner.texture_alloc.allocate_range(total_textures as _) { + Some(range) => range, + None => { + if sampler_range.end != 0 { + inner.sampler_alloc.free_range(sampler_range); + } + warn!("Not enough images for {}", total_textures); + return Err(pso::AllocationError::FragmentedPool); + } } + } else { + 0 .. 0 }; - let buffer_range = match inner.buffer_alloc.allocate_range(total_buffers as _) { - Some(range) => range, - None => { - inner.sampler_alloc.free_range(sampler_range); - inner.texture_alloc.free_range(texture_range); - warn!("Not enough buffers for {}", total_buffers); - return Err(pso::AllocationError::FragmentedPool); + let buffer_range = if total_buffers != 0 { + match inner.buffer_alloc.allocate_range(total_buffers as _) { + Some(range) => range, + None => { + if sampler_range.end != 0 { + inner.sampler_alloc.free_range(sampler_range); + } + if texture_range.end != 0 { + inner.texture_alloc.free_range(texture_range); + } + warn!("Not enough buffers for {}", total_buffers); + return Err(pso::AllocationError::FragmentedPool); + } } + } else { + 0 .. 0 }; // step[3]: fill out immutable samplers @@ -399,25 +417,25 @@ impl hal::DescriptorPool for DescriptorPool { let mut inner = pool_inner.write().unwrap(); for descriptor_set in descriptor_sets { match descriptor_set { - DescriptorSet::Emulated { ref sampler_range, ref texture_range, ref buffer_range, .. } => { - if sampler_range.start != sampler_range.end { - inner.sampler_alloc.free_range(sampler_range.clone()); - } + DescriptorSet::Emulated { sampler_range, texture_range, buffer_range, .. } => { for sampler in &mut inner.samplers[sampler_range.start as usize .. sampler_range.end as usize] { *sampler = None; } - if texture_range.start != texture_range.end { - inner.texture_alloc.free_range(texture_range.clone()); + if sampler_range.start != sampler_range.end { + inner.sampler_alloc.free_range(sampler_range); } for image in &mut inner.textures[texture_range.start as usize .. texture_range.end as usize] { *image = None; } - if buffer_range.start != buffer_range.end { - inner.buffer_alloc.free_range(buffer_range.clone()); + if texture_range.start != texture_range.end { + inner.texture_alloc.free_range(texture_range); } for buffer in &mut inner.buffers[buffer_range.start as usize .. buffer_range.end as usize] { buffer.base = None; } + if buffer_range.start != buffer_range.end { + inner.buffer_alloc.free_range(buffer_range); + } } DescriptorSet::ArgumentBuffer{..} => { panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!")