vulkano-rs / vulkano

Safe and rich Rust wrapper around the Vulkan API
Apache License 2.0
4.45k stars 435 forks source link

Handle more depends values in vk.xml #2511

Closed 0xcaff closed 4 months ago

0xcaff commented 4 months ago

Overview

These changes are necessary to support the vk.xml from the latest version of ash. To exercise these codepaths, cherry pick this commit https://github.com/0xcaff/vulkano/commit/f43f1188

Merge Order

These changes add behavior which is used when the new vk.xml is used. This can be safely merged independently of https://github.com/vulkano-rs/vulkano/pull/2510 (either before or after).

I plan to make a PR on top of both these changes and https://github.com/vulkano-rs/vulkano/pull/2510 pulling in the new vk.xml.

Details

  1. It would be nice to have tests for parse_depends but given this is code in a buildscript, not sure how to set that up. Open to ideas here. Would like to do this in a separate PR as the priors lack test coverage.

  2. This conjuctive normal form transformation is kinda complex. It is clever but not the most straightforward thing. I think it would be simpler to store the original boolean expression instead and evaluate against that. Left it as is as I don't completely understand the usages of requires_all_of enough to safely change them.

Rua commented 4 months ago

Great work! Thank you very much!