Add image support

Adds image data types and operations. At this point, lightly tested.

piet-gpu-hal/examples/dx12_toy.rs

@@ -6,6 +6,8 @@ use piet_gpu_hal::{dx12, BufferUsage, CmdBuf, Device, Error};
 
 const SHADER_CODE: &str = r#"RWByteAddressBuffer _53 : register(u0, space0);
 
+RWTexture2D<float4> textureOut : register(u1);
+
 static uint3 gl_GlobalInvocationID;
 struct SPIRV_Cross_Input
 {
@@ -36,6 +38,7 @@ void comp_main()
     uint param = _53.Load(index * 4 + 0);
     uint _61 = collatz_iterations(param);
     _53.Store(index * 4 + 0, _61);
+    textureOut[uint2(index, 0)] = float4(1.0, 0.0, 0.0, 1.0);
 }
 
 [numthreads(256, 1, 1)]
@@ -60,12 +63,15 @@ fn toy() -> Result<(), Error> {
         1024,
         BufferUsage::STORAGE | BufferUsage::COPY_SRC | BufferUsage::COPY_DST,
     )?;
+    let img_readback_buf =
+        device.create_buffer(1024, BufferUsage::MAP_READ | BufferUsage::COPY_DST)?;
     let data: Vec<u32> = (1..257).collect();
     let query_pool = device.create_query_pool(2)?;
     unsafe {
+        let img = device.create_image2d(256, 1)?;
         device.write_buffer(&buf, &data)?;
-        let pipeline = device.create_simple_compute_pipeline(SHADER_CODE, 1, 0)?;
+        let pipeline = device.create_simple_compute_pipeline(SHADER_CODE, 1, 1)?;
-        let ds = device.create_descriptor_set(&pipeline, &[&dev_buf], &[])?;
+        let ds = device.create_descriptor_set(&pipeline, &[&dev_buf], &[&img])?;
         let mut cmd_buf = device.create_cmd_buf()?;
         let fence = device.create_fence(false)?;
         cmd_buf.begin();
@@ -76,6 +82,7 @@ fn toy() -> Result<(), Error> {
         cmd_buf.write_timestamp(&query_pool, 1);
         cmd_buf.memory_barrier();
         cmd_buf.copy_buffer(&dev_buf, &buf);
+        cmd_buf.copy_image_to_buffer(&img, &img_readback_buf);
         cmd_buf.finish_timestamps(&query_pool);
         cmd_buf.host_barrier();
         cmd_buf.finish();

piet-gpu-hal/src/dx12.rs

@@ -9,7 +9,7 @@ use winapi::shared::dxgi1_3;
 use winapi::shared::minwindef::TRUE;
 use winapi::um::d3d12;
 
-use crate::{BufferUsage, Error};
+use crate::{BufferUsage, Error, ImageLayout};
 
 use self::wrappers::{
     CommandAllocator, CommandQueue, Device, Factory4, GraphicsCommandList, Resource, ShaderByteCode,
@@ -33,8 +33,10 @@ pub struct Buffer {
     size: u64,
 }
 
+#[derive(Clone)]
 pub struct Image {
     resource: Resource,
+    size: (u32, u32),
 }
 
 pub struct CmdBuf(GraphicsCommandList);
@@ -75,6 +77,7 @@ pub struct PipelineBuilder {
 #[derive(Default)]
 pub struct DescriptorSetBuilder {
     buffers: Vec<Buffer>,
+    images: Vec<Image>,
 }
 
 #[derive(PartialEq, Eq)]
@@ -198,11 +201,17 @@ impl crate::Device for Dx12Device {
     }
 
     unsafe fn create_image2d(&self, width: u32, height: u32) -> Result<Self::Image, Error> {
-        todo!()
+        let format = winapi::shared::dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM;
+        let resource = self
+            .device
+            .create_texture2d_buffer(width.into(), height, format, true)?;
+        let size = (width, height);
+        Ok(Image { resource, size })
     }
 
     unsafe fn destroy_image(&self, image: &Self::Image) -> Result<(), Error> {
-        todo!()
+        image.resource.destroy();
+        Ok(())
     }
 
     fn create_cmd_buf(&self) -> Result<Self::CmdBuf, Error> {
@@ -380,8 +389,11 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
     }
 
     unsafe fn host_barrier(&mut self) {
-        // TODO: anything special here?
+        // My understanding is that a host barrier is not needed, but am still hunting
-        self.memory_barrier();
+        // down an authoritative source for that. Among other things, the docs for
+        // Map suggest that it does the needed visibility operation.
+        //
+        // https://docs.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-id3d12resource-map
     }
 
     unsafe fn image_barrier(
@@ -390,28 +402,42 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
         src_layout: crate::ImageLayout,
         dst_layout: crate::ImageLayout,
     ) {
-        todo!()
+        let src_state = resource_state_for_image_layout(src_layout);
+        let dst_state = resource_state_for_image_layout(dst_layout);
+        if src_state != dst_state {
+            let bar = wrappers::create_transition_resource_barrier(
+                image.resource.get_mut(),
+                src_state,
+                dst_state,
+            );
+            self.0.resource_barrier(&[bar]);
+        }
     }
 
     unsafe fn clear_buffer(&self, buffer: &Buffer, size: Option<u64>) {
+        // Open question: do we call ClearUnorderedAccessViewUint or dispatch a
+        // compute shader? Either way we will need descriptors here.
         todo!()
     }
 
     unsafe fn copy_buffer(&self, src: &Buffer, dst: &Buffer) {
+        // TODO: consider using copy_resource here (if sizes match)
         let size = src.size.min(dst.size);
         self.0.copy_buffer(&dst.resource, 0, &src.resource, 0, size);
     }
 
     unsafe fn copy_image_to_buffer(&self, src: &Image, dst: &Buffer) {
-        todo!()
+        self.0
+            .copy_texture_to_buffer(&src.resource, &dst.resource, src.size.0, src.size.1);
     }
 
     unsafe fn copy_buffer_to_image(&self, src: &Buffer, dst: &Image) {
-        todo!()
+        self.0
+            .copy_buffer_to_texture(&src.resource, &dst.resource, dst.size.0, dst.size.1);
     }
 
     unsafe fn blit_image(&self, src: &Image, dst: &Image) {
-        todo!()
+        self.0.copy_resource(&src.resource, &dst.resource);
     }
 
     unsafe fn reset_query_pool(&mut self, pool: &QueryPool) {
@@ -443,9 +469,8 @@ impl crate::PipelineBuilder<Dx12Device> for PipelineBuilder {
     }
 
     fn add_images(&mut self, n_images: u32) {
-        if n_images != 0 {
+        // These are UAV images, so the descriptor type is the same as buffers.
-            todo!()
+        self.add_buffers(n_images)
-        }
     }
 
     fn add_textures(&mut self, max_textures: u32) {
@@ -514,9 +539,7 @@ impl crate::DescriptorSetBuilder<Dx12Device> for DescriptorSetBuilder {
     }
 
     fn add_images(&mut self, images: &[&Image]) {
-        if !images.is_empty() {
+        self.images.extend(images.iter().copied().cloned());
-            todo!()
-        }
     }
 
     fn add_textures(&mut self, images: &[&Image]) {
@@ -528,7 +551,7 @@ impl crate::DescriptorSetBuilder<Dx12Device> for DescriptorSetBuilder {
         device: &Dx12Device,
         _pipeline: &Pipeline,
     ) -> Result<DescriptorSet, Error> {
-        let n_descriptors = self.buffers.len();
+        let n_descriptors = self.buffers.len() + self.images.len();
         let heap_desc = d3d12::D3D12_DESCRIPTOR_HEAP_DESC {
             Type: d3d12::D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV,
             NumDescriptors: n_descriptors.try_into()?,
@@ -548,6 +571,13 @@ impl crate::DescriptorSetBuilder<Dx12Device> for DescriptorSetBuilder {
                 );
             ix += 1;
         }
+        for image in self.images {
+            device.device.create_unordered_access_view(
+                &image.resource,
+                heap.get_cpu_descriptor_handle_at_offset(ix),
+            );
+            ix += 1;
+        }
         Ok(DescriptorSet(heap))
     }
 }
@@ -579,3 +609,14 @@ impl MemoryArchitecture {
         }
     }
 }
+
+fn resource_state_for_image_layout(layout: ImageLayout) -> d3d12::D3D12_RESOURCE_STATES {
+    match layout {
+        ImageLayout::Undefined => d3d12::D3D12_RESOURCE_STATE_COMMON,
+        ImageLayout::Present => d3d12::D3D12_RESOURCE_STATE_PRESENT,
+        ImageLayout::BlitSrc => d3d12::D3D12_RESOURCE_STATE_COPY_SOURCE,
+        ImageLayout::BlitDst => d3d12::D3D12_RESOURCE_STATE_COPY_DEST,
+        ImageLayout::General => d3d12::D3D12_RESOURCE_STATE_COMMON,
+        ImageLayout::ShaderRead => d3d12::D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE,
+    }
+}

piet-gpu-hal/src/dx12/error.rs

@@ -41,6 +41,7 @@ impl std::error::Error for Error {}
 /// See https://docs.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-error
 fn err_str_for_hr(hr: winerror::HRESULT) -> Option<&'static str> {
     Some(match hr as u32 {
+        0x80070057 => "E_INVALIDARG",
         0x887a0001 => "DXGI_ERROR_INVALID_CALL",
         0x887a0002 => "DXGI_ERROR_NOT_FOUND",
         0x887a0004 => "DXGI_ERROR_UNSUPPORTED",

piet-gpu-hal/src/dx12/wrappers.rs

@@ -852,18 +852,17 @@ impl Device {
         Ok(buffer)
     }
 
-    pub unsafe fn create_gpu_only_texture2d_buffer(
+    pub unsafe fn create_texture2d_buffer(
         &self,
-        descriptor_heap_offset: u32,
         width: u64,
         height: u32,
         format: dxgiformat::DXGI_FORMAT,
         allow_unordered_access: bool,
     ) -> Result<Resource, Error> {
+        // Images are always created device-local.
         let heap_properties = d3d12::D3D12_HEAP_PROPERTIES {
             Type: d3d12::D3D12_HEAP_TYPE_DEFAULT,
             CPUPageProperty: d3d12::D3D12_CPU_PAGE_PROPERTY_UNKNOWN,
-            //TODO: what should MemoryPoolPreference flag be?
             MemoryPoolPreference: d3d12::D3D12_MEMORY_POOL_UNKNOWN,
             //we don't care about multi-adapter operation, so these next two will be zero
             CreationNodeMask: 0,
@@ -1336,29 +1335,10 @@ impl GraphicsCommandList {
             aligned_destination_buffer_offset,
         );
     }
-    pub unsafe fn update_texture2d_using_intermediate_buffer(
-        &self,
-        device: Device,
-        intermediate_buffer: Resource,
-        texture: &Resource,
-    ) {
-        let mut src = d3d12::D3D12_TEXTURE_COPY_LOCATION {
-            pResource: intermediate_buffer.get_mut(),
-            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_PLACED_FOOTPRINT,
-            ..mem::zeroed()
-        };
-        let (layout, _, _, _) = device.get_copyable_footprint(0, 1, 0, texture);
-        *src.u.PlacedFootprint_mut() = layout[0];
 
-        let mut dst = d3d12::D3D12_TEXTURE_COPY_LOCATION {
+    /// Copy an entire resource (buffer or image)
-            pResource: texture.get_mut(),
+    pub unsafe fn copy_resource(&self, src: &Resource, dst: &Resource) {
-            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX,
+        self.0.CopyResource(src.get_mut(), dst.get_mut());
-            ..mem::zeroed()
-        };
-        *dst.u.SubresourceIndex_mut() = 0;
-
-        self.0
-            .CopyTextureRegion(&dst as *const _, 0, 0, 0, &src as *const _, ptr::null());
     }
 
     pub unsafe fn copy_buffer(
@@ -1377,6 +1357,84 @@ impl GraphicsCommandList {
             size,
         );
     }
+
+    pub unsafe fn copy_buffer_to_texture(
+        &self,
+        buffer: &Resource,
+        texture: &Resource,
+        width: u32,
+        height: u32,
+    ) {
+        let mut src = d3d12::D3D12_TEXTURE_COPY_LOCATION {
+            pResource: buffer.get_mut(),
+            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_PLACED_FOOTPRINT,
+            ..mem::zeroed()
+        };
+        let row_pitch = width * 4;
+        assert!(
+            row_pitch % d3d12::D3D12_TEXTURE_DATA_PITCH_ALIGNMENT == 0,
+            "TODO: handle unaligned row pitch"
+        );
+        let footprint = d3d12::D3D12_PLACED_SUBRESOURCE_FOOTPRINT {
+            Offset: 0,
+            Footprint: d3d12::D3D12_SUBRESOURCE_FOOTPRINT {
+                Format: dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM,
+                Width: width,
+                Height: height,
+                Depth: 1,
+                RowPitch: row_pitch,
+            },
+        };
+        *src.u.PlacedFootprint_mut() = footprint;
+
+        let mut dst = d3d12::D3D12_TEXTURE_COPY_LOCATION {
+            pResource: texture.get_mut(),
+            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX,
+            ..mem::zeroed()
+        };
+        *dst.u.SubresourceIndex_mut() = 0;
+
+        self.0.CopyTextureRegion(&dst, 0, 0, 0, &src, ptr::null());
+    }
+
+    pub unsafe fn copy_texture_to_buffer(
+        &self,
+        texture: &Resource,
+        buffer: &Resource,
+        width: u32,
+        height: u32,
+    ) {
+        let mut src = d3d12::D3D12_TEXTURE_COPY_LOCATION {
+            pResource: texture.get_mut(),
+            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX,
+            ..mem::zeroed()
+        };
+        *src.u.SubresourceIndex_mut() = 0;
+
+        let mut dst = d3d12::D3D12_TEXTURE_COPY_LOCATION {
+            pResource: buffer.get_mut(),
+            Type: d3d12::D3D12_TEXTURE_COPY_TYPE_PLACED_FOOTPRINT,
+            ..mem::zeroed()
+        };
+        let row_pitch = width * 4;
+        assert!(
+            row_pitch % d3d12::D3D12_TEXTURE_DATA_PITCH_ALIGNMENT == 0,
+            "TODO: handle unaligned row pitch"
+        );
+        let footprint = d3d12::D3D12_PLACED_SUBRESOURCE_FOOTPRINT {
+            Offset: 0,
+            Footprint: d3d12::D3D12_SUBRESOURCE_FOOTPRINT {
+                Format: dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM,
+                Width: width,
+                Height: height,
+                Depth: 1,
+                RowPitch: row_pitch,
+            },
+        };
+        *dst.u.PlacedFootprint_mut() = footprint;
+
+        self.0.CopyTextureRegion(&dst, 0, 0, 0, &src, ptr::null());
+    }
 }
 
 pub fn default_render_target_blend_desc() -> d3d12::D3D12_RENDER_TARGET_BLEND_DESC {

piet-gpu-hal/src/lib.rs

@@ -13,7 +13,7 @@ pub mod vulkan;
 /// This isn't great but is expedient.
 pub type Error = Box<dyn std::error::Error>;
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum ImageLayout {
     Undefined,
     Present,

piet-gpu/bin/cli.rs

@@ -254,8 +254,7 @@ fn main() -> Result<(), Error> {
 
         let renderer = Renderer::new(&session, scene, n_paths, n_pathseg, n_trans)?;
         let image_usage = BufferUsage::MAP_READ | BufferUsage::COPY_DST;
-        let image_buf =
+        let image_buf = session.create_buffer((WIDTH * HEIGHT * 4) as u64, image_usage)?;
-            session.create_buffer((WIDTH * HEIGHT * 4) as u64, image_usage)?;
 
         cmd_buf.begin();
         renderer.record(&mut cmd_buf, &query_pool);

piet-gpu/src/lib.rs

@@ -488,8 +488,7 @@ impl Renderer {
             let host_upload = BufferUsage::MAP_WRITE | BufferUsage::COPY_SRC;
             let mut buffer = session.create_buffer(buf.len() as u64, host_upload)?;
             buffer.write(buf)?;
-            let image =
+            let image = session.create_image2d(width.try_into()?, height.try_into()?)?;
-                session.create_image2d(width.try_into()?, height.try_into()?)?;
             let mut cmd_buf = session.cmd_buf()?;
             cmd_buf.begin();
             cmd_buf.image_barrier(