Skip to content

Add Graphics tests for ddx/y_fine #510

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Alexander-Johnston wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add Graphics tests for ddx/y_fine #510

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a3d015
Add Graphics tests for ddx/y_fine
Alexander-Johnston Nov 20, 2025
5140b87
Update supported test matrix now that SV_TARGET is supported
Alexander-Johnston Dec 11, 2025
506c482
Update style and formatting
Alexander-Johnston Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions test/Graphics/ddx_fine.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#--- vertex.hlsl
struct PSInput
{
float4 position : SV_POSITION;
};
Copy link
Collaborator

@bogner bogner Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow llvm-style in these tests. So here, we'd want to use braces like so:

Suggested change
struct PSInput
{
float4 position : SV_POSITION;
};
struct PSInput {
float4 position : SV_POSITION;
};

It's a bit annoying because of how the files are embedded, but you can run clang-format on the split files in the build directory or copy the code to a temp file to do the same, which makes it easier to be consistent.


PSInput main(float4 position : POSITION)
{
PSInput result;

result.position = position;

return result;
}


#--- pixel.hlsl

// Offset into the image of our circle
#define offset 128.0
Copy link
Collaborator

@bogner bogner Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably better to either name this OFFSET following the usual preprocessor macro naming conventions or make it a static const float instead of using a define.

// Radius of our circle
#define radius 32.0
// Blur intensity
#define intensity 5.0

struct PSInput
{
float4 position : SV_POSITION;
};

float4 main(PSInput input) : SV_TARGET
{
float4 c1 = float4(1.0, 1.0, 1.0, 1.0);
float4 c2 = float4(0.0, 0.0, 0.0, 1.0);

float2 coord = input.position.xy - offset;

float dist = length(coord) - radius;
float ddx = abs(ddx_fine(dist));
float effect = clamp(dist / ddx / intensity, 0.0, 1.0);

float result = lerp(c1, c2, effect);

return float4(result, result, result, 1.0);
}
#--- pipeline.yaml
---
Shaders:
- Stage: Vertex
Entry: main
- Stage: Pixel
Entry: main
Buffers:
- Name: VertexData
Format: Float32
Stride: 12 # 16 bytes per vertex
Data: [ -1.0, -1.0, 0.0,
-1.0, 1.0, 0.0,
1.0, 1.0, 0.0,
1.0, 1.0, 0.0,
1.0, -1.0, 0.0,
-1.0, -1.0, 0.0 ]
- Name: Output
Format: Float32
Channels: 4
FillSize: 1048576 # 256x256 @ 16 bytes per pixel
OutputProps:
Height: 256
Width: 256
Depth: 16
Bindings:
VertexBuffer: VertexData
VertexAttributes:
- Format: Float32
Channels: 3
Offset: 0
Name: POSITION
RenderTarget: Output
DescriptorSets: []
...
#--- rules.yaml
---
- Type: PixelPercent
Val: 0.2 # No more than 0.2% of pixels may be visibly different.
...
#--- end

# UNSUPPORTED: Clang
Copy link
Member

@farzonl farzonl Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you merge the clang changes?

Copy link
Author

@Alexander-Johnston Alexander-Johnston Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ddx/y_coarse was merged, but _fine is waiting for review.

Regardless, all the graphics tests current fail on clang, including the original SimpleTriangle.test, because of other issues.
The output from clang-d3d12-graphics for all tests is

# .---command stderr------------
# | error: Unsupported intrinsic llvm.dx.load.input.v4f32 for DXIL lowering
# | error: Unsupported intrinsic llvm.dx.store.output.v4f32 for DXIL lowering
# | 2 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

Copy link
Member

@farzonl farzonl Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting can you file an issue and I'll triage it.

Copy link
Author

@Alexander-Johnston Alexander-Johnston Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some more testing and created #540 (comment).
I'm happy to look into the failure if you'd like

Copy link
Contributor

@Icohedron Icohedron Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's only failing with DirectX + Clang, you should do this instead:

Suggested change
# UNSUPPORTED: Clang
# Bug https://github.com/llvm/offload-test-suite/issues/540
# XFAIL: Clang && DirectX

Copy link
Member

