Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Support complex implicit casts, complex explicit casts and complex broadcasts. (PART 1/2)

Merged Francisco Casas requested to merge fcasas/vkd3d:complex_broadcasts_1 into master
4 unresolved threads

Hello, this is the first part of a patch series to support:

  • Complex broadcasts.
  • Complex implicit casts between component-wise equal types.
  • Complex explicit casts between component-wise compatible types.

This first part of the series includes mostly tests.

Following patches in: https://gitlab.winehq.org/fcasas/vkd3d/-/commits/complex_broadcasts_2/

This supersedes !16 (closed) .

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Francisco Casas mentioned in merge request !16 (closed)

    mentioned in merge request !16 (closed)

  • Francisco Casas added 10 commits

    added 10 commits

    • 69e501b2...6b45f290 - 5 commits from branch wine:master
    • 76a02bee - tests: Set ULPs to 2 in normalize() test.
    • 57d264c2 - tests: Test for invalid complex broadcasts.
    • 2acce9f4 - vkd3d-shader/hlsl: Always go through implicit conversion in assignments.
    • a4c0e0f6 - tests: Test implicit casts between types that are equal component-wise.
    • cfd3289c - tests: Test explicit casts between types that are component-wise

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

    • Zebediah Figura replied on the mailing list:

      On 10/13/22 11:40, Francisco Casas wrote:
      > From: Francisco Casas <fcasas@codeweavers.com>
      > 
      > ---
      >   Makefile.am                                |   1 +
      >   tests/cast-componentwise-equal.shader_test | 181 +++++++++++++++++++++
      >   2 files changed, 182 insertions(+)
      >   create mode 100644 tests/cast-componentwise-equal.shader_test
      
      So this is great on its own, but naturally it makes me wonder about even 
      more tests :-)
      
      Currently we're testing vector -> struct (illegal), struct -> vector 
      (illegal), array -> struct (legal iff the types match), and struct -> 
      struct (legal [iff the types match?]). We don't test struct -> array, 
      though, and we also don't test struct -> struct with mismatched types.
      
      Similarly with the explicit cast tests, I think there's no tests there 
      for vector -> struct or struct -> vector casts.
      
      That all might not make a difference to the patches you have queued (I 
      think the concern at this point is just about objects?), but I'd be 
      remiss if I didn't at least write down every interesting test that 
      occured to me ;-)
  • In a couple of commit messages you put a --- marker. I guess that means that you don't want the message below that point to be committed. That used to work with the previous workflow because AJ imported the patch with git am, but I am not sure it works any more. I think you should remove from the --- onward if you don't want it to be committed. @julliard, could you please comment on that?

    BTW, I am generally in favor of verbose commit messages rather than not, so I think that some of the messages that you want to see discarded are instead sensible to keep in the git history. For example the note on the video card on which the test fails.

  • Giovanni Mascellani
1 1 [pixel shader]
2
3 struct foo
2 struct apple
  • The "While at it" expression suggests that you're doing two different things in the same patch. I think it's better to split it up, even if this means creating a very silly patch just renaming test structs.

  • Please register or sign in to reply
  • Giovanni Mascellani
  • 5 int bb;
    6 };
    7
    8 float4 main() : sv_target
    9 {
    10 float f[4] = {1, 2, 3, 4};
    11 struct apple a;
    12
    13 a = (struct apple) f;
    14 return a.aa.xyzx;
    15 }
    16
    17
    18 [test]
    19 todo draw quad
    20 todo probe all rgba (1.0, 2.0, 3.0, 1.0)
  • In a couple of commit messages you put a --- marker. I guess that means that you don't want the message below that point to be committed. That used to work with the previous workflow because AJ imported the patch with git am, but I am not sure it works any more. I think you should remove from the --- onward if you don't want it to be committed. @julliard, could you please comment on that?

    It still works, though ideally comments that are not meant to be committed would be put in the MR description instead.

    • Francisco Casas replied on the mailing list:

      
      
      On 13-10-22 18:51, Zebediah Figura wrote:
      > On 10/13/22 11:40, Francisco Casas wrote:
      >> From: Francisco Casas <fcasas@codeweavers.com>
      >>
      >> ---
      >>   Makefile.am                                |   1 +
      >>   tests/cast-componentwise-equal.shader_test | 181 +++++++++++++++++++++
      >>   2 files changed, 182 insertions(+)
      >>   create mode 100644 tests/cast-componentwise-equal.shader_test
      > 
      > So this is great on its own, but naturally it makes me wonder about even 
      > more tests :-)
      > 
      > Currently we're testing vector -> struct (illegal), struct -> vector 
      > (illegal), array -> struct (legal iff the types match), and struct -> 
      > struct (legal [iff the types match?]). We don't test struct -> array, 
      > though, and we also don't test struct -> struct with mismatched types.
      > 
      > Similarly with the explicit cast tests, I think there's no tests there 
      > for vector -> struct or struct -> vector casts.
      > 
      > That all might not make a difference to the patches you have queued (I 
      > think the concern at this point is just about objects?), but I'd be 
      > remiss if I didn't at least write down every interesting test that 
      > occured to me ;-)
      > 
      > 
      
      Ok, I am adding these tests in v2.
      
      It made a small difference: it seems that explicit casts are allowed 
      between vectors and structs/arrays with less components.
  • Francisco Casas added 6 commits

    added 6 commits

    • ff7ef7f0 - tests: Set ULPs to 2 in normalize() test.
    • 1152c9bb - tests: Rename structs for readability in broadcast test.
    • fb9c13ea - tests: Test for invalid complex broadcasts.
    • 36347f33 - vkd3d-shader/hlsl: Always go through implicit conversion in assignments.
    • 41a66268 - tests: Test implicit casts between types that are equal component-wise.
    • c4b59eeb - tests: Test explicit casts between types that are component-wise

    Compare with previous version

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • added 14 commits

    • c4b59eeb...ab8f4182 - 8 commits from branch wine:master
    • 3bae0c92 - tests: Set ULPs to 2 in normalize() test.
    • 791f8a8f - tests: Rename structs for readability in broadcast test.
    • e56db9bd - tests: Test for invalid complex broadcasts.
    • 5a1b0dbf - vkd3d-shader/hlsl: Always go through implicit conversion in assignments.
    • 653b109d - tests: Test implicit casts between types that are equal component-wise.
    • 09d7442c - tests: Test explicit casts between types that are component-wise compatible.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading