Skip to content
Snippets Groups Projects

vkd3d-shader: Implement tex2Dproj().

Merged Nikolay Sivov requested to merge nsivov/vkd3d:tex2dproj into master

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
    • I suggest adding at least this test at the end of tests/sampler.shader_test:

      [pixel shader todo]
      sampler sam;
      float4 p;
      
      float4 main() : sv_target
      {
          return tex2Dproj(sam, p);
      }
      
      [test]
      uniform 0 float4 1.0 1.0 1234.0 2.0
      todo draw quad
      todo probe all rgba (0.25, 0.0, 0.25, 0.0)

      I confirmed that is the expected behavior using the minitestbot.

      Even though to get this to pass we need to lower combined sample expressions, but that seems close in the horizon (!209 (merged)).

      Edited by Francisco Casas
    • Author Developer

      Yes, that was exactly the reason for a lack of tests. I'm going to wait for !209 (merged) to get in first.

    • Please register or sign in to reply
  • Francisco Casas
    Francisco Casas @fcasas started a thread on an outdated change in commit d291b9f0
  • 2455 unsigned int dim_count;
    2456
    2457 if (instr->type != HLSL_IR_RESOURCE_LOAD)
    2458 return false;
    2459 load = hlsl_ir_resource_load(instr);
    2460 if (load->load_type != HLSL_RESOURCE_SAMPLE_PROJ)
    2461 return false;
    2462
    2463 dim_count = hlsl_sampler_dim_count(load->sampling_dim);
    2464 if (!(swizzle = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(W, W, W, W), dim_count, load->coords.node, &instr->loc)))
    2465 return false;
    2466 list_add_before(&instr->entry, &swizzle->entry);
    2467
    2468 operands[0] = load->coords.node;
    2469 operands[1] = swizzle;
    2470 if (!(coords = hlsl_new_expr(ctx, HLSL_OP2_DIV, operands, hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, dim_count),
    • Since the result has the same type as the operands, you can use hlsl_new_binary_expr() here.

    • Author Developer

      It does not accept coords.node / swizzle, because types are different, I think.

    • Just to summarize what we discussed in IRC: there are few hlsl_ir_expr ops that are allowed to handle different types of arguments and return value. My understanding is that HLSL_OP2_DIV is not one of them (like, e.g. HLSL_OP2_DOT). When replacing this hlsl_new_expr() call with a call to hlsl_new_binary_expr(), the assertion should hold true.

      This is not happening however, because load->coords.node is float4 and the swizzle is float2 (or float3 for tex3Dproj). What is missing is to create a new .XY (or .XYZ) swizzle for load->coords.node and pass it as arg1 instead of load->coords.node directly.

      Edited by Francisco Casas
    • Author Developer

      Turns out 1D case works differently. It's using 2d resource for all profiles, using (x/w, 0.5) as a coordinate in SM4+, I think this justifies moving profile-specific logic out of codegen.c, back to the function.

    • Author Developer

      Another option is to retain original 1D dim value and carry it all the way to lower_tex_proj(), but it will produce awkward combination of dim and coordinates types that is resolved too late and for each binary format, when writing declarations.

    • After some thought, I think that what we are missing is a pass that lowers 1D combined samples to 2D. But only combined samples. So we end up with something like:

      // Only 1D combined samples (with load->sampler == NULL) get converted into 2D.
      hlsl_transform_ir(ctx, lower_1d_combined_samples_to_2d, body, NULL);
      // ...
      if (profile->major_version >= 4)
      {
          hlsl_transform_ir(ctx, lower_tex_proj, body, NULL);
      }
      // ...
      if (profile->major_version >= 4)
      {
          hlsl_transform_ir(ctx, lower_combined_samples, body, NULL); // From !209
      }
      // ...

      If we do this, you wouldn't have to worry about 1D HLSL_RESOURCE_SAMPLE_PROJ here.

      Edited by Francisco Casas
    • nevermind, that is not a solution because you would end up dividing 0.5 by w anyways.

    • Native pretty much throws whatever junk into the y coordinate for regular tex1D(). It's more consistent with tex1Dproj() but I don't think that's saying much. I doubt we want to worry that much about what we're putting in the y coordinate.

      I also don't think that tex1D() should really emit anything 1d. It seems like it should just emit 2D IR.

    • Nikolay Sivov changed this line in version 2 of the diff

      changed this line in version 2 of the diff

    • Please register or sign in to reply
    • So, SM1 doesn't care what we pass in the Y component of the src0 of texld nor texldp when we are working with tex1D() and tex1Dproj(), we don't have to worry about making any transformation to 2D.

      It is for SM4 that we have to care about making the second coordinate 0.5. Since this is also true for tex1D(), my opinion is that this should be deferred to a later pass:

      if (profile->major_version >= 4)
      {
          // Lower [tex1Dproj -> 1d sample] normally.
          hlsl_transform_ir(ctx, lower_tex_proj, body, NULL);
          // Add a 0.5 component to the coords of 1D combined sample resource loads, making them 2D.
          hlsl_transform_ir(ctx, lower_1d_combined_samples_to_2d, body, NULL);
      }
      // ...
      hlsl_transform_ir(ctx, lower_combined_samples, body, NULL); // as in !209
      Edited by Francisco Casas
    • Author Developer

      That means tex1D* should produce 1D or 2D load initially, depending on profile. It might as well set coordinates correctly at the same time.

    • No, my proposal is to always produce a 1D load, which should be promoted to a 2D load by a SM4-specific pass.

    • I don't see how it ever makes sense to emit a 1D load in the IR from intrinsic_tex1D(). The load is fundamentally 2D, regardless of profile version. Lowering is a thing to deal with differences between profile bytecode, or high-level constructions that can't easily be emitted already lowered; I don't see how either applies here?

    • Author Developer

      It's not to make sense as a load, but to be able to tell them apart later. If it was tex2D() initially there is nothing else to do, if it was tex1D(), SM4 needs to do something (checking for combined sampling case to distinguish it from regular SM4 1D load).

      What was proposed is to produce something "common" for functions, and then have multiple passes to fix that up, instead of producing final result immediately.

    • It's still not clear to me that we need to treat the combined sampling case specially, though.

    • Author Developer

      I don't understand what you are proposing to do. Treat combined case specially where? Turning all 1D to 2D right away does not work without extra logic to adjust coordinates in intrinsic_tex() for sm4, or later.

    • I'm trying to say that we can just implement tex1D(s, coords) as if it were tex2D(s, float2(coords.x, 0.5)) in the intrinsic handler. Similarly tex1Dproj(s, coords) can be tex2Dproj(s, float4(coords.x, 0.5, 0, coords.w)). The only potential difference is that native throws different garbage into the y coordinate, depending on shader target, but I'm not really sure that it matters.

    • Please register or sign in to reply
  • Francisco Casas mentioned in merge request !209 (merged)

    mentioned in merge request !209 (merged)

  • Nikolay Sivov added 5 commits

    added 5 commits

    • ff7d6956 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • b96e4fbb - vkd3d-shader/hlsl: Implement tex1Dproj().
    • d7ee8062 - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 6bed0f5c - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 2be036db - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Author Developer

    Alright, pushed something that should work.

  • Francisco Casas approved this merge request

    approved this merge request

  • Nikolay Sivov added 12 commits

    added 12 commits

    • 2be036db...ebf75735 - 7 commits from branch wine:master
    • 9af19825 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 5aef0583 - vkd3d-shader/hlsl: Implement tex1Dproj().
    • 4b102240 - vkd3d-shader/hlsl: Implement tex3Dproj().
    • f4ae421f - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 6d33de96 - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Nikolay Sivov added 68 commits

    added 68 commits

    • 6d33de96...d5a0b3af - 63 commits from branch wine:master
    • 9228c3c6 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 2c3b5605 - vkd3d-shader/hlsl: Implement tex1Dproj().
    • 2437b1fc - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 3e12f71c - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 8d577925 - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Nikolay Sivov added 96 commits

    added 96 commits

    • 8d577925...f95ab2a5 - 91 commits from branch wine:master
    • edd6a89c - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 8e6e34ed - vkd3d-shader/hlsl: Implement tex1Dproj().
    • 7ff00d19 - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 31f5326f - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 5bde8419 - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Nikolay Sivov added 11 commits

    added 11 commits

    • 5bde8419...3d49b59a - 6 commits from branch wine:master
    • 196e28f2 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 15a76316 - vkd3d-shader/hlsl: Implement tex1Dproj().
    • b5d7401a - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 098dac4d - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 1c30a5af - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Nikolay Sivov added 13 commits

    added 13 commits

    • 1c30a5af...0d1bc77b - 8 commits from branch wine:master
    • 9883c297 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 37dca4be - vkd3d-shader/hlsl: Implement tex1Dproj().
    • 0135b967 - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 8e5766a6 - vkd3d-shader/d3dbc: Disallow 1D sampler types when writing sampler declaration.
    • 3ba8b20c - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Nikolay Sivov added 161 commits

    added 161 commits

    • 3ba8b20c...45541dd9 - 157 commits from branch wine:master
    • 53220d11 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 8d0c2026 - vkd3d-shader/hlsl: Implement tex1Dproj().
    • d1edfe8a - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 936f8512 - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Nikolay Sivov added 332 commits

    added 332 commits

    • 936f8512...0805ce12 - 329 commits from branch wine:master
    • 25d9ae36 - vkd3d-shader/hlsl: Implement tex2Dproj().
    • 4c7f83fc - vkd3d-shader/hlsl: Implement tex3Dproj().
    • 2c19a5a6 - vkd3d-shader/hlsl: Implement texCUBEproj().

    Compare with previous version

  • Author Developer

    I dropped tex1Dproj() for now, it needs a bigger change that I'll make later, as a separate MR.

  • Henri Verbeet approved this merge request

    approved this merge request

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading