Skip to content

Fix include dxcapi.h #41

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix include dxcapi.h #41

wants to merge 1 commit into from

Conversation

TLescoatTFX
Copy link

Hello, I am currently doing a port of Dawn on vcpkg. During this I had to fix an include of dxcapi.h, the code in tint prefixes that filename... As a result, the compiler cannot find dxcapi.h when provided by vcpkg. Sorry for this 6-characters PR.

Almost unrelated, but still about header inclusion, it seems tint includes private headers from SPIRV-Tools (namely source/opt_ir_context.h), is that really necessary ? If so, is there a way to bring that back to upstream ?

Copy link

google-cla bot commented Jun 18, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@dj2
Copy link
Collaborator

dj2 commented Jun 18, 2025

The path is setup this way so it pulls the header from the third_party/dxc/include/ folder which is the copy of DXC we use with Tint.

The SPIR-V Tools header is required if you're building the SPIR-V Reader as we use the optimizer from SPIR-V Tools. As far as I know, there are no plans on making the optimizer API public. That header does exist in upstream SPIR-V tools already though.

@TLescoatTFX
Copy link
Author

The path is setup this way so it pulls the header from the third_party/dxc/include/ folder which is the copy of DXC we use with Tint.

Yes. I also think this should not be necessary, notably if the Dawn copy of DXC is the only copy of DXC in the include paths. Furthermore, dawn native already include this header without the prefix...

The SPIR-V Tools header is required if you're building the SPIR-V Reader as we use the optimizer from SPIR-V Tools. As far as I know, there are no plans on making the optimizer API public. That header does exist in upstream SPIR-V tools already though.

Thanks for the info. There seems to be a public API for the SPIRV-Tools optimizer, but I'm not sure if it covers your needs.

@dj2
Copy link
Collaborator

dj2 commented Jun 18, 2025

If you have both the Dawn copy and DXC, we want to choose the dawn version over the system header.

Yes, there is an interface to run the optimizer, but that isn't what we use/need.

@dj2
Copy link
Collaborator

dj2 commented Jun 18, 2025

@amaiorano would be in the best position to comment if changing this include to drop the prefix would work as we need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants