Skip to content

Commit 64da18c

Browse files
devshgraphicsprogrammingdevshgraphicsprogramming
authored
Merge pull request #499 from Devsh-Graphics-Programming/fix_472
Validation Error Fixes and Threadsafety
2 parents 57a69bd + 4018e72 commit 64da18c

28 files changed

+869
-546
lines changed

‎examples_tests

‎include/nbl/asset/IImage.h

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
// Copyright (C) 2018-2020 - DevSH Graphics Programming Sp. z O.O.
1+
// Copyright (C) 2018-2023 - DevSH Graphics Programming Sp. z O.O.
22
// This file is part of the "Nabla Engine".
33
// For conditions of distribution and use, see copyright notice in nabla.h
4-
5-
#ifndef __NBL_ASSET_I_IMAGE_H_INCLUDED__
6-
#define __NBL_ASSET_I_IMAGE_H_INCLUDED__
4+
#ifndef _NBL_ASSET_I_IMAGE_H_INCLUDED_
5+
#define _NBL_ASSET_I_IMAGE_H_INCLUDED_
76

87
#include "nbl/core/util/bitflag.h"
98
#include "nbl/core/containers/refctd_dynamic_array.h"
@@ -136,18 +135,18 @@ class IImage : public IDescriptor
136135
};
137136
struct SSubresourceRange
138137
{
139-
E_ASPECT_FLAGS aspectMask = E_ASPECT_FLAGS::EAF_NONE;
140-
uint32_t baseMipLevel = 0u;
141-
uint32_t levelCount = 0u;
142-
uint32_t baseArrayLayer = 0u;
143-
uint32_t layerCount = 0u;
138+
core::bitflag<E_ASPECT_FLAGS> aspectMask = E_ASPECT_FLAGS::EAF_NONE;
139+
uint32_t baseMipLevel = 0u;
140+
uint32_t levelCount = 0u;
141+
uint32_t baseArrayLayer = 0u;
142+
uint32_t layerCount = 0u;
144143
};
145144
struct SSubresourceLayers
146145
{
147-
E_ASPECT_FLAGS aspectMask = E_ASPECT_FLAGS::EAF_NONE;
148-
uint32_t mipLevel = 0u;
149-
uint32_t baseArrayLayer = 0u;
150-
uint32_t layerCount = 0u;
146+
core::bitflag<E_ASPECT_FLAGS> aspectMask = E_ASPECT_FLAGS::EAF_NONE;
147+
uint32_t mipLevel = 0u;
148+
uint32_t baseArrayLayer = 0u;
149+
uint32_t layerCount = 0u;
151150

152151
auto operator<=>(const SSubresourceLayers&) const = default;
153152
};
@@ -214,7 +213,7 @@ class IImage : public IDescriptor
214213
inline bool isValid() const
215214
{
216215
// TODO: more complex check of compatible aspects when planar format support arrives
217-
if (srcSubresource.aspectMask^dstSubresource.aspectMask)
216+
if ((srcSubresource.aspectMask^dstSubresource.aspectMask).value)
218217
return false;
219218

220219
if (srcSubresource.layerCount!=dstSubresource.layerCount)
@@ -235,14 +234,14 @@ class IImage : public IDescriptor
235234
};
236235
struct SCreationParams
237236
{
238-
E_TYPE type;
239-
E_SAMPLE_COUNT_FLAGS samples;
240-
E_FORMAT format;
241-
VkExtent3D extent;
242-
uint32_t mipLevels;
243-
uint32_t arrayLayers;
244-
core::bitflag<E_CREATE_FLAGS> flags = ECF_NONE;
245-
core::bitflag<E_USAGE_FLAGS> usage = EUF_NONE;
237+
E_TYPE type;
238+
E_SAMPLE_COUNT_FLAGS samples;
239+
E_FORMAT format;
240+
VkExtent3D extent;
241+
uint32_t mipLevels;
242+
uint32_t arrayLayers;
243+
core::bitflag<E_CREATE_FLAGS> flags = ECF_NONE;
244+
core::bitflag<E_USAGE_FLAGS> usage = EUF_NONE;
246245

247246
inline bool operator==(const SCreationParams& rhs) const
248247
{
@@ -329,6 +328,8 @@ class IImage : public IDescriptor
329328
return false;
330329
if (_params.extent.width != _params.extent.height)
331330
return false;
331+
if (_params.extent.depth > 1u)
332+
return false;
332333
if (_params.arrayLayers < 6u)
333334
return false;
334335
if (_params.samples != ESCF_1_BIT)
@@ -587,7 +588,7 @@ class IImage : public IDescriptor
587588
//if (!formatHasAspects(m_creationParams.format,subresource.aspectMask))
588589
//return false;
589590
// The aspectMask member of imageSubresource must only have a single bit set
590-
if (!core::bitCount(static_cast<uint32_t>(subresource.aspectMask)) == 1u)
591+
if (!core::bitCount<uint32_t>(subresource.aspectMask.value) == 1u)
591592
return false;
592593
if (subresource.mipLevel >= m_creationParams.mipLevels)
593594
return false;
@@ -716,7 +717,7 @@ class IImage : public IDescriptor
716717
for (auto it2=it+1u; it2!=pRegionsEnd; it2++)
717718
{
718719
const auto& subresource2 = it2->getDstSubresource();
719-
if (!(subresource2.aspectMask&subresource.aspectMask))
720+
if (!(subresource2.aspectMask&subresource.aspectMask).value)
720721
continue;
721722
if (subresource2.mipLevel!=subresource.mipLevel)
722723
continue;

‎include/nbl/asset/IImageView.h

Lines changed: 99 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
// Copyright (C) 2018-2020 - DevSH Graphics Programming Sp. z O.O.
1+
// Copyright (C) 2018-2023 - DevSH Graphics Programming Sp. z O.O.
22
// This file is part of the "Nabla Engine".
33
// For conditions of distribution and use, see copyright notice in nabla.h
4-
5-
#ifndef __NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED__
6-
#define __NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED__
4+
#ifndef _NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED_
5+
#define _NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED_
76

87
#include "nbl/asset/IImage.h"
98

@@ -16,8 +15,8 @@ template<class ImageType>
1615
class IImageView : public IDescriptor
1716
{
1817
public:
19-
_NBL_STATIC_INLINE_CONSTEXPR size_t remaining_mip_levels = ~static_cast<size_t>(0u);
20-
_NBL_STATIC_INLINE_CONSTEXPR size_t remaining_array_layers = ~static_cast<size_t>(0u);
18+
static inline constexpr uint32_t remaining_mip_levels = ~static_cast<uint32_t>(0u);
19+
static inline constexpr uint32_t remaining_array_layers = ~static_cast<uint32_t>(0u);
2120

2221
// no flags for now, yet
2322
enum E_CREATE_FLAGS
@@ -80,12 +79,18 @@ class IImageView : public IDescriptor
8079
};
8180
struct SCreationParams
8281
{
83-
E_CREATE_FLAGS flags = static_cast<E_CREATE_FLAGS>(0);
84-
core::smart_refctd_ptr<ImageType> image;
85-
E_TYPE viewType;
86-
E_FORMAT format;
87-
SComponentMapping components;
88-
IImage::SSubresourceRange subresourceRange;
82+
E_CREATE_FLAGS flags = static_cast<E_CREATE_FLAGS>(0);
83+
// These are the set of usages for this ImageView, they must be a subset of the usages that `image` was created with.
84+
// If you leave it as the default NONE we'll inherit all usages from the `image`, setting it to anything else is
85+
// ONLY useful when creating multiple views of an image created with EXTENDED_USAGE to use different view formats.
86+
// Example: Create SRGB image with usage STORAGE, and two views with formats SRGB and R32_UINT. Then the SRGB view
87+
// CANNOT have STORAGE usage because the format doesn't support it, but the R32_UINT can.
88+
core::bitflag<IImage::E_USAGE_FLAGS> subUsages = IImage::EUF_NONE;
89+
core::smart_refctd_ptr<ImageType> image;
90+
E_TYPE viewType;
91+
E_FORMAT format;
92+
SComponentMapping components = {};
93+
IImage::SSubresourceRange subresourceRange = {IImage::EAF_COLOR_BIT,0,remaining_mip_levels,0,remaining_array_layers};
8994
};
9095
//!
9196
inline static bool validateCreationParameters(const SCreationParams& _params)
@@ -97,11 +102,45 @@ class IImageView : public IDescriptor
97102
return false;
98103

99104
const auto& imgParams = _params.image->getCreationParameters();
100-
bool mutableFormat = imgParams.flags.hasFlags(IImage::ECF_MUTABLE_FORMAT_BIT);
105+
/* TODO: LAter
106+
image must have been created with a usage value containing at least one of VK_IMAGE_USAGE_SAMPLED_BIT,
107+
VK_IMAGE_USAGE_STORAGE_BIT, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
108+
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT, VK_IMAGE_USAGE_SHADING_RATE_IMAGE_BIT_NV, or VK_IMAGE_USAGE_FRAGMENT_DENSITY_MAP_BIT_EXT
109+
if (imgParams.)
110+
return false;
111+
*/
112+
113+
// declared usages that are not a subset
114+
if (!imgParams.usage.hasFlags(_params.subUsages))
115+
return false;
116+
117+
const bool mutableFormat = imgParams.flags.hasFlags(IImage::ECF_MUTABLE_FORMAT_BIT);
118+
const bool blockTexelViewCompatible = imgParams.flags.hasFlags(IImage::ECF_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT);
119+
const auto& subresourceRange = _params.subresourceRange;
101120
if (mutableFormat)
102121
{
103-
//if (!isFormatCompatible(_params.format,imgParams.format))
104-
//return false;
122+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageCreateInfo-flags-01573
123+
// BlockTexelViewCompatible implies MutableFormat
124+
if (blockTexelViewCompatible)
125+
{
126+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-01583
127+
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag, format must be compatible with,
128+
// or must be an uncompressed format that is size-compatible with, the format used to create image
129+
if (getTexelOrBlockBytesize(_params.format)!=getTexelOrBlockBytesize(imgParams.format))
130+
return false;
131+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-07072
132+
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag and
133+
// format is a non-compressed format, the levelCount and layerCount members of subresourceRange must both be 1
134+
if (!asset::isBlockCompressionFormat(_params.format) && (subresourceRange.levelCount!=1u || subresourceRange.layerCount!=1u))
135+
return false;
136+
}
137+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-01761
138+
// If image was created with the VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT flag, but without the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag,
139+
// and if the format of the image is not a multi-planar format, format must be compatible with the format used to create image
140+
else if (getFormatClass(_params.format)!=getFormatClass(imgParams.format))
141+
return false;
142+
else if (asset::isBlockCompressionFormat(_params.format)!=asset::isBlockCompressionFormat(imgParams.format))
143+
return false;
105144

106145
/*
107146
TODO: if the format of the image is a multi-planar format, and if subresourceRange.aspectMask
@@ -110,90 +149,51 @@ class IImageView : public IDescriptor
110149
as defined in Compatible formats of planes of multi-planar formats
111150
*/
112151
}
113-
114-
115-
const auto& subresourceRange = _params.subresourceRange;
116-
117-
if (imgParams.flags.hasFlags(IImage::ECF_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT))
118-
{
119-
/*
120-
TODO: format must be compatible with, or must be an uncompressed format that is size-compatible with,
121-
the format used to create image.
122-
*/
123-
if (subresourceRange.levelCount!=1u || subresourceRange.layerCount!=1u)
124-
return false;
125-
}
126-
else
127-
{
128-
if (mutableFormat)
129-
{
130-
/*
131-
TODO: if the format of the image is not a multi-planar format,
132-
format must be compatible with the format used to create image,
133-
as defined in Format Compatibility Classes
134-
*/
135-
}
136-
}
137-
138-
if (!mutableFormat || asset::isPlanarFormat(imgParams.format))
152+
else if (_params.format!=imgParams.format)
139153
{
140-
/*
141-
TODO: format must be compatible with, or must be an uncompressed format that is size-compatible with,
142-
the format used to create image.
143-
*/
144-
}
145-
146-
/*
147-
image must have been created with a usage value containing at least one of VK_IMAGE_USAGE_SAMPLED_BIT,
148-
VK_IMAGE_USAGE_STORAGE_BIT, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
149-
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT, VK_IMAGE_USAGE_SHADING_RATE_IMAGE_BIT_NV, or VK_IMAGE_USAGE_FRAGMENT_DENSITY_MAP_BIT_EXT
150-
if (imgParams.)
154+
// TODO: multi-planar exceptions
151155
return false;
152-
*/
156+
}
157+
158+
//! sanity checks
153159

160+
// we have some layers
161+
if (subresourceRange.layerCount==0u)
162+
return false;
163+
164+
// mip level ranges
154165
if (subresourceRange.baseMipLevel>=imgParams.mipLevels)
155166
return false;
156167
if (subresourceRange.levelCount!=remaining_mip_levels &&
157168
(subresourceRange.levelCount==0u ||
158169
subresourceRange.baseMipLevel+subresourceRange.levelCount>imgParams.mipLevels))
159170
return false;
171+
160172
auto mipExtent = _params.image->getMipSize(subresourceRange.baseMipLevel);
161-
162-
if (subresourceRange.layerCount==0u)
163-
return false;
173+
auto actualLayerCount = subresourceRange.layerCount;
164174

165-
bool sourceIs3D = imgParams.type==IImage::ET_3D;
166-
bool sourceIs2DCompat = imgParams.flags.hasFlags(IImage::ECF_2D_ARRAY_COMPATIBLE_BIT) && (_params.viewType==ET_2D||_params.viewType==ET_2D_ARRAY);
167-
auto actualLayerCount = subresourceRange.layerCount!=remaining_array_layers ? subresourceRange.layerCount:
168-
((sourceIs3D&&sourceIs2DCompat ? mipExtent.z:imgParams.arrayLayers)-subresourceRange.baseArrayLayer);
169-
bool checkLayers = true;
170-
auto hasCubemapProporties = [&](bool isItACubemapArray = false)
175+
// the fact that source is 3D is implied by IImage::validateCreationParams
176+
const bool sourceIs2DCompat = imgParams.flags.hasFlags(IImage::ECF_2D_ARRAY_COMPATIBLE_BIT);
177+
if (subresourceRange.layerCount==remaining_array_layers)
171178
{
172-
if (!imgParams.flags.hasFlags(IImage::ECF_CUBE_COMPATIBLE_BIT))
173-
return false;
174-
if (imgParams.samples > 1u)
175-
return false;
176-
if (imgParams.extent.height != imgParams.extent.width)
177-
return false;
178-
if (imgParams.extent.depth > 1u)
179-
return false;
180-
if (actualLayerCount % 6u)
181-
return false;
179+
if (sourceIs2DCompat && _params.viewType!=ET_3D)
180+
actualLayerCount = mipExtent.z;
182181
else
183-
if (isItACubemapArray)
184-
{
185-
if (imgParams.arrayLayers < 6u)
186-
return false;
187-
}
188-
else
189-
if (imgParams.arrayLayers != 6u)
190-
return false;
191-
192-
if (subresourceRange.baseArrayLayer + actualLayerCount > imgParams.arrayLayers)
193-
return false;
194-
return true;
195-
};
182+
actualLayerCount = imgParams.arrayLayers;
183+
actualLayerCount -= subresourceRange.baseArrayLayer;
184+
}
185+
186+
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag, ... or must be an uncompressed format
187+
if (blockTexelViewCompatible && !asset::isBlockCompressionFormat(_params.format))
188+
{
189+
// In this case, the resulting image view’s texel dimensions equal the dimensions of the selected mip level divided by the compressed texel block size and rounded up.
190+
mipExtent = _params.image->getTexelBlockInfo().convertTexelsToBlocks(mipExtent);
191+
if (subresourceRange.layerCount==remaining_array_layers)
192+
actualLayerCount = 1;
193+
}
194+
const auto endLayer = actualLayerCount+subresourceRange.baseArrayLayer;
196195

196+
bool checkLayers = true;
197197
switch (_params.viewType)
198198
{
199199
case ET_1D:
@@ -203,8 +203,6 @@ class IImageView : public IDescriptor
203203
return false;
204204
[[fallthrough]];
205205
case ET_1D_ARRAY:
206-
if (imgParams.extent.height>1u || imgParams.extent.depth>1u)
207-
return false;
208206
break;
209207
case ET_2D:
210208
if (imgParams.type==IImage::ET_1D)
@@ -213,7 +211,7 @@ class IImageView : public IDescriptor
213211
return false;
214212
[[fallthrough]];
215213
case ET_2D_ARRAY:
216-
if (sourceIs3D)
214+
if (imgParams.type==IImage::ET_3D)
217215
{
218216
if (!sourceIs2DCompat)
219217
return false;
@@ -226,16 +224,22 @@ class IImageView : public IDescriptor
226224
return false;
227225
if (subresourceRange.baseArrayLayer>=mipExtent.z)
228226
return false;
229-
if (subresourceRange.baseArrayLayer+actualLayerCount>mipExtent.z)
227+
if (endLayer>mipExtent.z)
230228
return false;
231229
}
232230
break;
231+
case ET_CUBE_MAP:
232+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02960
233+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02962
234+
if (actualLayerCount!=6u)
235+
return false;
236+
[[fallthrough]];
233237
case ET_CUBE_MAP_ARRAY:
234-
if (!hasCubemapProporties(true))
238+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02961
239+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02963
240+
if (actualLayerCount%6u)
235241
return false;
236-
break;
237-
case ET_CUBE_MAP:
238-
if (!hasCubemapProporties(false))
242+
if (!imgParams.flags.hasFlags(IImage::ECF_CUBE_COMPATIBLE_BIT))
239243
return false;
240244
break;
241245
case ET_3D:
@@ -245,15 +249,14 @@ class IImageView : public IDescriptor
245249
return false;
246250
break;
247251
}
252+
248253
if (checkLayers)
249254
{
250255
if (subresourceRange.baseArrayLayer>=imgParams.arrayLayers)
251256
return false;
252-
if (subresourceRange.layerCount!=remaining_array_layers &&
253-
(subresourceRange.baseArrayLayer+subresourceRange.layerCount>imgParams.arrayLayers))
257+
if (endLayer>imgParams.arrayLayers)
254258
return false;
255259
}
256-
257260
return true;
258261
}
259262

0 commit comments

Comments
 (0)