Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Add support for SampleGrad() method

Merged Ethan Lee requested to merge flibitijibibo/vkd3d:samplegrad into master

This one's marked as a draft, as there seems to be a blocker with the method parameters.

The first commit totally works, if the ddx/ddy parameters are literals - they do not work when passing a variable of any kind. The test comes from tests/d3d12.c, so I'm mostly just trying to migrate that to the HLSL test suite, but it currently hits an assert before we get to the resource load (which does eventually work) and I'm not sure what's causing it:

vkd3d-compiler: libs/vkd3d-shader/tpf.c:3190: sm4_register_from_node: Assertion `instr->reg.allocated' failed.

Seems like it's surprised when we try to load from the constant buffer maybe? Fixed!

Edited by Ethan Lee

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
  • Author Contributor

    Pushed it a little bit harder and got it to compile this:

    Texture2D t;
    SamplerState s;
    
    float4 dd;
    
    float4 main(float4 position : SV_POSITION) : SV_Target
    {
        float2 p;
    
        p.x = position.x / 640.0f;
        p.y = position.y / 480.0f;
        float2 ddx = dd.xy / float2(1, 1);
        float2 ddy = dd.zw / float2(1, 1);
        return t.SampleGrad(s, p, ddx, ddy);
    }

    This just makes it so the compiler is forced to make a temp register for ddx/ddy, and that makes it happy. So temp registers work too... so why might load instructions be fussy with these last two parameters?

    Edited by Ethan Lee
    • Resolved by Ethan Lee

      Hi! I got this dissasembly from your test (replacing ddx = dd.xy; and ddy = dd.zw;):

      vkd3d:trace:hlsl_dump_function Function parameters:
      vkd3d:trace:hlsl_dump_function in float4 position : SV_POSITION0
      vkd3d:trace:hlsl_dump_function    2:     float4 | <input-SV_POSITION0>
      vkd3d:trace:hlsl_dump_function    3:     float4 | dd
      vkd3d:trace:hlsl_dump_function    4:      float | @2.x
      vkd3d:trace:hlsl_dump_function    5:      float | 6.40000000e+02 
      vkd3d:trace:hlsl_dump_function    6:      float | / (@4 @5 )
      vkd3d:trace:hlsl_dump_function    7:            | = (p.x @6)
      vkd3d:trace:hlsl_dump_function    8:      float | @2.y
      vkd3d:trace:hlsl_dump_function    9:      float | 4.80000000e+02 
      vkd3d:trace:hlsl_dump_function   10:      float | / (@8 @9 )
      vkd3d:trace:hlsl_dump_function   11:            | = (p.y @10)
      vkd3d:trace:hlsl_dump_function   12:     float2 | p
      vkd3d:trace:hlsl_dump_function   13:     float2 | @3.xy
      vkd3d:trace:hlsl_dump_function   14:     float2 | @3.zw
      vkd3d:trace:hlsl_dump_function   15:     float4 | sample_grad(resource = t, sampler = s, coords = @12, ddx = @13, ddy = @14)
      vkd3d:trace:hlsl_dump_function   16:            | = (<output-SV_Target0> @15)

      Using FIXME()'s bombardment ™️ I realized that the assertion fails because the register for instruction 13 is not allocated.

      This register is not allocated because compute_livenes_recurse is failing to inform that instructions 13 and 14 are used by sample_grad.

      Every time you add new fields to an instruction node type, that refer to other instruction nodes (generally hlsl_src) you have to perform certain updates to functions:

      • The clone_instr() in hlsl.c (well, actually one if its callees).
      • The compute_liveness_recurse() function in hlsl_codegen.c. (this causes the assertion failed in this case).

      If C had reflection we could get away with writing these functions in a way that doesn't require these updates.

      Edited by Francisco Casas
  • Ethan Lee added 2 commits

    added 2 commits

    • e4e95ca8 - vkd3d-shader/hlsl: Add support for SampleGrad() method
    • fb51a47d - tests: Add tests for SampleGrad() method.

    Compare with previous version

  • Author Contributor

    Latest update fixes the compilation issues, with the last TODO item being the test... currently it passes but I haven't copied over the texture contents, so the results are kind of useless - if anyone has some spare cycles between now and tomorrow afternoon and is in a copypasting mood, feel free to throw a diff at me!

  • Ethan Lee changed the description

    changed the description

  • Ethan Lee added 1 commit

    added 1 commit

    • 914e12f5 - tests: Add a basic compilation test for SampleGrad() method.

    Compare with previous version

  • Author Contributor

    Latest push updates the test to fall in line with the others in sampler.shader_test - like SampleBias it mostly just checks to make sure that it properly recognizes the function signature, and otherwise produces a result similar to plain Sample(). Porting over the full d3d12 test ended up being a pretty beefy task since it does a per-pixel test with mips, so I fell back to what the HLSL tests were already doing and will let the d3d12 test do the hard work 😅

  • Ethan Lee marked this merge request as ready

    marked this merge request as ready

  • Ethan Lee resolved all threads

    resolved all threads

  • Francisco Casas
  • Francisco Casas approved this merge request

    approved this merge request

  • Giovanni Mascellani
  • Giovanni Mascellani
  • Ethan Lee added 2 commits

    added 2 commits

    • e3e5eb98 - vkd3d-shader/hlsl: Add support for SampleGrad() method
    • ff25e441 - tests: Add a basic compilation test for SampleGrad() method.

    Compare with previous version

  • Ethan Lee added 2 commits

    added 2 commits

    • b050440c - vkd3d-shader/hlsl: Add support for SampleGrad() method
    • 0b4321d4 - tests: Add a basic compilation test for SampleGrad() method.

    Compare with previous version

  • Ethan Lee added 8 commits

    added 8 commits

    • 0b4321d4...7ba37394 - 6 commits from branch wine:master
    • 36e67dbe - vkd3d-shader/hlsl: Add support for SampleGrad() method
    • 8a22de15 - tests: Add a basic compilation test for SampleGrad() method.

    Compare with previous version

  • Ethan Lee added 23 commits

    added 23 commits

    • 8a22de15...abb207fa - 21 commits from branch wine:master
    • 8f18603c - vkd3d-shader/hlsl: Add support for SampleGrad() method
    • 2e0d0b6e - tests: Add a basic compilation test for SampleGrad() method.

    Compare with previous version

  • Ethan Lee added 6 commits

    added 6 commits

    • 719e4a68 - tests/shader_runner: Add support for creating mipmapped textures.
    • 99c13c01 - tests: Add a test for loading from nonzero mipmap levels.
    • bd229465 - tests: Add a test for sampling from nonzero mipmap levels.
    • 82519b62 - tests: Add a test for SampleBias() with multiple mipmap levels.
    • ade544e9 - tests: Add a test for SampleGrad() method
    • 532c146c - vkd3d-shader/hlsl: Add support for SampleGrad() method

    Compare with previous version

  • Author Contributor

    Rebased, but instead of rebasing on main I rebased on Zebediah's patchset - !191 (merged) should be prioritized before this MR!

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