[PATCH 0/1] MR632: Draft: vkd3d: Implement d3d12_command_list_OMSetDepthBounds()
Spec https://learn.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-id3d12gra.... I am just unsure if checking the max and minimum range is necessary, or if that can be understood as programming error. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632
From: raphaelabrantes <antenabr2(a)gmail.com> --- libs/vkd3d/command.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index f9940b3d3..2327bf7a7 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -5897,7 +5897,13 @@ static void STDMETHODCALLTYPE d3d12_command_list_AtomicCopyBufferUINT64(ID3D12Gr static void STDMETHODCALLTYPE d3d12_command_list_OMSetDepthBounds(ID3D12GraphicsCommandList5 *iface, FLOAT min, FLOAT max) { - FIXME("iface %p, min %.8e, max %.8e stub!\n", iface, min, max); + TRACE("iface %p, min %.8e, max %.8e \n", iface, min, max); + struct d3d12_command_list *list = impl_from_ID3D12GraphicsCommandList5(iface); + const struct vkd3d_vk_device_procs *vk_procs; + + vk_procs = &list->device->vk_procs; + VK_CALL(vkCmdSetDepthBounds(list->vk_command_buffer, min, max)); + } static void STDMETHODCALLTYPE d3d12_command_list_SetSamplePositions(ID3D12GraphicsCommandList5 *iface, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632
Nikolay Sivov (@nsivov) commented about libs/vkd3d/command.c:
static void STDMETHODCALLTYPE d3d12_command_list_OMSetDepthBounds(ID3D12GraphicsCommandList5 *iface, FLOAT min, FLOAT max) { - FIXME("iface %p, min %.8e, max %.8e stub!\n", iface, min, max); + TRACE("iface %p, min %.8e, max %.8e \n", iface, min, max); + struct d3d12_command_list *list = impl_from_ID3D12GraphicsCommandList5(iface); + const struct vkd3d_vk_device_procs *vk_procs; + + vk_procs = &list->device->vk_procs; + VK_CALL(vkCmdSetDepthBounds(list->vk_command_buffer, min, max)); + } Do we need to update vulkan command buffer at all? I mean do these bounds affect anything else other than the next drawing command?
P.S. it is an actual question, because I don't know nearly enough to suggest that one way is better than the other. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632#note_60210
I am just unsure if checking the max and minimum range is necessary, or if that can be understood as programming error.
It's probably fine to leave that the way it currently is. For better or worse, both Vulkan and d3d12 generally don't verify these things and take the position that it's the responsibility of the application to pass valid values to the API.
Do we need to update vulkan command buffer at all? I mean do these bounds affect anything else other than the next drawing command?
P.S. it is an actual question, because I don't know nearly enough to suggest that one way is better than the other.
I'm not sure I completely understand the question, but this sets the parameters for the per fragment/sample depth bounds test, somewhat similar to e.g. d3d12_command_list_RSSetScissorRects() or d3d12_command_list_OMSetStencilRef(). The MR does have a couple of problems though: - It has the TRACE before the variable declarations, and that causes -Wdeclaration-after-statement warnings, and the corresponding CI failures. - The commit author ("raphaelabrantes <antenabr2(a)gmail.com>") isn't quite right. That's perhaps not immediately obvious in GitLab because GitLab displays the name configured on the account, but it's there in the actual git commit. - The TRACE has a trailing space before "\n", and is missing the full stop that most of our other TRACEs have. Ideally we'd also have some tests to go along with this, although the implementation seems straightforward enough... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632#note_60214
The question was about whether we need to set them right away, or store them separately, and set the once they are needed. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632#note_60215
The question was about whether we need to set them right away, or store them separately, and set the once they are needed.
We could store them and set them later, but I don't think we'd gain anything from that. We could revisit that if we think there's something to be gained, but as it is, it would just make the behaviour inconsistent with all the other command list functions. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/632#note_60217
participants (4)
-
Henri Verbeet (＠hverbeet) -
Nikolay Sivov (＠nsivov)
-
Raphael Abrantes (＠antenabr2)
-
raphaelabrantes