Vulkan-Samples icon indicating copy to clipboard operation
Vulkan-Samples copied to clipboard

Unifying approach for C- and C++-bindings, on DebugUtils classes.

Open asuessenbach opened this issue 1 year ago • 1 comments

Description

This is a proposal how we could unify the C- and the C++-bindings of Vulkan in the framework classes: Replace the two classes HPPFoo and Foo by one class Foo, that provides interfaces for both bindings. Pro:

  • reduced class count
  • no parallel class hierarchy handling
  • can be rolled in on a class-by-class basis

Contra:

  • more complex interface of those classes.

Build tested on Win10 with VS2022. Run tested on Win10 with NVidia GPU.

Note, that this change will not run with the non-hpp samples, as there's no dispatcher initialization done for them. It's just for demonstration purposes, how unifying C- and C++-bindings could look like.

General Checklist:

Please ensure the following points are checked:

  • [x] My code follows the coding style
  • [x] I have reviewed file licenses
  • [x] I have commented any added functions (in line with Doxygen)
  • [x] I have commented any code that could be hard to understand
  • [x] My changes do not add any new compiler warnings
  • [x] My changes do not add any new validation layer errors or warnings
  • [x] I have used existing framework/helper functions where possible
  • [x] My changes do not add any regressions
  • [x] I have tested every sample to ensure everything runs correctly
  • [x] This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:

  • [x] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
  • [ ] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

  • [ ] I have tested the sample on at least one compliant Vulkan implementation
  • [ ] If the sample is vendor-specific, I have tagged it appropriately
  • [ ] I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
  • [ ] Any dependent assets have been merged and published in downstream modules
  • [ ] For new samples, I have added a paragraph with a summary to the appropriate chapter in the samples readme
  • [ ] For new samples, I have added a tutorial README.md file to guide users through what they need to know to implement code using this feature. For example, see conditional_rendering
  • [ ] For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site

asuessenbach avatar Jan 09 '24 12:01 asuessenbach

Replace the two classes HPPFoo and Foo by one class Foo, that provides interfaces for both bindings.

I am all for this. I think this would help dramatically! Ill make sure i find time to review this properly

tomadamatkinson avatar Jan 15 '24 15:01 tomadamatkinson

This will be resolved - see PR910

marty-johnson59 avatar Mar 11 '24 15:03 marty-johnson59