@farzonl farzonl Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the CI indicates it is failing on vulkan too. The only runner that is passing is the metal one. I agree these need to be changed to XFAIL. But maybe it should be Clang && (DirectX || Vulkan)?

Copy link
Author

@Alexander-Johnston Alexander-Johnston Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the runners failed because it couldn't find the image in the golden-images repo. For this and the other graphics tests I've added to run the corresponding PRs for the image comparison will need merged first.

I've updated this to XFAIL: Clang && DirectX as I believe once llvm/offload-golden-images#5 is merged it will pass on Vulkan. I'm unsure what will happen with Metal.

Copy link
Member

@farzonl farzonl Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t have permissions in that repository. Will follow up with @llvm-beanz

# REQUIRES: goldenimage

# RUN: split-file %s %t
# RUN: %dxc_target -T vs_6_0 -Fo %t-vertex.o %t/vertex.hlsl
# RUN: %dxc_target -T ps_6_0 -Fo %t-pixel.o %t/pixel.hlsl
# RUN: %offloader %t/pipeline.yaml %t-vertex.o %t-pixel.o -r Output -o %t/Output.png
# RUN: imgdiff %t/Output.png %goldenimage_dir/hlsl/Graphics/DdxFine.png -rules %t/rules.yaml
Copy link
Member

@farzonl farzonl Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see an uploaded image in this PR. How are you testing this?

Copy link
Author

@Alexander-Johnston Alexander-Johnston Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graphics test images are in a different repo. The PR for them is here llvm/offload-golden-images#5

95 changes: 95 additions & 0 deletions test/Graphics/ddy_fine.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#--- vertex.hlsl
struct PSInput
{
float4 position : SV_POSITION;
};

PSInput main(float4 position : POSITION)
{
PSInput result;

result.position = position;

return result;
}


#--- pixel.hlsl

// Offset into the image of our circle
#define offset 128.0
// Radius of our circle
#define radius 32.0
// Blur intensity
#define intensity 5.0

struct PSInput
{
float4 position : SV_POSITION;
};

float4 main(PSInput input) : SV_TARGET
{
float4 c1 = float4(1.0, 1.0, 1.0, 1.0);
float4 c2 = float4(0.0, 0.0, 0.0, 1.0);

float2 coord = input.position.xy - offset;

float dist = length(coord) - radius;
float ddx = abs(ddy_fine(dist));
float effect = clamp(dist / ddx / intensity, 0.0, 1.0);

float result = lerp(c1, c2, effect);

return float4(result, result, result, 1.0);
}
#--- pipeline.yaml
---
Shaders:
- Stage: Vertex
Entry: main
- Stage: Pixel
Entry: main
Buffers:
- Name: VertexData
Format: Float32
Stride: 12 # 16 bytes per vertex
Data: [ -1.0, -1.0, 0.0,
-1.0, 1.0, 0.0,
1.0, 1.0, 0.0,
1.0, 1.0, 0.0,
1.0, -1.0, 0.0,
-1.0, -1.0, 0.0 ]
- Name: Output
Format: Float32
Channels: 4
FillSize: 1048576 # 256x256 @ 16 bytes per pixel
OutputProps:
Height: 256
Width: 256
Depth: 16
Bindings:
VertexBuffer: VertexData
VertexAttributes:
- Format: Float32
Channels: 3
Offset: 0
Name: POSITION
RenderTarget: Output
DescriptorSets: []
...
#--- rules.yaml
---
- Type: PixelPercent
Val: 0.2 # No more than 0.2% of pixels may be visibly different.
...
#--- end

# UNSUPPORTED: Clang
Copy link
Member

@farzonl farzonl Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you merge the clang changes?

# REQUIRES: goldenimage

# RUN: split-file %s %t
# RUN: %dxc_target -T vs_6_0 -Fo %t-vertex.o %t/vertex.hlsl
# RUN: %dxc_target -T ps_6_0 -Fo %t-pixel.o %t/pixel.hlsl
# RUN: %offloader %t/pipeline.yaml %t-vertex.o %t-pixel.o -r Output -o %t/Output.png
# RUN: imgdiff %t/Output.png %goldenimage_dir/hlsl/Graphics/DdyFine.png -rules %t/rules.yaml
Copy link
Member

@farzonl farzonl Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here.

Loading