Skip to content

Commit 27da404

Browse files
devshgraphicsprogrammingdevshgraphicsprogramming
committed
fix up validation of image view creation parameters
1 parent b455726 commit 27da404

File tree

3 files changed

+123
-119
lines changed

3 files changed

+123
-119
lines changed

‎include/nbl/asset/IImage.h

Lines changed: 24 additions & 23 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;

‎include/nbl/asset/IImageView.h

Lines changed: 99 additions & 94 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

@@ -80,12 +79,20 @@ 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+
#if 0
84+
// These are the set of usages for this ImageView, they must be a subset of the usages that `image` was created with.
85+
// If you leave it as the default NONE we'll inherit all usages from the `image`, setting it to anything else is
86+
// ONLY useful when creating multiple views of an image created with EXTENDED_USAGE to use different view formats.
87+
// Example: Create SRGB image with usage STORAGE, and two views with formats SRGB and R32_UINT. Then the SRGB view
88+
// CANNOT have STORAGE usage because the format doesn't support it, but the R32_UINT can.
89+
core::bitflag<IImage::E_USAGE_FLAGS> subUsages = EUF_NONE;
90+
#endif
91+
core::smart_refctd_ptr<ImageType> image;
92+
E_TYPE viewType;
93+
E_FORMAT format;
94+
SComponentMapping components;
95+
IImage::SSubresourceRange subresourceRange;
8996
};
9097
//!
9198
inline static bool validateCreationParameters(const SCreationParams& _params)
@@ -97,11 +104,45 @@ class IImageView : public IDescriptor
97104
return false;
98105

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

106147
/*
107148
TODO: if the format of the image is a multi-planar format, and if subresourceRange.aspectMask
@@ -110,90 +151,51 @@ class IImageView : public IDescriptor
110151
as defined in Compatible formats of planes of multi-planar formats
111152
*/
112153
}
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))
154+
else if (_params.format!=imgParams.format)
139155
{
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.)
156+
// TODO: multi-planar exceptions
151157
return false;
152-
*/
158+
}
153159

160+
//! sanity checks
161+
162+
// we have some layers
163+
if (subresourceRange.layerCount==0u)
164+
return false;
165+
166+
// mip level ranges
154167
if (subresourceRange.baseMipLevel>=imgParams.mipLevels)
155168
return false;
156169
if (subresourceRange.levelCount!=remaining_mip_levels &&
157170
(subresourceRange.levelCount==0u ||
158171
subresourceRange.baseMipLevel+subresourceRange.levelCount>imgParams.mipLevels))
159172
return false;
173+
160174
auto mipExtent = _params.image->getMipSize(subresourceRange.baseMipLevel);
161-
162-
if (subresourceRange.layerCount==0u)
163-
return false;
175+
auto actualLayerCount = subresourceRange.layerCount;
164176

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)
177+
// the fact that source is 3D is implied by IImage::validateCreationParams
178+
const bool sourceIs2DCompat = imgParams.flags.hasFlags(IImage::ECF_2D_ARRAY_COMPATIBLE_BIT);
179+
if (subresourceRange.layerCount==remaining_array_layers)
171180
{
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;
181+
if (sourceIs2DCompat && _params.viewType!=ET_3D)
182+
actualLayerCount = mipExtent.z;
182183
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-
};
184+
actualLayerCount = imgParams.arrayLayers;
185+
actualLayerCount -= subresourceRange.baseArrayLayer;
186+
}
187+
188+
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag, ... or must be an uncompressed format
189+
if (blockTexelViewCompatible && !asset::isBlockCompressionFormat(_params.format))
190+
{
191+
// 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.
192+
mipExtent = _params.image->getTexelBlockInfo().convertTexelsToBlocks(mipExtent);
193+
if (subresourceRange.layerCount==remaining_array_layers)
194+
actualLayerCount = 1;
195+
}
196+
const auto endLayer = actualLayerCount+subresourceRange.baseArrayLayer;
196197

198+
bool checkLayers = true;
197199
switch (_params.viewType)
198200
{
199201
case ET_1D:
@@ -203,8 +205,6 @@ class IImageView : public IDescriptor
203205
return false;
204206
[[fallthrough]];
205207
case ET_1D_ARRAY:
206-
if (imgParams.extent.height>1u || imgParams.extent.depth>1u)
207-
return false;
208208
break;
209209
case ET_2D:
210210
if (imgParams.type==IImage::ET_1D)
@@ -213,7 +213,7 @@ class IImageView : public IDescriptor
213213
return false;
214214
[[fallthrough]];
215215
case ET_2D_ARRAY:
216-
if (sourceIs3D)
216+
if (imgParams.type==IImage::ET_3D)
217217
{
218218
if (!sourceIs2DCompat)
219219
return false;
@@ -226,16 +226,22 @@ class IImageView : public IDescriptor
226226
return false;
227227
if (subresourceRange.baseArrayLayer>=mipExtent.z)
228228
return false;
229-
if (subresourceRange.baseArrayLayer+actualLayerCount>mipExtent.z)
229+
if (endLayer>mipExtent.z)
230230
return false;
231231
}
232232
break;
233-
case ET_CUBE_MAP_ARRAY:
234-
if (!hasCubemapProporties(true))
233+
case ET_CUBE_MAP:
234+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02960
235+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02962
236+
if (actualLayerCount!=6u)
235237
return false;
236238
break;
237-
case ET_CUBE_MAP:
238-
if (!hasCubemapProporties(false))
239+
case ET_CUBE_MAP_ARRAY:
240+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02961
241+
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02963
242+
if (actualLayerCount%6u)
243+
return false;
244+
if (!imgParams.flags.hasFlags(IImage::ECF_CUBE_COMPATIBLE_BIT))
239245
return false;
240246
break;
241247
case ET_3D:
@@ -245,15 +251,14 @@ class IImageView : public IDescriptor
245251
return false;
246252
break;
247253
}
254+
248255
if (checkLayers)
249256
{
250257
if (subresourceRange.baseArrayLayer>=imgParams.arrayLayers)
251258
return false;
252-
if (subresourceRange.layerCount!=remaining_array_layers &&
253-
(subresourceRange.baseArrayLayer+subresourceRange.layerCount>imgParams.arrayLayers))
259+
if (endLayer>imgParams.arrayLayers)
254260
return false;
255261
}
256-
257262
return true;
258263
}
259264

‎include/nbl/video/utilities/IGPUObjectFromAssetConverter.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,6 @@ auto IGPUObjectFromAssetConverter::create(const asset::ICPUImage** const _begin,
10641064
assert(newFormat != asset::EF_UNKNOWN); // No feasible supported format found for creating this image
10651065
params.format = newFormat;
10661066

1067-
#if 0 // TODO: Bump our minimum Vulkan version to 1.2, then implement https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkImageFormatListCreateInfo.html in our API
10681067
// now add the STORAGE USAGE
10691068
if (computeMips)
10701069
{
@@ -1079,7 +1078,6 @@ auto IGPUObjectFromAssetConverter::create(const asset::ICPUImage** const _begin,
10791078
params.flags |= asset::IImage::ECF_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT;
10801079
}
10811080
}
1082-
#endif
10831081

10841082
auto gpuimg = _params.device->createImage(std::move(params));
10851083
auto gpuimgMemReqs = gpuimg->getMemoryReqs();

0 commit comments

Comments
 (0)