Skip to content

[Bug] Add COMPARISON_NONE; fix the error message related to it - #8245

Merged
Finn Plummer (inbelic) merged 3 commits into
microsoft:mainfrom
ModulePillow:pillow-patch
Mar 12, 2026
Merged

[Bug] Add COMPARISON_NONE; fix the error message related to it#8245
Finn Plummer (inbelic) merged 3 commits into
microsoft:mainfrom
ModulePillow:pillow-patch

Conversation

@ModulePillow

@ModulePillow ModulePillow commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Bug Description

Regarding the sampler descriptions, I found two issues:

  1. D3D12_COMPARISON_FUNC_NONE should be added; it is a valid value for the non-comparison samplers.
  2. The error message should be revised; the original one shows "Unexpected texture address mode value", which is confusing.

Comments

  1. D3D12_COMPARISON_FUNC_NONE is a valid enumerator. It's listed in d3d12.h.
  2. I verified this value by PIX; the result shown below confirmed my expectation.
image

@ModulePillow ModulePillow changed the title Add COMPARISON_NONE; fix the error message [Bug] Add COMPARISON_NONE; fix the error message related to it Mar 9, 2026
@ModulePillow ModulePillow mentioned this pull request Mar 9, 2026
Closed
@damyanp Damyan Pepper (damyanp) linked an issue Mar 10, 2026 that may be closed by this pull request
[Bug] Add COMPARISON_NONE; fix the error message related to it #8246
Closed
@damyanp

Damyan Pepper (damyanp) commented Mar 10, 2026

Copy link
Copy Markdown
Member

Finn Plummer (@inbelic) - can you have a look at this PR please?

tex3d
Tex Riddell (tex3d) approved these changes Mar 10, 2026
View reviewed changes

@tex3d Tex Riddell (tex3d) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me.

inbelic
Finn Plummer (inbelic) approved these changes Mar 10, 2026
View reviewed changes

@inbelic Finn Plummer (inbelic) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly LGTM

Comment thread tools/clang/lib/Parse/HLSLRootSignature.cpp
default:
IFC(Error(ERR_RS_UNEXPECTED_TOKEN,
"Unexpected texture address mode value: '%s'.", Token.GetStr()));
"Unexpected comparison function value: '%s'.", Token.GetStr()));

@inbelic Finn Plummer (inbelic) Mar 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like there is the same typo below in ParseBorderColor, we can correct that while here

@ModulePillow ModulePillow Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Noticed, I have fixed it in the new commit

Comment thread tools/clang/lib/Parse/HLSLRootSignature.h

// Comparison function
comparisonFunc,
COMPARISON_NONE,

@inbelic Finn Plummer (inbelic) Mar 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We might also want to add this enum to the COMPARISON_FUNC_TABLE in ShaderOpTest.cpp

@ModulePillow ModulePillow Mar 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added.

@github-actions

github-actions Bot commented Mar 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ With the latest revision this PR passed the C/C++ code formatter.

@alsepkow

Alex Sepkowski (alsepkow) commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

azure-pipelines Bot commented Mar 12, 2026

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@inbelic
Finn Plummer (inbelic) merged commit 9485f9e into microsoft:main Mar 12, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap Mar 12, 2026
@inbelic

Finn Plummer (inbelic) commented Mar 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Awesome, thanks a lot ModulePillow! I created this issue to propagate the fix forward into clang.

If you'd be interested in contributing the fix forward, that'd be great. Just let me know one way or the other.

Thanks

@ModulePillow

ModulePillow commented Mar 13, 2026

Copy link
Copy Markdown
Contributor Author

Finn Plummer (@inbelic) That's fine, I will check the new issue and resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inbelic Finn Plummer (inbelic) inbelic approved these changes
+1 more reviewer
@tex3d Tex Riddell (tex3d) tex3d approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

HLSL Roadmap
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Add COMPARISON_NONE; fix the error message related to it

5 participants

@ModulePillow @damyanp @alsepkow @inbelic @tex3d