Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[DirectX] Adding support for static samplers in yaml2obj/obj2yaml #139963

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.

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

Merged
merged 80 commits into from
May 29, 2025

Conversation

joaosaffran
Copy link
Contributor

  • Adds support for static samplers ins dxcontainer binary format.
  • Adds writing logic to mcdxbc
  • adds reading logic to Object
  • adds tests
    Closes: 126636

@joaosaffran joaosaffran force-pushed the users/joaosaffran/138318 branch from 24e77c1 to aabd424 Compare May 23, 2025 01:16
@joaosaffran joaosaffran changed the base branch from users/joaosaffran/138318 to users/joaosaffran/138315 May 29, 2025 17:41
@joaosaffran joaosaffran changed the base branch from users/joaosaffran/138315 to users/joaosaffran/138318 May 29, 2025 17:43
@joaosaffran joaosaffran changed the base branch from users/joaosaffran/138318 to users/joaosaffran/138315 May 29, 2025 17:43
Copy link

github-actions bot commented May 29, 2025

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

@joaosaffran joaosaffran requested a review from inbelic May 29, 2025 19:17
@joaosaffran joaosaffran requested a review from bogner May 29, 2025 19:17
Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

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

LGTM other than one comment about consistently using the ASSERT_* vs EXPECT_* macros in the tests.

ASSERT_EQ(Sampler.MaxAnisotropy, 20u);
ASSERT_EQ(Sampler.ComparisonFunc, 4u);
ASSERT_EQ(Sampler.BorderColor, 0u);
EXPECT_FLOAT_EQ(Sampler.MinLOD, 4.56);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these be ASSERT_FLOAT_EQ? IIUC ASSERT_* stops immediately if the condition isn't met and EXPECT_* allow us to diagnose multiple issues. I don't think it makes sense to use one for the floating point fields and the other for the rest of the fields.

Copy link
Contributor

@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.

LGTM. Did you want something to track and update going through all the previously defined parameters and marking them as optional yaml parameters if applicable?

@inbelic inbelic changed the title [DirectX] Adding support for static samples is yaml2obj/obj2yaml [DirectX] Adding support for static samplers in yaml2obj/obj2yaml May 29, 2025
@joaosaffran joaosaffran changed the base branch from users/joaosaffran/138315 to main May 29, 2025 20:35
@joaosaffran joaosaffran merged commit c7cbaef into llvm:main May 29, 2025
6 of 9 checks passed
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
…vm#139963)

- Adds support for static samplers ins dxcontainer binary format.
- Adds writing logic to mcdxbc
- adds reading logic to Object
- adds tests
Closes: [126636](llvm#126636)

---------

Co-authored-by: joaosaffran <joao.saffran@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DirectX] Add Static Sampler element support to obj2yaml/yaml2obj
4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.