vkd3d: Implement ID3D12Device1.
Merge request reports
Activity
I'm not quite sure how I feel about the d3d12_device_iface typedef, so I'd like to hear from others on that one. On the one hand, this is probably one of the few legitimate uses of "typedef". On the other hand, it does add a layer of obfuscation, and I'm not sure it's solving a problem that's in dire need of solving.
if (IsEqualGUID(riid, &IID_ID3D12Device) + || IsEqualGUID(riid, &IID_ID3D12Device1) || IsEqualGUID(riid, &IID_ID3D12Object) || IsEqualGUID(riid, &IID_IUnknown))
By convention, we check the newest IID first.
+typedef enum D3D12_PIPELINE_STATE_SUBOBJECT_TYPE +{ + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_ROOT_SIGNATURE = 0, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_VS = 1, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_PS = 2, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_DS = 3, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_HS = 4, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_GS = 5, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_CS = 6, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_STREAM_OUTPUT = 7, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_BLEND = 8, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_SAMPLE_MASK = 9, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_RASTERIZER = 10, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_DEPTH_STENCIL = 11, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_INPUT_LAYOUT = 12, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_IB_STRIP_CUT_VALUE = 13, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_PRIMITIVE_TOPOLOGY = 14, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_RENDER_TARGET_FORMATS = 15, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_DEPTH_STENCIL_FORMAT = 16, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_SAMPLE_DESC = 17, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_NODE_MASK = 18, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_CACHED_PSO = 19, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_FLAGS = 20, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_DEPTH_STENCIL1 = 21, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_VIEW_INSTANCING = 22, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_AS = 24, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_MS = 25, + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_MAX_VALID = 26, +} D3D12_PIPELINE_STATE_SUBOBJECT_TYPE;
We haven't been entirely consistent about it, but in general we prefer defining enum values in hexadecimal, matching the way we typically print them in debug traces.
+#define VKD3D_HANDLE_SUBOBJECT_EXPLICIT(type_enum, type_name, assignment) \ + case D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_ ## type_enum: \ + {\ + const struct\ + {\ + D3D12_PIPELINE_STATE_SUBOBJECT_TYPE type; \ + type_name data; \ + } *subobject = (void *)stream_ptr; \ + if (stream_ptr + sizeof(*subobject) > stream_end) \ + { \ + WARN("Invalid pipeline state stream.\n"); \ + return E_INVALIDARG; \ + } \ + stream_ptr += align(sizeof(*subobject), sizeof(void*)); \ + assignment; \ + break;\ + } + +#define VKD3D_HANDLE_SUBOBJECT(type_enum, type, left_side) \ + VKD3D_HANDLE_SUBOBJECT_EXPLICIT(type_enum, type, left_side = subobject->data)
Do we need the macros?
The ID3D12Device2 and depth bounds test bits probably each deserve their own series.
Do we need the macros?
Instead of using a fixed header size, it depends upon packing of each struct, so the only truly reliable way to get the data is declare a struct. Without the macro the first case becomes:
case D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_ROOT_SIGNATURE:
{
struct
{
D3D12_PIPELINE_STATE_SUBOBJECT_TYPE type;
ID3D12RootSignature *root_signature;
} *subobject = (void *)stream_ptr;
desc->root_signature = subobject->root_signature;
break;
}
It can be done that way if you prefer.
I'm not quite sure how I feel about the d3d12_device_iface typedef, so I'd like to hear from others on that one. On the one hand, this is probably one of the few legitimate uses of "typedef". On the other hand, it does add a layer of obfuscation, and I'm not sure it's solving a problem that's in dire need of solving.
Yeah, same here. It breaks a convention that is used throughout vkd3d and Wine, for what I see as little advantage. Updating the interface version happens pretty rarely, and even if the diff's got a lot of lines, it still is a patch that is rather quick to write and to review.
Do we need the macros?
Instead of using a fixed header size, it depends upon packing of each struct, so the only truly reliable way to get the data is declare a struct. Without the macro the first case becomes:
case D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_ROOT_SIGNATURE:
{
struct
{
D3D12_PIPELINE_STATE_SUBOBJECT_TYPE type;
ID3D12RootSignature *root_signature;
} *subobject = (void *)stream_ptr;
desc->root_signature = subobject->root_signature;
break;
}
It can be done that way if you prefer.
I think that's a fair bit more readable, yes, thanks.
We may be able to do slightly better though, if we build a table like this:
static const struct { size_t alignment; size_t size; size_t dst_offset; } subobject_info[] = { [D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_ROOT_SIGNATURE] = {__alignof__(ID3D12RootSignature *), sizeof(ID3D12RootSignature *), offsetof(struct d3d12_pipeline_state_desc, root_signature)}, [D3D12_PIPELINE_STATE_SUBOBJECT_TYPE_VS] = {__alignof__(ID3D12RootSignature *), sizeof(D3D12_SHADER_BYTECODE), offsetof(struct d3d12_pipeline_state_desc, vs)}, ... };
and then use that information to copy the data to the right location. We could even use macros to build that table if we're so inclined; I don't think it would be terribly necessary, but the macro in question would be simple enough and save some typing/reading.
-static inline struct d3d12_device *impl_from_ID3D12Device(ID3D12Device *iface) +static inline struct d3d12_device *impl_from_ID3D12Device(ID3D12Device1 *iface)
This introduces a mismatch between the function name and what it actually does. I think it's particularly jarring in places like vkd3d_get_vk_device().
static inline HRESULT d3d12_device_query_interface(struct d3d12_device *device, REFIID iid, void **object) { - return ID3D12Device_QueryInterface(&device->ID3D12Device_iface, iid, object); + return ID3D12Device_QueryInterface(&device->ID3D12Device1_iface, iid, object); } static inline ULONG d3d12_device_add_ref(struct d3d12_device *device) { - return ID3D12Device_AddRef(&device->ID3D12Device_iface); + return ID3D12Device_AddRef(&device->ID3D12Device1_iface); } static inline ULONG d3d12_device_release(struct d3d12_device *device) { - return ID3D12Device_Release(&device->ID3D12Device_iface); + return ID3D12Device_Release(&device->ID3D12Device1_iface); } static inline unsigned int d3d12_device_get_descriptor_handle_increment_size(struct d3d12_device *device, D3D12_DESCRIPTOR_HEAP_TYPE descriptor_type) { - return ID3D12Device_GetDescriptorHandleIncrementSize(&device->ID3D12Device_iface, descriptor_type); + return ID3D12Device_GetDescriptorHandleIncrementSize(&device->ID3D12Device1_iface, descriptor_type); }
We should use the ID3D12Device1 variants here.
I was slightly surprised the compiler didn't catch that; looks like we don't enable WIDL_C_INLINE_WRAPPERS. IIRC that was because it was slightly broken with older versions of MinGW-64, but perhaps we can revisit that.
added 93 commits
-
67d7427f...4f2e07a4 - 92 commits from branch
wine:master
- 27e418e9 - vkd3d: Implement ID3D12Device1 with stubs.
-
67d7427f...4f2e07a4 - 92 commits from branch
added 3 commits
-
27e418e9...8f8c89fb - 2 commits from branch
wine:master
- f3baf55d - vkd3d: Implement ID3D12Device1 with stubs.
-
27e418e9...8f8c89fb - 2 commits from branch