Skip to content

Commit

Permalink
Improve static common code (#21)
Browse files Browse the repository at this point in the history
Change to improve the static common code.

* Non-generated common code is no longer copied by the generator.
* GitHub Actions tests now validate that the generated common code matches the committed copy of that code to prevent committed template vs committed source desyncs.
* Non-generated common code uses consistent function name style.
* All common code uses consistent module-based include paths.
  • Loading branch information
solidpixel authored Dec 9, 2024
1 parent e181c48 commit 683c74d
Show file tree
Hide file tree
Showing 24 changed files with 439 additions and 643 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/build_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ jobs:
cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release ..
make -j4
- name: Check unexpected diffs
run: |
git diff --exit-code
build-ubuntu-x64-clang-new-project:
name: Ubuntu x64 generate new layer
runs-on: ubuntu-22.04
Expand Down
14 changes: 0 additions & 14 deletions generator/generate_vulkan_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -685,15 +685,6 @@ def main():
base_dir = os.path.dirname(__file__)
outdir = os.path.join(base_dir, '..', 'source_common', 'framework')

# Clean the output directory if needed
if os.path.exists(outdir):
if not os.path.isdir(outdir):
print(f'ERROR: Output location "{outdir}" is not a directory')
return 1

shutil.rmtree(outdir, ignore_errors=True)
os.makedirs(outdir)

# Parse the XML headers
tree = ET.parse('./khronos/vulkan/registry/vk.xml')
root = tree.getroot()
Expand Down Expand Up @@ -732,11 +723,6 @@ def main():
# Load hand written function bodies
manual_commands = load_handwritten_commands()

# Generate static resources
base_dir = os.path.dirname(__file__)
source_dir = os.path.join(base_dir, 'vk_common')
copy_resource(source_dir, outdir)

# Generate dynamic resources
outfile = os.path.join(outdir, 'instance_dispatch_table.hpp')
with open(outfile, 'w', encoding='utf-8', newline='\n') as handle:
Expand Down
5 changes: 4 additions & 1 deletion generator/vk_codegen/device_defs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
#include <mutex>
#include <thread>

// Include from per-layer code
#include "utils.hpp"
#include "device.hpp"
#include "device_functions.hpp"

// Include from common code
#include "framework/device_functions.hpp"

extern std::mutex g_vulkanLock;

Expand Down
1 change: 1 addition & 0 deletions generator/vk_codegen/device_dispatch_table.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <vulkan/vulkan.h>

#include "framework/device_functions.hpp"
#include "framework/utils.hpp"
#include "utils/misc.hpp"

#if __has_include ("layer_device_functions.hpp")
Expand Down
8 changes: 3 additions & 5 deletions generator/vk_codegen/function_vkCreateDevice.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// Release the lock to call into the driver
lock.unlock();

auto* chainInfo = get_chain_info(pCreateInfo, VK_LAYER_LINK_INFO);
auto* chainInfo = getChainInfo(pCreateInfo);
auto fpGetInstanceProcAddr = chainInfo->u.pLayerInfo->pfnNextGetInstanceProcAddr;
auto fpGetDeviceProcAddr = chainInfo->u.pLayerInfo->pfnNextGetDeviceProcAddr;
auto fpCreateDevice = reinterpret_cast<PFN_vkCreateDevice>(fpGetInstanceProcAddr(layer->instance, "vkCreateDevice"));
Expand All @@ -16,17 +16,15 @@

// Advance the link info for the next element on the chain
chainInfo->u.pLayerInfo = chainInfo->u.pLayerInfo->pNext;

auto res = fpCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice);
if (res != VK_SUCCESS)
{
return res;
}

auto device = std::make_unique<Device>(layer, physicalDevice, *pDevice, fpGetDeviceProcAddr);

// Hold the lock to access layer-wide global store
// Retake the lock to access layer-wide global store
lock.lock();
auto device = std::make_unique<Device>(layer, physicalDevice, *pDevice, fpGetDeviceProcAddr);
Device::store(*pDevice, std::move(device));

return VK_SUCCESS;
38 changes: 35 additions & 3 deletions generator/vk_codegen/function_vkCreateInstance.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
auto* chainInfo = get_chain_info(pCreateInfo, VK_LAYER_LINK_INFO);
auto* chainInfo = getChainInfo(pCreateInfo);
auto supportedExtensions = getInstanceExtensionList(pCreateInfo);

auto fpGetInstanceProcAddr = chainInfo->u.pLayerInfo->pfnNextGetInstanceProcAddr;
auto fpCreateInstance = reinterpret_cast<PFN_vkCreateInstance>(fpGetInstanceProcAddr(nullptr, "vkCreateInstance"));
Expand All @@ -7,14 +8,45 @@
return VK_ERROR_INITIALIZATION_FAILED;
}

// Create a copy we can write
VkInstanceCreateInfo newCreateInfo = *pCreateInfo;

// Query extension state
std::string targetExt("VK_EXT_debug_utils");
bool targetSupported = isIn(targetExt, supportedExtensions);
bool targetEnabled = isInExtensionList(
targetExt,
pCreateInfo->enabledExtensionCount,
pCreateInfo->ppEnabledExtensionNames);

if (!targetSupported)
{
LAYER_LOG("WARNING: Cannot enable additional extension: %s", targetExt.c_str());
}

