Merge #109

109: [WIP] Handle sanitation and gfx-hal update r=msiglreith,grovesNL a=kvark

Depends on gfx-rs/gfx#2195
Depends on gfx-rs/metal-rs#59
Fixes #103 

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

.gitignore

@@ -5,3 +5,4 @@ conformance/*.qpa
 conformance/*.txt
 **/*.rs.bk
 .vscode/
+.lldbinit

Makefile

@@ -12,7 +12,8 @@ DEQP_DIR=$(CTS_DIR)/build/external/vulkancts/modules/vulkan/
 DEQP=cd $(DEQP_DIR) && RUST_LOG=debug LD_LIBRARY_PATH=$(FULL_LIBRARY_PATH) ./deqp-vk
 DOTA_DIR=../dota2/bin/osx64
 DOTA_EXE=$(DOTA_DIR)/dota2.app/Contents/MacOS/dota2
-DOTA_PARAMS=-vulkan_disable_occlusion_queries -vulkan_scene_system_job_cost 2
+#DOTA_PARAMS=-vulkan_disable_occlusion_queries -vulkan_scene_system_job_cost 2
+DOTA_PARAMS=-vulkan_disable_occlusion_queries
 
 RUST_BACKTRACE:=1
 BACKEND:=gl
@@ -46,7 +47,7 @@ FULL_LIBRARY_PATH=$(CURDIR)/target/debug
 LIBRARY=target/debug/libportability.$(LIB_EXTENSION)
 LIBRARY_FAST=target/release/libportability.$(LIB_EXTENSION)
 
-.PHONY: all rebuild debug release version-debug version-release binding run cts clean cherry dota-debug dota-release
+.PHONY: all rebuild debug release version-debug version-release binding run cts clean cherry dota-debug dota-release dota-orig
 
 all: $(TARGET)
 
@@ -65,10 +66,15 @@ version-release:
 	cargo rustc --release --manifest-path libportability/Cargo.toml --features $(BACKEND) -- -Clink-arg="-current_version 1.0.0" -Clink-arg="-compatibility_version 1.0.0"
 
 dota-debug: version-debug $(DOTA_EXE)
-	DYLD_LIBRARY_PATH=`pwd`/target/debug:`pwd`/$(DOTA_DIR) $(DOTA_EXE) $(DOTA_PARAMS)
+	echo "env DYLD_LIBRARY_PATH=$(CURDIR)/target/debug:$(CURDIR)/$(DOTA_DIR)" >.lldbinit
+	DYLD_LIBRARY_PATH=$(CURDIR)/target/debug:$(CURDIR)/$(DOTA_DIR) $(DEBUGGER) $(DOTA_EXE) $(DOTA_PARAMS)
 
 dota-release: version-release $(DOTA_EXE)
-	DYLD_LIBRARY_PATH=`pwd`/target/release:`pwd`/$(DOTA_DIR) $(DOTA_EXE) $(DOTA_PARAMS)
+	DYLD_LIBRARY_PATH=$(CURDIR)/target/release:$(CURDIR)/$(DOTA_DIR) $(DOTA_EXE) $(DOTA_PARAMS)
+dota-molten:
+	DYLD_LIBRARY_PATH=$(CURDIR)/../MoltenVK/Package/Release/MoltenVK/macOS:$(CURDIR)/$(DOTA_DIR) $(DOTA_EXE)
+dota-orig:
+	DYLD_LIBRARY_PATH=$(CURDIR)/$(DOTA_DIR) $(DOTA_EXE)
 
 binding: $(BINDING)
 
@@ -103,6 +109,7 @@ cts:
 else
 ifdef debug
 cts: $(LIBRARY)
+	echo "env LD_LIBRARY_PATH=$(FULL_LIBRARY_PATH)" >.lldbinit
 	#(cd $(DEQP_DIR) && LD_LIBRARY_PATH=$(FULL_LIBRARY_PATH) $(DEBUGGER) ./deqp-vk -n $(debug))
 	LD_LIBRARY_PATH=$(FULL_LIBRARY_PATH) $(DEBUGGER) $(DEQP_DIR)/deqp-vk -n $(debug)
 else

libportability-gfx/Cargo.toml

@@ -18,6 +18,7 @@ nightly = []
 [dependencies]
 lazy_static = "1.0"
 log = "0.4"
+smallvec = "0.6"
 
 [dependencies.env_logger]
 version = "0.5"

libportability-gfx/src/handle.rs

@@ -1,13 +1,14 @@
 use VK_NULL_HANDLE;
 use std::{borrow, cmp, fmt, ops};
 #[cfg(feature = "nightly")]
-use std::collections::HashMap;
-#[cfg(feature = "nightly")]
 use std::sync::{Arc, Mutex};
 
