On Tue Oct 31 16:07:18 2023 +0000, Giovanni Mascellani wrote:
Also, I would reorder the commits in roughly this way: first the tests, then the HLSL compiler, then the SPIR-V and vkd3d bits. The idea is that you implement features in the same order they appear in the pipeline, so that reviewers can follow patches in their order and build on top of earlier patches at each step. As for commit messages, I think that in general the following features are appreciated:
- Terminate each sentence, including the commit subject, with a full stop.
- Avoid nominal sentences, like "ROV support", but rather describe what
is actually happening with a verb.
- When possible, use specific verbs. I won't say that "support" is
banned, but often there are better alternatives, like: "Parse ROV types", "Enable EXT_fragment_shader_interlock", "Support SV_PrimitiveID in pixel shaders" (using "pixel shader" rather than "fragment shader" because that change is in the context of the TPF format).
- This is probably my taste more than anything, but at least when a
certain acronym is often associated with a specific capitalization, I'd use it. Case in point: "SPIR-V".
I reordered the commits.
One minus of adding tests first is in b1a7c4efc3e1428e05bc3c1d509495272a00fbdf, the todos can fail if your GPU runs everything ordered regardless of FSI (e.g. my Intel iGPU and probably llvmpipe).
Commit https://gitlab.winehq.org/wine/vkd3d/-/commit/c1de65a99ba851f97cb5f345b66a2b... should do what you want, if I interpret it well.
It does not. For an example of what I mean, check out 0605202ea21b58ee826831586f4d4e138eaecc80 and run `build/tests/shader_runner tests/hlsl/rasterizer-ordered-views.shader_test`, and compare it to the results on the final commit. The test succeeds, but doesn't run at all on vkd3d because vkd3d doesn't advertise ROV support. The only indication that the test is succeeding due to lack of support (rather than actually running the test) is the fact that there's ~50 fewer tests run than there should be. Which means that if you mess up adding the support flag (e.g. because you checked FSI status before `vkd3d_check_extensions` is run), you'll still pass the tests and might not notice that your code isn't actually being tested.