On Wed Nov 15 21:05:11 2023 +0000, Zebediah Figura wrote:
> > Because it doesn't really matter,
> On what grounds?
> > and it makes our implementation simpler.
> It makes our implementation simpler to type E_INVALIDARG rather than HRESULT_FROM_WIN32(ERROR_INVALID_NAME)?
In the past when I've added tests for behavior that differs from one Windows version to another, I've been asked to designate one behavior as the "correct" behavior and mark the other as broken. In this case, the applications in question require the function to report success, so testing and implementing the function's argument checking is really just to help us understand what the arguments are. The patch already has an if statement that returns E_INVALIDARG, and it would be slightly simpler to add to the existing if statement than to add a second if statement to return HRESULT_FROM_WIN32(ERROR_INVALID_NAME).
If you would prefer to return HRESULT_FROM_WIN32(ERROR_INVALID_NAME) instead, or to not handle that error case at all in the stub implementation (as Fabian originally proposed), that's totally fine. The applications we're trying to support will work either way.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52417
Currently we are not properly handling register(cX) reservations for SM1, this is one of the things required for the SNK shaders (CW Bug Bug 18092).
register(cX) reservations also change the offset in the $Globals buffer in SM4, so support for this is also included.
---
Patch 1/4 is required to specify:
```
[require]
shader model < 4.0
```
so that the tests that follow do not get run with the vulkan backend on SM4. I think nobody disagreed with that patch.
--
v5: vkd3d-shader/hlsl: Turn register(cX) reservations into buffer offset for SM4.
vkd3d-shader/hlsl: Make register(cX) reservations work for SM1.
tests: Test register(cX) reservations.
tests: Rename register-reservations.shader_test to register-reservations-resources.shader_test.
tests/shader-runner: Run compilation tests with SM1 when SM1 models are selected.
tests/shader-runner: Allow passing (sm<4) and (sm>=4) to "fail" and "todo" qualifiers.
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/458
> Because it doesn't really matter,
On what grounds?
> and it makes our implementation simpler.
It makes our implementation simpler to type E_INVALIDARG rather than HRESULT_FROM_WIN32(ERROR_INVALID_NAME)?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52414
On Wed Nov 15 20:52:54 2023 +0000, Zebediah Figura wrote:
> > assume that ERROR_INVALID_NAME is broken behavior, and make Wine
> return E_INVALIDARG in this situation.
> Why?
Because it doesn't really matter, and it makes our implementation simpler.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52413
Currently we are not properly handling register(cX) reservations for SM1, this is one of the things required for the SNK shaders (CW Bug Bug 18092).
register(cX) reservations also change the offset in the $Globals buffer in SM4, so support for this is also included.
---
Patch 1/4 is required to specify:
```
[require]
shader model < 4.0
```
so that the tests that follow do not get run with the vulkan backend on SM4. I think nobody disagreed with that patch.
--
v4: vkd3d-shader/hlsl: Turn register(cX) reservations into buffer offset for SM4.
vkd3d-shader/hlsl: Make register(cX) reservations work for SM1.
tests: Test register(cX) reservations.
tests: Rename register-reservations.shader_test to register-reservations-resources.shader_test.
tests/vkd3d-shader: Allow passing (sm<4) and (sm>=4) to "fail" and "todo" qualifiers.
tests/shader-runner: Discern between the minimum_shader_model and the selected_shader_model.
tests/shader-runner: Add missing requirement checks for backends.
ci: Execute the shader runner on the correct test data on Windows.
ci: Deduplicate the CI configuration for Windows.
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/458