Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Support reflect() intrinsic.

Merged Nikolay Sivov requested to merge nsivov/vkd3d:hlsl_reflect into master
4 unresolved threads

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
1 [pixel shader]
2 float4 main(uniform float4 i, uniform float4 n) : sv_target
3 {
4 return reflect(i, n);
5 }
6
7 [test]
8 uniform 0 float4 0.5 -0.1 0.2 0.3
9 uniform 1 float4 0.6 0.4 -0.3 1.0
  • Giovanni Mascellani
    Giovanni Mascellani @giomasce started a thread on an outdated change in commit b7f74103
  • 2767 2767 return true;
    2768 2768 }
    2769 2769
    2770 static bool intrinsic_reflect(struct hlsl_ctx *ctx,
    2771 const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
    2772 {
    2773 struct hlsl_ir_node *i = params->args[0], *n = params->args[1];
    2774 struct hlsl_ir_node *dot, *mul_n, *two_dot, *neg;
    2775
    2776 if (!elementwise_intrinsic_float_convert_args(ctx, params, loc))
    2777 return false;
    2778
    2779 if (!(dot = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_DOT, i, n, loc)))
  • Nikolay Sivov added 1 commit

    added 1 commit

    • f3855a85 - vkd3d-shader/hlsl: Support reflect() intrinsic.

    Compare with previous version

  • Nikolay Sivov added 11 commits

    added 11 commits

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Nikolay Sivov added 6 commits

    added 6 commits

    Compare with previous version

  • Nikolay Sivov added 15 commits

    added 15 commits

    Compare with previous version

    • Author Developer

      Hi, @giomasce. Is anything blocking this MR?

    • I'd like to see more tests that reflect() really behaves like on native with different types combinations. For example, what happens if you call reflect() on a float and a float2? Or on a float2 and a float3? From my tests, it seems that the current implementation is correct, but it would be nice to have this shown by tests, so that we don't regress it in the future. It should be quite a quick thing to do.

    • Author Developer

      Thanks. Please take a look.

    • Please register or sign in to reply
  • Nikolay Sivov added 1 commit

    added 1 commit

    • 661b0f59 - vkd3d-shader/hlsl: Support reflect() intrinsic.

    Compare with previous version

  • 32 uniform float2 i;
    33 uniform float4 n;
    34
    35 float4 main() : sv_target
    36 {
    37 return reflect(i, n);
    38 }
    39
    40 [pixel shader fail]
    41 uniform float3 i;
    42 uniform float2 n;
    43
    44 float4 main() : sv_target
    45 {
    46 return reflect(i, n);
    47 }
    • Comment on lines +31 to +47

      These two are slightly misleading. The call to reflect() is totally fine, the test fails because it cannot convert float2 to float4 when returning.

      I am attaching the tests as I fixed them, which probably better pinpoint the behavior of reflect():

      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          return reflect(i, n);
      }
      
      [test]
      uniform 0 float4 0.5 -0.1 0.2 0.3
      uniform 4 float4 0.6 0.4 -0.3 1.0
      draw quad
      probe all rgba (-0.1, -0.5, 0.5, -0.7) 4
      
      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          float i1 = i.x;
      
          return reflect(i1, n);
      }
      
      [test]
      uniform 0 float4 0.5 0.0 0.0 0.0
      uniform 4 float4 0.6 0.4 -0.3 1.0
      draw quad
      probe all rgba (-0.52, -0.18, 1.01, -1.2) 4
      
      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          float n1 = n.x;
      
          return reflect(i, n1);
      }
      
      [test]
      uniform 0 float4 0.5 -0.1 0.2 0.3
      uniform 4 float4 0.6 0.0 0.0 0.0
      draw quad
      probe all rgba (-0.148, -0.748, -0.448, -0.348) 4
      
      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          float i1 = i.x;
          float n1 = n.x;
      
          return reflect(i1, n1);
      }
      
      [test]
      uniform 0 float4 0.5 0.0 0.0 0.0
      uniform 4 float4 0.6 0.0 0.0 0.0
      draw quad
      probe all rgba (0.14, 0.14, 0.14, 0.14) 4
      
      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          float2 i1 = i.xy;
      
          return float4(reflect(i1, n), 0.0, 0.0);
      }
      
      [test]
      uniform 0 float4 0.5 -0.1 0.0 0.0
      uniform 4 float4 0.6 0.4 -0.3 1.0
      draw quad
      probe all rgba (0.188, -0.308, 0.0, 0.0) 4
      
      [pixel shader]
      uniform float4 i;
      uniform float4 n;
      
      float4 main() : sv_target
      {
          float3 i1 = i.xyz;
          float2 n1 = n.xy;
      
          return float4(reflect(i1, n1), 0.0, 0.0);
      }
      
      [test]
      uniform 0 float4 0.5 -0.1 0.2 0.0
      uniform 4 float4 0.6 0.4 0.0 0.0
      draw quad
      probe all rgba (0.188, -0.308, 0.0, 0.0) 4

      The reason why in some tests I declare a float4 uniform and then use only some components is that if I declare a float uniform the layout might change between SM1 and SM4.

    • Author Developer

      Thanks, I pushed these tests. d3d11+ does not fail for me, but d3d9 does, and that appears to be unrelated regression.

    • Author Developer

      Filed a report for sm1 issue https://bugs.winehq.org/show_bug.cgi?id=54522.

    • Please register or sign in to reply
  • Nikolay Sivov added 3 commits

    added 3 commits

    Compare with previous version

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet mentioned in merge request !88 (merged)

    mentioned in merge request !88 (merged)

  • added 1 commit

    • e7bc6343 - vkd3d-shader/hlsl: Support reflect() intrinsic.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading