Skip to content

Commit ae27b7d

Browse files
author
devsh
committed
enforce some constraints, stop the double instantiation of Triangles and AABBs with const BufferType and BufferType 
1 parent bc9b5f1 commit ae27b7d

File tree

7 files changed

+104
-109
lines changed

7 files changed

+104
-109
lines changed

‎include/nbl/asset/IAccelerationStructure.h

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,32 @@ class IBottomLevelAccelerationStructure : public IAccelerationStructure
8888
NO_DUPLICATE_ANY_HIT_INVOCATION_BIT = 0x1u<<1u,
8989
};
9090

91+
enum class GeometryType : uint8_t
92+
{
93+
Triangles = 0,
94+
AABBs = 1,
95+
// Later: LSS and friends
96+
Count = 2
97+
};
98+
9199
// Note that in Vulkan strides are 64-bit value but restricted to be 32-bit in range
92-
template<typename BufferType> requires std::is_base_of_v<IBuffer,BufferType>
100+
template<typename BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<IBuffer,BufferType>)
93101
struct Triangles
94102
{
95103
public:
96-
using buffer_t = std::remove_const_t<BufferType>;
97-
constexpr static inline bool Host = std::is_same_v<buffer_t,ICPUBuffer>;
104+
using buffer_t = BufferType;
105+
constexpr static inline GeometryType Type = GeometryType::Triangles;
106+
107+
private:
108+
constexpr static inline bool HostTransform = std::is_same_v<buffer_t,ICPUBuffer>;
109+
110+
public:
98111
// we make our life easier by not taking pointers to single matrix values
99-
using transform_t = std::conditional_t<Host,hlsl::float32_t3x4,asset::SBufferBinding<const buffer_t>>;
112+
using transform_t = std::conditional_t<HostTransform,hlsl::float32_t3x4,asset::SBufferBinding<const buffer_t>>;
100113

101114
inline bool hasTransform() const
102115
{
103-
if constexpr (Host)
116+
if constexpr (HostTransform)
104117
return !core::isnan(transform[0][0]);
105118
else
106119
return bool(transform.buffer);
@@ -122,17 +135,18 @@ class IBottomLevelAccelerationStructure : public IAccelerationStructure
122135
private:
123136
constexpr static transform_t __transform_initializer()
124137
{
125-
if constexpr (Host)
138+
if constexpr (HostTransform)
126139
return hlsl::float32_t3x4(std::numeric_limits<float>::quiet_NaN());
127140
return {};
128141
}
129142
};
130143

131144
//
132-
template<typename BufferType> requires std::is_base_of_v<IBuffer,BufferType>
145+
template<typename BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<IBuffer,BufferType>)
133146
struct AABBs
134147
{
135-
using buffer_t = std::remove_const_t<BufferType>;
148+
using buffer_t = BufferType;
149+
constexpr static inline GeometryType Type = GeometryType::Triangles;
136150

137151
// for `MOTION_BIT` you don't get a second buffer for AABBs at different times because linear interpolation of AABBs doesn't work
138152
asset::SBufferBinding<const BufferType> data = {};

‎include/nbl/video/IGPUAccelerationStructure.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class IGPUAccelerationStructure : public IBackendObject
4545
#endif
4646

4747
//! builds
48-
template<class BufferType>
48+
template<class BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<asset::IBuffer,BufferType>)
4949
struct BuildInfo
5050
{
5151
public:
@@ -112,7 +112,7 @@ class IGPUAccelerationStructure : public IBackendObject
112112
IGPUAccelerationStructure* dst = nullptr;
113113
COPY_MODE mode = COPY_MODE::CLONE;
114114
};
115-
template<typename BufferType>
115+
template<typename BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<asset::IBuffer,BufferType>)
116116
struct CopyToMemoryInfo
117117
{
118118
const IGPUAccelerationStructure* src = nullptr;
@@ -121,7 +121,7 @@ class IGPUAccelerationStructure : public IBackendObject
121121
};
122122
using DeviceCopyToMemoryInfo = CopyToMemoryInfo<IGPUBuffer>;
123123
using HostCopyToMemoryInfo = CopyToMemoryInfo<asset::ICPUBuffer>;
124-
template<typename BufferType>
124+
template<typename BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<asset::IBuffer,BufferType>)
125125
struct CopyFromMemoryInfo
126126
{
127127
asset::SBufferBinding<const BufferType> src = nullptr;
@@ -181,7 +181,7 @@ class IGPUBottomLevelAccelerationStructure : public asset::IBottomLevelAccelerat
181181
using DirectBuildRangeRangeInfos = const BuildRangeInfo* const*;
182182
using MaxInputCounts = const uint32_t* const;
183183

184-
template<class BufferType>
184+
template<class BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<asset::IBuffer,BufferType>)
185185
struct BuildInfo final : IGPUAccelerationStructure::BuildInfo<BufferType>
186186
{
187187
private:
@@ -203,7 +203,7 @@ class IGPUBottomLevelAccelerationStructure : public asset::IBottomLevelAccelerat
203203
NBL_API2 uint32_t valid(const T* const buildRangeInfosOrMaxPrimitiveCounts) const;
204204

205205
// really expensive to call, `valid` only calls it when `_NBL_DEBUG` is defined
206-
inline bool validGeometry(size_t& totalPrims, const AABBs<const BufferType>& geometry, const BuildRangeInfo& buildRangeInfo) const
206+
inline bool validGeometry(size_t& totalPrims, const AABBs<BufferType>& geometry, const BuildRangeInfo& buildRangeInfo) const
207207
{
208208
constexpr size_t AABBalignment = 8ull;
209209
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkAccelerationStructureBuildRangeInfoKHR-primitiveOffset-03659
@@ -222,7 +222,7 @@ class IGPUBottomLevelAccelerationStructure : public asset::IBottomLevelAccelerat
222222
totalPrims += buildRangeInfo.primitiveCount;
223223
return true;
224224
}
225-
inline bool validGeometry(size_t& totalPrims, const Triangles<const BufferType>& geometry, const BuildRangeInfo& buildRangeInfo) const
225+
inline bool validGeometry(size_t& totalPrims, const Triangles<BufferType>& geometry, const BuildRangeInfo& buildRangeInfo) const
226226
{
227227
//
228228
if (!dstAS->validVertexFormat(geometry.vertexFormat))
@@ -306,7 +306,7 @@ class IGPUBottomLevelAccelerationStructure : public asset::IBottomLevelAccelerat
306306
*(oit++) = core::smart_refctd_ptr<const IReferenceCounted>(srcAS);
307307
*(oit++) = core::smart_refctd_ptr<const IReferenceCounted>(dstAS);
308308

309-
if (buildFlags.hasFlags(IGPUBottomLevelAccelerationStructure::BUILD_FLAGS::GEOMETRY_TYPE_IS_AABB_BIT))
309+
if (buildFlags.hasFlags(asset::IBottomLevelAccelerationStructure::BUILD_FLAGS::GEOMETRY_TYPE_IS_AABB_BIT))
310310
{
311311
for (auto i=0u; i<geometryCount; i++)
312312
*(oit++) = aabbs[i].data.buffer;
@@ -337,8 +337,8 @@ class IGPUBottomLevelAccelerationStructure : public asset::IBottomLevelAccelerat
337337
// please interpret based on `buildFlags.hasFlags(GEOMETRY_TYPE_IS_AABB_BIT)`
338338
union
339339
{
340-
const Triangles<const BufferType>* triangles = nullptr;
341-
const AABBs<const BufferType>* aabbs;
340+
const Triangles<BufferType>* triangles = nullptr;
341+
const AABBs<BufferType>* aabbs;
342342
};
343343
};
344344
using DeviceBuildInfo = BuildInfo<IGPUBuffer>;
@@ -393,7 +393,7 @@ class IGPUTopLevelAccelerationStructure : public asset::ITopLevelAccelerationStr
393393
using DirectBuildRangeRangeInfos = const BuildRangeInfo*;
394394
using MaxInputCounts = const uint32_t;
395395

396-
template<typename BufferType>
396+
template<typename BufferType> requires (!std::is_const_v<BufferType> && std::is_base_of_v<asset::IBuffer,BufferType>)
397397
struct BuildInfo final : IGPUAccelerationStructure::BuildInfo<BufferType>
398398
{
399399
private:

‎include/nbl/video/ILogicalDevice.h

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -412,19 +412,20 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
412412
};
413413
// fun fact: you can use garbage/invalid pointers/offset for the Device/Host addresses of the per-geometry data, just make sure what was supposed to be null is null
414414
template<class Geometry> requires nbl::is_any_of_v<Geometry,
415-
IGPUBottomLevelAccelerationStructure::Triangles<const IGPUBuffer>,
416-
IGPUBottomLevelAccelerationStructure::Triangles<const asset::ICPUBuffer>,
417-
IGPUBottomLevelAccelerationStructure::AABBs<const IGPUBuffer>,
418-
IGPUBottomLevelAccelerationStructure::AABBs<const asset::ICPUBuffer>
415+
asset::IBottomLevelAccelerationStructure::Triangles<IGPUBuffer>,
416+
asset::IBottomLevelAccelerationStructure::Triangles<asset::ICPUBuffer>,
417+
asset::IBottomLevelAccelerationStructure::AABBs<IGPUBuffer>,
418+
asset::IBottomLevelAccelerationStructure::AABBs<asset::ICPUBuffer>
419419
>
420420
inline AccelerationStructureBuildSizes getAccelerationStructureBuildSizes(
421-
const core::bitflag<IGPUBottomLevelAccelerationStructure::BUILD_FLAGS> flags,
421+
const bool hostBuild,
422+
const core::bitflag<asset::IBottomLevelAccelerationStructure::BUILD_FLAGS> flags,
422423
const bool motionBlur,
423424
const std::span<const Geometry> geometries,
424425
const uint32_t* const pMaxPrimitiveCounts
425426
) const
426427
{
427-
if (invalidFeaturesForASBuild<typename Geometry::buffer_t>(motionBlur))
428+
if (invalidFeaturesForASBuild(hostBuild,motionBlur))
428429
{
429430
NBL_LOG_ERROR("Required features are not enabled");
430431
return {};
@@ -455,13 +456,29 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
455456
uint32_t primsFree = limits.maxAccelerationStructurePrimitiveCount;
456457
for (auto i=0u; i<geometries.size(); i++)
457458
{
458-
if constexpr (std::is_same_v<IGPUBottomLevelAccelerationStructure::Triangles<const Geometry::buffer_t>,Geometry>)
459+
const auto& geom = geometries[i];
460+
if constexpr (Geometry::Type==asset::IBottomLevelAccelerationStructure::GeometryType::Triangles)
459461
{
460-
// TODO: do we check `maxVertex`, `vertexStride` and `indexType` for validity?
462+
if (flags.hasFlags(asset::IBottomLevelAccelerationStructure::BUILD_FLAGS::GEOMETRY_TYPE_IS_AABB_BIT))
463+
{
464+
NBL_LOG_ERROR("Primitive type is Triangles but build flag says BLAS build is AABBs");
465+
return {};
466+
}
467+
if (!getPhysicalDevice()->getBufferFormatUsages()[geom.vertexFormat].accelerationStructureVertex)
468+
{
469+
NBL_LOG_ERROR("Vertex Format %d not supported as Acceleration Structure Vertex Position Input on this Device",geom.vertexFormat);
470+
return {};
471+
}
472+
// TODO: do we check `maxVertex`, `vertexStride` and `indexType` for validity
461473
}
462-
if constexpr (std::is_same_v<IGPUBottomLevelAccelerationStructure::AABBs<const Geometry::buffer_t>,Geometry>)
474+
if constexpr (Geometry::Type==asset::IBottomLevelAccelerationStructure::GeometryType::AABBs)
463475
{
464-
// TODO: check stride and geometry flags for validity?
476+
if (!flags.hasFlags(asset::IBottomLevelAccelerationStructure::BUILD_FLAGS::GEOMETRY_TYPE_IS_AABB_BIT))
477+
{
478+
NBL_LOG_ERROR("Primitive type is AABB but build flag says BLAS build is not AABBs");
479+
return {};
480+
}
481+
// TODO: check stride and geometry flags for validity
465482
}
466483
if (pMaxPrimitiveCounts[i] > primsFree)
467484
{
@@ -471,16 +488,16 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
471488
primsFree -= pMaxPrimitiveCounts[i];
472489
}
473490

474-
return getAccelerationStructureBuildSizes_impl(flags,motionBlur,geometries,pMaxPrimitiveCounts);
491+
return getAccelerationStructureBuildSizes_impl(hostBuild,flags,motionBlur,geometries,pMaxPrimitiveCounts);
475492
}
476493
inline AccelerationStructureBuildSizes getAccelerationStructureBuildSizes(
477494
const bool hostBuild,
478-
const core::bitflag<IGPUTopLevelAccelerationStructure::BUILD_FLAGS> flags,
495+
const core::bitflag<asset::ITopLevelAccelerationStructure::BUILD_FLAGS> flags,
479496
const bool motionBlur,
480497
const uint32_t maxInstanceCount
481498
) const
482499
{
483-
if (invalidFeaturesForASBuild<IGPUBuffer>(motionBlur))
500+
if (invalidFeaturesForASBuild(hostBuild,motionBlur))
484501
{
485502
NBL_LOG_ERROR("Required features are not enabled");
486503
return {};
@@ -504,7 +521,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
504521
}
505522
// little utility
506523
template<typename BufferType=IGPUBuffer>
507-
inline AccelerationStructureBuildSizes getAccelerationStructureBuildSizes(const core::bitflag<IGPUTopLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur, const uint32_t maxInstanceCount) const
524+
inline AccelerationStructureBuildSizes getAccelerationStructureBuildSizes(const core::bitflag<asset::ITopLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur, const uint32_t maxInstanceCount) const
508525
{
509526
return getAccelerationStructureBuildSizes(std::is_same_v<std::remove_cv_t<BufferType>,asset::ICPUBuffer>,flags,motionBlur,maxInstanceCount);
510527
}
@@ -1070,20 +1087,20 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
10701087
virtual core::smart_refctd_ptr<IGPUTopLevelAccelerationStructure> createTopLevelAccelerationStructure_impl(IGPUTopLevelAccelerationStructure::SCreationParams&& params) = 0;
10711088

10721089
virtual AccelerationStructureBuildSizes getAccelerationStructureBuildSizes_impl(
1073-
const core::bitflag<IGPUBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1074-
const std::span<const IGPUBottomLevelAccelerationStructure::AABBs<const IGPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
1090+
const bool hostBuild, const core::bitflag<asset::IBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1091+
const std::span<const asset::IBottomLevelAccelerationStructure::AABBs<IGPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
10751092
) const = 0;
10761093
virtual AccelerationStructureBuildSizes getAccelerationStructureBuildSizes_impl(
1077-
const core::bitflag<IGPUBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1078-
const std::span<const IGPUBottomLevelAccelerationStructure::AABBs<const asset::ICPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
1094+
const bool hostBuild, const core::bitflag<asset::IBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1095+
const std::span<const asset::IBottomLevelAccelerationStructure::AABBs<asset::ICPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
10791096
) const = 0;
10801097
virtual AccelerationStructureBuildSizes getAccelerationStructureBuildSizes_impl(
1081-
const core::bitflag<IGPUBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1082-
const std::span<const IGPUBottomLevelAccelerationStructure::Triangles<const IGPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
1098+
const bool hostBuild, const core::bitflag<asset::IBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1099+
const std::span<const asset::IBottomLevelAccelerationStructure::Triangles<IGPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
10831100
) const = 0;
10841101
virtual AccelerationStructureBuildSizes getAccelerationStructureBuildSizes_impl(
1085-
const core::bitflag<IGPUBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1086-
const std::span<const IGPUBottomLevelAccelerationStructure::Triangles<const asset::ICPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
1102+
const bool hostBuild, const core::bitflag<asset::IBottomLevelAccelerationStructure::BUILD_FLAGS> flags, const bool motionBlur,
1103+
const std::span<const asset::IBottomLevelAccelerationStructure::Triangles<asset::ICPUBuffer>> geometries, const uint32_t* const pMaxPrimitiveCounts
10871104
) const = 0;
10881105
virtual AccelerationStructureBuildSizes getAccelerationStructureBuildSizes_impl(
10891106
const bool hostBuild, const core::bitflag<IGPUTopLevelAccelerationStructure::BUILD_FLAGS> flags,
@@ -1333,8 +1350,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
13331350
}
13341351
return false;
13351352
}
1336-
template<class BufferType>
1337-
bool invalidFeaturesForASBuild(const bool motionBlur) const
1353+
bool invalidFeaturesForASBuild(const bool hostBuild, const bool motionBlur) const
13381354
{
13391355
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkGetAccelerationStructureBuildSizesKHR-accelerationStructure-08933
13401356
if (!m_enabledFeatures.accelerationStructure)
@@ -1343,7 +1359,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
13431359
return true;
13441360
}
13451361
// not sure of VUID
1346-
if (std::is_same_v<BufferType, asset::ICPUBuffer> && !m_enabledFeatures.accelerationStructureHostCommands)
1362+
if (hostBuild && !m_enabledFeatures.accelerationStructureHostCommands)
13471363
{
13481364
NBL_LOG_ERROR("Feature `acceleration structure` host commands is not enabled");
13491365
return true;

0 commit comments

Comments
 (0)