Skip to content
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

Remove dEQP-ism from generated CTS helpers #2396

Closed
wants to merge 3 commits into from

Conversation

ShabbyX
Copy link
Contributor

@ShabbyX ShabbyX commented Jul 22, 2024

CTS no longer uses the de* types.

CTS no longer uses the de* types.
@ShabbyX
Copy link
Contributor Author

ShabbyX commented Jul 22, 2024

@oddhack not sure where vulkan_json_parser.hpp and vulkan_json_data.hpp are used outside CTS. If you know of any users, I'd appreciate it if you could verify that this change is a no-op for them.

@oddhack
Copy link
Contributor

oddhack commented Jul 24, 2024

@oddhack not sure where vulkan_json_parser.hpp and vulkan_json_data.hpp are used outside CTS. If you know of any users, I'd appreciate it if you could verify that this change is a no-op for them.

I believe these are Vulkan SC-specific. @dgkoch can you confirm and advise how to proceed? Putting it in the upstream like this seems OK if it's not an urgent fix.

@ShabbyX
Copy link
Contributor Author

ShabbyX commented Jul 24, 2024

@oddhack not sure where vulkan_json_parser.hpp and vulkan_json_data.hpp are used outside CTS. If you know of any users, I'd appreciate it if you could verify that this change is a no-op for them.

I believe these are Vulkan SC-specific. @dgkoch can you confirm and advise how to proceed? Putting it in the upstream like this seems OK if it's not an urgent fix.

FWIW, VK-GL-CTS hasn't regenerated this for like 3 years, and I've noticed the generated files sometimes receive manual edits in that repo. This is not urgent, it's been a few months already since VK-GL-CTS has removed these types.

@dgkoch
Copy link
Contributor

dgkoch commented Jul 24, 2024

Mobica is actually about to update these in VK-GL-CTS, so they'll probably be interested in this fix.
I wonder if there is more of the "isCTS" logic we can get rid of now that cts is no longer using the "de" types.

I might cherry-pick this straight into the vulkansc repo, where we added another change recently.

scripts/json_generator.py Outdated Show resolved Hide resolved
scripts/json_generator.py Show resolved Hide resolved
@dgkoch
Copy link
Contributor

dgkoch commented Jul 24, 2024

I've taken this to internal merge request: https://gitlab.khronos.org/vulkansc/vulkansc/-/merge_requests/382
Since we have some other recent changes there that haven't been merged out yet.

@oddhack
Copy link
Contributor

oddhack commented Aug 7, 2024

I've taken this to internal merge request: https://gitlab.khronos.org/vulkansc/vulkansc/-/merge_requests/382 Since we have some other recent changes there that haven't been merged out yet.

Taking this off the Vulkan maintenance agenda - please close when appropriate.

@dgkoch
Copy link
Contributor

dgkoch commented Oct 18, 2024

Something based on this was merged into VKSC branch and released in the VKSC 1.0.16 release, and was merged back up to Vulkan and published in the 1.3.299 release.

@dgkoch dgkoch closed this Oct 18, 2024
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.

3 participants