// Enable the extension if we need to
std::vector<const char*> newExtList;
if (targetSupported && !targetEnabled)
{
LAYER_LOG("Enabling additional extension: %s", targetExt.c_str());
newExtList = cloneExtensionList(
pCreateInfo->enabledExtensionCount,
pCreateInfo->ppEnabledExtensionNames);

newExtList.push_back(targetExt.c_str());

newCreateInfo.enabledExtensionCount = newExtList.size();
newCreateInfo.ppEnabledExtensionNames = newExtList.data();
}

chainInfo->u.pLayerInfo = chainInfo->u.pLayerInfo->pNext;
auto res = fpCreateInstance(pCreateInfo, pAllocator, pInstance);
auto res = fpCreateInstance(&newCreateInfo, pAllocator, pInstance);
if (res != VK_SUCCESS)
{
return res;
}

// Hold the lock to access layer-wide global store
// Retake the lock to access layer-wide global store
auto instance = std::make_unique<Instance>(*pInstance, fpGetInstanceProcAddr);
{
std::lock_guard<std::mutex> lock { g_vulkanLock };
Expand Down
36 changes: 7 additions & 29 deletions generator/vk_codegen/instance_defs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,15 @@
#include <mutex>
#include <thread>

#include "utils.hpp"
#include "instance.hpp"
// Include from per-layer code
#include "device.hpp"
#include "instance_functions.hpp"

extern std::mutex g_vulkanLock;

static VkLayerInstanceCreateInfo* get_chain_info(
const VkInstanceCreateInfo* pCreateInfo,
VkLayerFunction func
) {
auto* info = static_cast<const VkLayerInstanceCreateInfo*>(pCreateInfo->pNext);
while (info && !(info->sType == VK_STRUCTURE_TYPE_LOADER_INSTANCE_CREATE_INFO && info->function == func))
{
info = static_cast<const VkLayerInstanceCreateInfo*>(info->pNext);
}

return const_cast<VkLayerInstanceCreateInfo*>(info);
}
#include "instance.hpp"

static VkLayerDeviceCreateInfo* get_chain_info(
const VkDeviceCreateInfo* pCreateInfo,
VkLayerFunction func
) {
auto* info = static_cast<const VkLayerDeviceCreateInfo*>(pCreateInfo->pNext);
while (info && !(info->sType == VK_STRUCTURE_TYPE_LOADER_DEVICE_CREATE_INFO && info->function == func))
{
info = static_cast<const VkLayerDeviceCreateInfo*>(info->pNext);
}
// Include from common code
#include "framework/instance_functions_manual.hpp"
#include "framework/instance_functions.hpp"
#include "framework/utils.hpp"

return const_cast<VkLayerDeviceCreateInfo*>(info);
}
extern std::mutex g_vulkanLock;

{FUNCTION_DEFS}
1 change: 1 addition & 0 deletions generator/vk_codegen/instance_dispatch_table.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <vulkan/vulkan.h>

#include "framework/instance_functions.hpp"
#include "framework/utils.hpp"
#include "utils/misc.hpp"

#if __has_include ("layer_instance_functions.hpp")
Expand Down
2 changes: 1 addition & 1 deletion generator/vk_codegen/root_CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ set(CMAKE_CXX_STANDARD 20)
project({PROJECT_NAME} VERSION 1.0.0)

# Common configuration
set(LGL_LOG_TAG, "{LAYER_NAME}")
set(LGL_LOG_TAG "{LAYER_NAME}")
include(../source_common/compiler_helper.cmake)

# Build steps
Expand Down
42 changes: 0 additions & 42 deletions generator/vk_common/CMakeLists.txt

This file was deleted.

101 changes: 0 additions & 101 deletions generator/vk_common/utils.hpp

This file was deleted.

2 changes: 1 addition & 1 deletion generator/vk_layer/source/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#include <thread>

#include "framework/utils.hpp"
#include "framework/entry_utils.hpp"
#include "framework/instance_functions_manual.hpp"

std::mutex g_vulkanLock;

Expand Down
2 changes: 1 addition & 1 deletion layer_example/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ set(CMAKE_CXX_STANDARD 20)
project(VkLayerExample VERSION 1.0.0)

# Common configuration
set(LGL_LOG_TAG, "VkLayerExample")
set(LGL_LOG_TAG "VkLayerExample")
include(../source_common/compiler_helper.cmake)

# Build steps
Expand Down
2 changes: 1 addition & 1 deletion layer_example/source/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
#include <thread>

#include "framework/utils.hpp"
#include "framework/entry_utils.hpp"
#include "framework/instance_functions_manual.hpp"

std::mutex g_vulkanLock;

Expand Down
7 changes: 5 additions & 2 deletions source_common/framework/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ set(LIB_BINARY lib_layer_framework)
add_library(
${LIB_BINARY} STATIC
device_functions.cpp
instance_functions.cpp)
instance_functions.cpp
instance_functions_manual.cpp)

target_include_directories(
${LIB_BINARY} PRIVATE
# Note, this includes from the layer-specific tree
# Include from the layer-specific tree
${PROJECT_SOURCE_DIR}/source
# Needed for CMake generated version.hpp
${PROJECT_BINARY_DIR}/source
../)

target_include_directories(
Expand Down
Loading

0 comments on commit 683c74d

Please sign in to comment.