Skip to content

Commit d224016

Browse files
deprilula28deprilula28
committed
Add comments from feature API PR
1 parent 29b7ecb commit d224016

File tree

3 files changed

+105
-23
lines changed

3 files changed

+105
-23
lines changed

‎include/nbl/video/SPhysicalDeviceFeatures.h

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,75 @@ enum E_SWAPCHAIN_MODE : uint32_t
1212
/* TODO: KHR_swapchain if SURFACE or DISPLAY flag present & KHR_display_swapchain if DISPLAY flag present */
1313
};
1414

15+
//! [TODOS]:
16+
//!
17+
//! ## LogicalDevice creation enabled features shouldn't necessarily equal the ones it reports as enabled (superset)
18+
//!
19+
//! Basically what I'd imagine the usage of the API to be like.
20+
//!
21+
//! **RARE: Creating a physical device with all advertised features/extensions:**
22+
//! ```cpp
23+
//! auto features = physicalDevice->getFeatures();
24+
//!
25+
//! ILogicalDevice::SCreationParams params = {};
26+
//! params.queueParamsCount = ; // set queue stuff
27+
//! params.queueParams = ; // set queue stuff
28+
//! params.enabledFeatures = features;
29+
//! auto device = physicalDevice->createLogicalDevice(params);
30+
//! ```
31+
//!
32+
//! **FREQUENT: Choosing a physical device with the features**
33+
//! ```cpp
34+
//! IPhysicalDevice::SRequiredProperties props = {}; // default initializes to apiVersion=1.1, deviceType = ET_UNKNOWN, pipelineCacheUUID = '\0', device UUID=`\0`, driverUUID=`\0`, deviceLUID=`\0`, deviceNodeMask= ~0u, driverID=UNKNOWN
35+
//! // example of particular config
36+
//! props.apiVersion = 1.2;
37+
//! props.deviceTypeMask = ~IPhysicalDevice::ET_CPU; // would be good to turn the enum into a mask
38+
//! props.driverIDMask = ~(EDI_AMD_PROPRIETARY|EDI_INTEL_PROPRIETARY_WINDOWS); // would be goot to turn the enum into a mask
39+
//! props.conformanceVersion = 1.2;
40+
//!
41+
//! SDeviceFeatures requiredFeatures = {};
42+
//! requiredFeatures.rayQuery = true;
43+
//!
44+
//! SDeviceLimits minimumLimits = {}; // would default initialize to worst possible values (small values for maximum sizes, large values for alignments, etc.)
45+
//!
46+
//! // TODO: later add some stuff for requiring queue families, formats and minimum memory heap sizes
47+
//!
48+
//! auto physicalDeviceCandidates = api->getCompatiblePhysicalDevices(props,requiredFeatures,minimumLimits,numSwapchains,supportedSwapchains,/*optional: would enforce tighter checks to actually accept compatibility, like formats, present modes and surface caps*/swapchainSupportDecider);
49+
//! if (physicalDeviceCandidates.empty())
50+
//! {
51+
//! logError();
52+
//! exit();
53+
//! }
54+
//!
55+
//! // TODO: later iterate through candidate devices (fulfilling all the required criteria) to find the "best" one
56+
//! // std::sort(physicalDeviceCandidates.begin(),physicalDeviceCandidates.end(),SDefaultPhysicalDeviceOrder());
57+
//! auto physicalDevice = physicalDeviceCandidates.begin();
58+
//! assert(requiredFeatures < physicalDevice->getFeatures());
59+
//! assert(minimumLimits < physicalDevice->getLimits());
60+
//!
61+
//! ILogicalDevice::SCreationParams params = {};
62+
//! params.queueParamsCount = ; // set queue stuff
63+
//! params.queueParams = ; // set queue stuff
64+
//! params.enabledFeatures = requiredFeatures;
65+
//! auto device = physicalDevice->createLogicalDevice(params);
66+
//! // this would be wrong, because during device creation we would enable additional features either due to:
67+
//! // - dependencies (like buffer address for raytracing)
68+
//! // - backend force-enabling them (like in OpenGL, where extensions are just enabled, you have no choice)
69+
//! // assert(requiredFeatures != device->getEnabledFeatures());
70+
//! assert(requiredFeatures < device->getEnabledFeatures());
71+
//! ```
72+
//!
73+
//! ### `SDeviceFeatures` and `SDeviceLimits` should have a `operator<`
74+
//!
75+
//! Basically to let us establish if features or limits are a superset of the requested.
76+
//!
77+
//! If you need a `! = ` operator then define it as
78+
//! ```cpp
79+
//! inline bool operator!=(const& other) const
80+
//! {
81+
//! return *this < other || other < *this;
82+
//! }
83+
//! ```
1584
struct SPhysicalDeviceFeatures
1685
{
1786
/* Vulkan 1.0 Core */
@@ -643,7 +712,25 @@ struct SPhysicalDeviceFeatures
643712
// [TODO LATER] Won't expose for now, API changes necessary
644713
/* VK_AMD_texture_gather_bias_lod */
645714

646-
// [TODO LATER] when released in the SDK: https://github.com/Devsh-Graphics-Programming/Nabla/pull/357#discussion_r916899420
715+
// [TODO LATER] when released in the SDK:
716+
// -Support for `GLSL_EXT_ray_cull_mask`, lets call it `rayCullMask`
717+
// - new pipeline stage and access masks but only in `KHR_synchronization2` which we don't use
718+
// - two new acceleration structure query parameters
719+
// - `rayTracingPipelineTraceRaysIndirect2` feature, same as `rayTracingPipelineTraceRaysIndirect` but with indirect SBTand dispatch dimensions
720+
//
721+
// Lets have
722+
// ```cpp
723+
// bool accelerationStructureSizeAndBLASPointersQuery = false;
724+
//
725+
// // Do not expose, we don't use KHR_synchronization2 yet
726+
// //bool accelerationStructureCopyStageAndSBTAccessType;
727+
//
728+
// bool rayCullMask = false;
729+
//
730+
// bool rayTracingPipelineTraceRaysIndirectDimensionsAndSBT = false;
731+
// ```
732+
//
733+
// Lets enable `rayTracingMaintenance1`and `rayTracingPipelineTraceRaysIndirect2` whenever required by the above.
647734
/* VK_KHR_ray_tracing_maintenance1 *//* added in vk 1.3.213, the SDK isn't released yet at this moment :D */
648735

649736
// [TODO LATER] requires extra API work to use
@@ -714,6 +801,9 @@ struct SPhysicalDeviceFeatures
714801
// [TODO] Always enable, expose as limit
715802
/* VK_KHR_spirv_1_4 */
716803

804+
// [TODO] handle with a single num
805+
/* VK_KHR_display_swapchain */
806+
717807
// [TODO LATER] (When it has documentation): Always enable, expose as limit
718808
/* VK_AMD_gpu_shader_half_float_fetch */
719809

@@ -951,7 +1041,6 @@ struct SPhysicalDeviceFeatures
9511041
// [TODO] Triage leftover extensions below
9521042

9531043
/* VK_NV_present_barrier */
954-
/* VK_KHR_display_swapchain */
9551044
/* VK_EXT_queue_family_foreign */
9561045
/* VK_EXT_separate_stencil_usage */
9571046
/* VK_KHR_create_renderpass2 */

‎include/nbl/video/SPhysicalDeviceLimits.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -380,26 +380,6 @@ struct SPhysicalDeviceLimits
380380
//uint32_t maxDescriptorSetInlineUniformBlocks;
381381
//uint32_t maxDescriptorSetUpdateAfterBindInlineUniformBlocks;
382382

383-
// [DO NOT EXPOSE] We will never expose this vendor specific meta-data (no new feature) to the user, but might use the extension to provide some cross platform meta-info in the Nabla section
384-
/* ShaderCoreProperties2AMD *//* provided by VK_AMD_shader_core_properties2 */
385-
//VkShaderCorePropertiesFlagsAMD shaderCoreFeatures;
386-
//uint32_t activeComputeUnitCount;
387-
/* ShaderCorePropertiesAMD *//* provided by VK_AMD_shader_core_properties */
388-
//uint32_t shaderEngineCount;
389-
//uint32_t shaderArraysPerEngineCount;
390-
//uint32_t computeUnitsPerShaderArray;
391-
//uint32_t simdPerComputeUnit;
392-
//uint32_t wavefrontsPerSimd;
393-
//uint32_t wavefrontSize;
394-
//uint32_t sgprsPerSimd;
395-
//uint32_t minSgprAllocation;
396-
//uint32_t maxSgprAllocation;
397-
//uint32_t sgprAllocationGranularity;
398-
//uint32_t vgprsPerSimd;
399-
//uint32_t minVgprAllocation;
400-
//uint32_t maxVgprAllocation;
401-
//uint32_t vgprAllocationGranularity;
402-
403383
// [DO NOT EXPOSE] right now, no idea if we'll ever expose and implement those but they'd all be false for OpenGL
404384
/* BlendOperationAdvancedPropertiesEXT *//* provided by VK_EXT_blend_operation_advanced */
405385
//uint32_t advancedBlendMaxColorAttachments;

‎src/nbl/video/IOpenGL_PhysicalDeviceBase.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ class IOpenGL_PhysicalDeviceBase : public IOpenGLPhysicalDeviceBase
766766
m_properties.limits.imageFootprint = m_glfeatures.isFeatureAvailable(COpenGLFeatureMap::NBL_NV_shader_texture_footprint);
767767

768768
// [TODO]
769-
// https://github.com/Devsh-Graphics-Programming/Nabla/pull/357#discussion_r917052568
769+
// not sure if any OpenGL extension provides that, but if it exists it would be one of those that allows 16bit int/float to be used for UBO/SSBO
770770
m_features.storagePushConstant8 = false;
771771
m_features.storagePushConstant16 = false;
772772
m_features.storageInputOutput16 = false;
@@ -1301,6 +1301,19 @@ class IOpenGL_PhysicalDeviceBase : public IOpenGLPhysicalDeviceBase
13011301
GetIntegerv(GL_MAX_WINDOW_RECTANGLES_EXT, reinterpret_cast<GLint*>(&m_properties.limits.maxDiscardRectangles));
13021302
}
13031303

1304+
/*
1305+
sampleLocationSampleCounts!=0 and variableSampleLocations=true only if
1306+
https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_sample_locations.txt
1307+
or
1308+
https://www.khronos.org/registry/OpenGL/extensions/AMD/AMD_sample_positions.txt
1309+
available
1310+
1311+
otherwise sampleLocationSampleCounts=0 and variableSampleLocations=false
1312+
1313+
you can always get the remaining info from:
1314+
https://www.khronos.org/registry/OpenGL/extensions/NV/ARB_texture_multisample.txt
1315+
which I think is core in GL 4.4 and possibly even in GLES 3.1
1316+
*/
13041317
if (m_glfeatures.isFeatureAvailable(COpenGLFeatureMap::NBL_ARB_sample_locations))
13051318
{
13061319
m_properties.limits.variableSampleLocations = true;

0 commit comments

Comments
 (0)