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

Builtin Resources CMake Refactor #477

Merged
merged 52 commits into from
May 27, 2023
Merged

Builtin Resources CMake Refactor #477

merged 52 commits into from
May 27, 2023

Conversation

devshgraphicsprogramming
Copy link
Member

@devshgraphicsprogramming devshgraphicsprogramming commented Mar 16, 2023

Description

The PR introduces new way of handling built in resources thanks to nbl::system::CFileArchive. Built-in resources used to output

  • builtinResources.h with get_resource() declarations
  • builtinResourceData.cpp with get_resource() definitions
  • resources.txt helper file for python built-in resources generation scripts with resource URI paths

It were compiling to a static library, but Nabla namespaces were hardcoded and our CMake API could not let you abstract from Nabla. It has been changed.

Now multiple built-in resources can be created with custom namespace and they are handled by autogenerated CArchive class which inherits from nbl::system::CFileArchive. The class is defined in autogenerated CArchive.h next to old 3 autogenerated files.

The template of the autogenerated CArchive.h file is here, it's basically overriding nbl::system::CFileArchive's methods making it an archiver using nbl::system::CNullAllocator to obtain files.

There are additional modifications the PR covers like

  • fixing ambiguity in spirv-tools headers causing linker errors
  • inserting DXC DLL name given configuration to defines.h

Testing

Example 52. perform tests on built-in resources - it list a test text file as built-in resource, then it creates built-in resources library with custom namespace called yourNamespace::builtin and both Nabla and test built-in resources and then read at runtime

TODO list:

we were also talking about resource aliases and "turning some input into output and listing it as built-in resource" - however we have not discussed those in details so I have not covered them yet in the PR

AnastaZIuk and others added 13 commits February 23, 2023 20:29
…/builtin/template/CAPKResourcesArchieve.h.in files, rewrite src/nbl/builtin/CMakeLists.txt and update src/nbl/CMakeLists.txt - save work. TODO: bring back python generation data
…ompiler

Signed-off-by: Michael Pollind <mpollind@gmail.com>
Signed-off-by: Michael Pollind <mpollind@gmail.com>
…nbl/CMakeLists.txt with builtin python generation, update some source files, introduce Nabla internal NBL_BUILTIN_RESOURCES_DIRECTORY_PATH, add CArchive.h.in file
…bl/builtin/CMakeLists.txt. Update examples_tests submodule
…iltin-linux-window

bugfix: address build issues with embedded assets for clang and gnu compiler
…de of libraries we include in correspondingHeaderFile header
…re validation and fix bugs, set builtin resources library target CXX_STANDARD to 20
@devshgraphicsprogramming
Copy link
Member Author

@AnastaZIuk can you fill out the PR description with a summary of our discord talk from 3 weeks ago?

@devshgraphicsprogramming devshgraphicsprogramming mentioned this pull request Mar 16, 2023
@devshgraphicsprogramming
Copy link
Member Author

merge latest master into this and fill out the PR description after all the CI tasks.

@Erfan-Ahmadi
Copy link
Contributor

@AnastaZIuk When is this going to get merged? Do you need any help with this? resolving conflicts and whatnot

@AnastaZIuk
Copy link
Member

@AnastaZIuk When is this going to get merged? Do you need any help with this? resolving conflicts and whatnot

we are waiting for @devshgraphicsprogramming approval, it's been ready

…ave it compiled with one single builtin resources library. Rename BR_API to NBL_BR_API. Move creation of nblBuiltinResourceData, have Nabla created first to be capable of using it's properties. Target source builtin files with PRIVATE qualifier to Nabla target. Perform tests
@devshgraphicsprogramming
Copy link
Member Author

@AnastaZIuk after you merge #497 and answer #477 (comment) you can self-merge this PR

@AnastaZIuk AnastaZIuk merged commit 547586e into master May 27, 2023
@AnastaZIuk AnastaZIuk deleted the newBuiltinResources branch May 27, 2023 11:12
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.

6 participants