+#[cfg(feature = "nightly")]
+use hal::backend::FastHashMap;
+
 #[cfg(feature = "nightly")]
 lazy_static! {
-    static ref REGISTRY: Arc<Mutex<HashMap<usize, &'static str>>> = Arc::new(Mutex::new(HashMap::new()));
+    static ref REGISTRY: Arc<Mutex<FastHashMap<usize, &'static str>>> = Arc::new(Mutex::new(FastHashMap::default()));
 }
 
 #[repr(C)]
@@ -61,6 +62,17 @@ impl<T: 'static> Handle<T> {
     }
 }
 
+impl<T> Handle<T> {
+    #[cfg(feature = "nightly")]
+    #[inline]
+    fn check(&self) {
+        assert!(REGISTRY.lock().unwrap().contains_key(&(self.0 as _)));
+    }
+    #[cfg(not(feature = "nightly"))]
+    #[inline]
+    fn check(&self) {}
+}
+
 impl<T> Clone for Handle<T> {
     fn clone(&self) -> Self {
         Handle(self.0)
@@ -72,18 +84,21 @@ impl<T> Copy for Handle<T> {}
 impl<T> ops::Deref for Handle<T> {
     type Target = T;
     fn deref(&self) -> &T {
+        self.check();
         unsafe { &*self.0 }
     }
 }
 
 impl<T> ops::DerefMut for Handle<T> {
     fn deref_mut(&mut self) -> &mut T {
+        self.check();
         unsafe { &mut *self.0 }
     }
 }
 
 impl<T> borrow::Borrow<T> for Handle<T> {
     fn borrow(&self) -> &T {
+        self.check();
         unsafe { &*self.0 }
     }
 }

libportability-gfx/src/impls.rs

@@ -9,6 +9,8 @@ use hal::device::WaitFor;
 use hal::pool::RawCommandPool;
 use hal::queue::RawCommandQueue;
 
+use smallvec::SmallVec;
+
 use std::ffi::{CStr, CString};
 #[cfg(feature = "renderdoc")]
 use std::os::raw::c_void;
@@ -357,7 +359,7 @@ pub extern "C" fn gfxGetPhysicalDeviceMemoryProperties(
 }
 #[inline]
 pub extern "C" fn gfxGetInstanceProcAddr(
-    instance: VkInstance,
+    _instance: VkInstance,
     pName: *const ::std::os::raw::c_char,
 ) -> PFN_vkVoidFunction {
     let name = unsafe { CStr::from_ptr(pName) };
@@ -1105,9 +1107,7 @@ pub extern "C" fn gfxBindBufferMemory(
     memory: VkDeviceMemory,
     memoryOffset: VkDeviceSize,
 ) -> VkResult {
-    let temp = unsafe { mem::zeroed() };
-
-    *buffer = match mem::replace(&mut *buffer, temp) {
+    let new = match mem::replace(&mut *buffer, unsafe { mem::zeroed() }) {
         Buffer::Buffer(_) => panic!("A non-sparse buffer can only be bound once!"),
         Buffer::Unbound(unbound) => {
             Buffer::Buffer(
@@ -1118,6 +1118,11 @@ pub extern "C" fn gfxBindBufferMemory(
         }
     };
 
+    // We need to move the value out of the Handle here,
+    // and then put something else back in.
+    let temp = mem::replace(&mut *buffer, new);
+    mem::forget(temp);
+
     VkResult::VK_SUCCESS
 }
 #[inline]
@@ -1127,9 +1132,7 @@ pub extern "C" fn gfxBindImageMemory(
     memory: VkDeviceMemory,
     memoryOffset: VkDeviceSize,
 ) -> VkResult {
-    let temp = unsafe { mem::zeroed() };
-
-    *image = match mem::replace(&mut *image, temp) {
+    let new = match mem::replace(&mut *image, unsafe { mem::zeroed() }) {
         Image::Image { .. } => panic!("An non-sparse image can only be bound once!"),
         Image::Unbound { raw, mip_levels, array_layers } => {
             Image::Image {
@@ -1140,6 +1143,11 @@ pub extern "C" fn gfxBindImageMemory(
         }
     };
 
+    // We need to move the value out of the Handle here,
+    // and then put something else back in.
+    let temp = mem::replace(&mut *image, new);
+    mem::forget(temp);
+
     VkResult::VK_SUCCESS
 }
 #[inline]
@@ -1719,7 +1727,13 @@ pub extern "C" fn gfxCreateGraphicsPipelines(
         };
 
         let shaders = {
-            let mut set: pso::GraphicsShaderSet<_> = unsafe { mem::zeroed() };
+            let mut set = pso::GraphicsShaderSet {
+                vertex: unsafe { mem::zeroed() }, // fake entry point
+                hull: None,
+                domain: None,
+                geometry: None,
+                fragment: None,
+            };
 
             let stages = unsafe {
                 slice::from_raw_parts(info.pStages, info.stageCount as _)
@@ -1738,7 +1752,10 @@ pub extern "C" fn gfxCreateGraphicsPipelines(
                 };
 
                 match stage.stage {
-                    VK_SHADER_STAGE_VERTEX_BIT => { set.vertex = entry_point; }
+                    VK_SHADER_STAGE_VERTEX_BIT => {
+                        let fake_vs_entry = mem::replace(&mut set.vertex, entry_point);
+                        mem::forget(fake_vs_entry);
+                    }
                     VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT => { set.hull = Some(entry_point); }
                     VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT => { set.domain = Some(entry_point); }
                     VK_SHADER_STAGE_GEOMETRY_BIT => { set.geometry = Some(entry_point); }
@@ -2431,10 +2448,9 @@ pub extern "C" fn gfxFreeDescriptorSets(
     assert!(descriptorPool.sets.is_none());
 
     descriptorPool.raw.free_sets(
-        &descriptor_sets
+        descriptor_sets
             .into_iter()
             .filter_map(|set| set.unbox())
-            .collect::<Vec<_>>()
     );
 
     VkResult::VK_SUCCESS
@@ -2450,7 +2466,8 @@ pub extern "C" fn gfxUpdateDescriptorSets(
     let write_infos = unsafe {
         slice::from_raw_parts(pDescriptorWrites, descriptorWriteCount as _)
     };
-    let mut writes = Vec::new(); //TODO: avoid allocation here and below
+    //TODO: investigate the safety of passing one giant iterator here
+    let mut writes = SmallVec::<[pso::DescriptorSetWrite<_, _>; 16]>::with_capacity(write_infos.len());
 
     for write in write_infos {
         let image_info = unsafe {
@@ -2464,14 +2481,12 @@ pub extern "C" fn gfxUpdateDescriptorSets(
         };
 
         let ty = conv::map_descriptor_type(write.descriptorType);
-        let descriptors = match ty {
+        let descriptors: SmallVec<[_; 4]> = match ty {
             pso::DescriptorType::Sampler => {
                 image_info
                     .into_iter()
-                    .map(|image| pso::Descriptor::Sampler(
-                        &*image.sampler,
-                    ))
-                    .collect::<Vec<_>>()
+                    .map(|image| pso::Descriptor::Sampler(&*image.sampler))
+                    .collect()
             }
             pso::DescriptorType::InputAttachment |
             pso::DescriptorType::SampledImage |
@@ -2482,23 +2497,23 @@ pub extern "C" fn gfxUpdateDescriptorSets(
                         &*image.imageView,
                         conv::map_image_layout(image.imageLayout),
                     ))
-                    .collect::<Vec<_>>()
+                    .collect()
             }
             pso::DescriptorType::UniformTexelBuffer => {
                 texel_buffer_views
                     .into_iter()
                     .map(|view| pso::Descriptor::UniformTexelBuffer(
                         &**view,
                     ))
-                    .collect::<Vec<_>>()
+                    .collect()
             }
             pso::DescriptorType::StorageTexelBuffer => {
                 texel_buffer_views
                     .into_iter()
                     .map(|view| pso::Descriptor::StorageTexelBuffer(
                         &**view,
                     ))
-                    .collect::<Vec<_>>()
+                    .collect()
             }
             pso::DescriptorType::UniformBuffer |
             pso::DescriptorType::StorageBuffer |
@@ -2518,7 +2533,7 @@ pub extern "C" fn gfxUpdateDescriptorSets(
                             Some(buffer.offset) .. end,
                         )
                     })
-                    .collect::<Vec<_>>()
+                    .collect()
             }
             pso::DescriptorType::CombinedImageSampler => {
                 image_info
@@ -2528,7 +2543,7 @@ pub extern "C" fn gfxUpdateDescriptorSets(
                         conv::map_image_layout(image.imageLayout),
                         &*image.sampler,
                     ))
-                    .collect::<Vec<_>>()
+                    .collect()
             }
         };
 
@@ -3096,15 +3111,13 @@ pub extern "C" fn gfxCmdBindVertexBuffers(
 
     let views = buffers
         .into_iter()
-        .zip(offsets.into_iter())
+        .zip(offsets)
         .map(|(buffer, offset)| {
             let buffer = buffer.expect("Non-sparse buffers need to be bound to device memory.");
+            (buffer, *offset)
+        });
 
-            (buffer, *offset as _)
-        })
-        .collect();
-
-    commandBuffer.bind_vertex_buffers(firstBinding, pso::VertexBufferSet(views));
+    commandBuffer.bind_vertex_buffers(firstBinding, views);
 }
 #[inline]
 pub extern "C" fn gfxCmdDraw(

libportability-gfx/src/lib.rs

@@ -18,6 +18,7 @@ extern crate gfx_backend_vulkan as back;
 extern crate lazy_static;
 #[macro_use]
 extern crate log;
+extern crate smallvec;
 #[cfg(feature = "env_logger")]
 extern crate env_logger;
 #[cfg(feature = "renderdoc")]