Skip to content

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

Merged
inbelic merged 3 commits intomicrosoft:mainfrom
ModulePillow:pillow-patch
Mar 12, 2026
Merged

[Bug] Add COMPARISON_NONE; fix the error message related to it#8245
inbelic merged 3 commits intomicrosoft:mainfrom
ModulePillow:pillow-patch

Conversation

@ModulePillow
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
@damyanp damyanp linked an issue Mar 10, 2026 that may be closed by this pull request
@damyanp
Copy link
Copy Markdown
Member

damyanp commented Mar 10, 2026

@inbelic - can you have a look at this PR please?

Copy link
Copy Markdown
Contributor

@tex3d tex3d left a comment

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.

Copy link
Copy Markdown
Collaborator

@inbelic inbelic left a comment

Choose a reason for hiding this comment

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

Mostly LGTM

default:
IFC(Error(ERR_RS_UNEXPECTED_TOKEN,
"Unexpected texture address mode value: '%s'.", Token.GetStr()));
"Unexpected comparison function value: '%s'.", Token.GetStr()));
Copy link
Copy Markdown
Collaborator

@inbelic inbelic Mar 10, 2026

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

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


// Comparison function
comparisonFunc,
COMPARISON_NONE,
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

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
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 11, 2026

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

@ModulePillow ModulePillow requested a review from inbelic March 12, 2026 10:04
@alsepkow
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@inbelic 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
Copy link
Copy Markdown
Collaborator

inbelic commented Mar 12, 2026

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
Copy link
Copy Markdown
Contributor Author

@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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

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

5 participants