On Wed Sep 13 08:53:39 2023 +0000, Henri Verbeet wrote:
Just a couple of nits:
+typedef enum D3D12_DRED_ALLOCATION_TYPE +{ ... + D3D12_DRED_ALLOCATION_TYPE_INVALID = 0xFFFFFFFF +} D3D12_DRED_ALLOCATION_TYPE;Could we make that lower case hexadecimal?
+ D3D12_RESOURCE_ALLOCATION_INFO GetResourceAllocationInfo1( + UINT visible_mask, + UINT num_resource_descs, + const D3D12_RESOURCE_DESC *resource_descs, + D3D12_RESOURCE_ALLOCATION_INFO1 *resource_allocation_info1);We'd typically use "resource_desc_count" instead of "num_resource_descs" above. There are a couple of other "num_*" parameters in this MR as well. Ultimately I'm not too bothered about these, so don't worry about it if you feel changing them is too much effort, but if you did change them that would certainly be welcome.
+ void CreateSamplerFeedbackUnorderedAccessView( + ID3D12Resource *targeted_resource, + ID3D12Resource *feedback_resource, + D3D12_CPU_DESCRIPTOR_HANDLE dest_descriptor);Similarly, we'd typically use "dst_descriptor" above and for similar parameters.
+interface ID3D12Device6 : ID3D12Device5 +{ + HRESULT SetBackgroundProcessingMode( + D3D12_BACKGROUND_PROCESSING_MODE Mode, + D3D12_MEASUREMENTS_ACTION MeasurementsAction, + HANDLE hEventToSignalUponCompletion, + BOOL*pbFurtherMeasurementsDesired); +}I'd suggest something like the following:
interface ID3D12Device6 : ID3D12Device5 { HRESULT SetBackgroundProcessingMode( D3D12_BACKGROUND_PROCESSING_MODE mode, D3D12_MEASUREMENTS_ACTION action, HANDLE event, BOOL *further_measurements_desired); }+cpp_quote("#define D3D12_ENCODE_BASIC_FILTER(min, mag, mip,reduction) \")
+cpp_quote(" ((D3D12_FILTER) ( \") +cpp_quote(" (((min) & D3D12_FILTER_TYPE_MASK) <<
D3D12_MIN_FILTER_SHIFT) | \")
+cpp_quote(" (((mag) & D3D12_FILTER_TYPE_MASK) <<
D3D12_MAG_FILTER_SHIFT) | \")
+cpp_quote(" (((mip) & D3D12_FILTER_TYPE_MASK) <<
D3D12_MIP_FILTER_SHIFT) | \")
+cpp_quote(" (((reduction) & D3D12_FILTER_REDUCTION_TYPE_MASK) << D3D12_FILTER_REDUCTION_TYPE_SHIFT)))")
+cpp_quote("#define D3D12_ENCODE_ANISOTROPIC_FILTER(reduction) \") +cpp_quote(" ((D3D12_FILTER) ( \") +cpp_quote(" D3D12_ANISOTROPIC_FILTERING_BIT | \") +cpp_quote(" D3D12_ENCODE_BASIC_FILTER(D3D12_FILTER_TYPE_LINEAR, \") +cpp_quote(" D3D12_FILTER_TYPE_LINEAR, \") +cpp_quote(" D3D12_FILTER_TYPE_LINEAR, \") +cpp_quote(" reduction))) ")
We'd typically put the operators at the start of the next line; compare e.g. the existing D3D12_DECODE_IS_ANISOTROPIC_FILTER and D3D12_ENCODE_SHADER_4_COMPONENT_MAPPING macros.
Thanks; I believe I've addressed all of these comments now: - I made the `0xFFFFFFFF` lowercase (and added a trailing comma) - I changed all of the `num_foo` arguments into `foo_count` - Changed parameters `dest_foo` into `dst_foo` and `source_foo` into `src_foo` - Applied the suggested renamings on `ID3D12Device6` - Changed the `D3D12_ENCODE_BASIC_FILTER` and `D3D12_ENCODE_ANISOTROPIC_FILTER` macros to use operators at the start of the line, and removed a case of trailing unnecessary whitespace there.