Skip to content

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 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
@github-actions
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
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