Skip to content

Commit

Permalink
Merge pull request #499 from Devsh-Graphics-Programming/fix_472
Browse files Browse the repository at this point in the history 
Validation Error Fixes and Threadsafety
  • Loading branch information
@devshgraphicsprogramming
devshgraphicsprogramming authored Jun 1, 2023
2 parents 57a69bd + 4018e72 commit 64da18c
Show file tree
Hide file tree
Showing 28 changed files with 869 additions and 546 deletions.
2 changes: 1 addition & 1 deletion examples_tests
Submodule examples_tests updated 13 files
+ − 01.HelloWorld/bin/screenshot.png
+7 −81 02.ComputeShader/main.cpp
+7 −10 05.NablaTutorialExample/main.cpp
+2 −2 06.MeshLoaders/main.cpp
+2 −2 07.SubpassBaking/main.cpp
+0 −7 40.GLITest/CMakeLists.txt
+0 −76 40.GLITest/main.cpp
+0 −7 63.SierpinskiSDF/CMakeLists.txt
+0 −363 63.SierpinskiSDF/main.cpp
+0 −126 63.SierpinskiSDF/shader.frag
+1 −2 CMakeLists.txt
+2 −2 common/CommonAPI.cpp
+2 −2 common/CommonAPI.h
49 changes: 25 additions & 24 deletions include/nbl/asset/IImage.h
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// Copyright (C) 2018-2020 - DevSH Graphics Programming Sp. z O.O.
// Copyright (C) 2018-2023 - DevSH Graphics Programming Sp. z O.O.
// This file is part of the "Nabla Engine".
// For conditions of distribution and use, see copyright notice in nabla.h

#ifndef __NBL_ASSET_I_IMAGE_H_INCLUDED__
#define __NBL_ASSET_I_IMAGE_H_INCLUDED__
#ifndef _NBL_ASSET_I_IMAGE_H_INCLUDED_
#define _NBL_ASSET_I_IMAGE_H_INCLUDED_

#include "nbl/core/util/bitflag.h"
#include "nbl/core/containers/refctd_dynamic_array.h"
Expand Down Expand Up @@ -136,18 +135,18 @@ class IImage : public IDescriptor
};
struct SSubresourceRange
{
E_ASPECT_FLAGS aspectMask = E_ASPECT_FLAGS::EAF_NONE;
uint32_t baseMipLevel = 0u;
uint32_t levelCount = 0u;
uint32_t baseArrayLayer = 0u;
uint32_t layerCount = 0u;
core::bitflag<E_ASPECT_FLAGS> aspectMask = E_ASPECT_FLAGS::EAF_NONE;
uint32_t baseMipLevel = 0u;
uint32_t levelCount = 0u;
uint32_t baseArrayLayer = 0u;
uint32_t layerCount = 0u;
};
struct SSubresourceLayers
{
E_ASPECT_FLAGS aspectMask = E_ASPECT_FLAGS::EAF_NONE;
uint32_t mipLevel = 0u;
uint32_t baseArrayLayer = 0u;
uint32_t layerCount = 0u;
core::bitflag<E_ASPECT_FLAGS> aspectMask = E_ASPECT_FLAGS::EAF_NONE;
uint32_t mipLevel = 0u;
uint32_t baseArrayLayer = 0u;
uint32_t layerCount = 0u;

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

if (srcSubresource.layerCount!=dstSubresource.layerCount)
Expand All @@ -235,14 +234,14 @@ class IImage : public IDescriptor
};
struct SCreationParams
{
E_TYPE type;
E_SAMPLE_COUNT_FLAGS samples;
E_FORMAT format;
VkExtent3D extent;
uint32_t mipLevels;
uint32_t arrayLayers;
core::bitflag<E_CREATE_FLAGS> flags = ECF_NONE;
core::bitflag<E_USAGE_FLAGS> usage = EUF_NONE;
E_TYPE type;
E_SAMPLE_COUNT_FLAGS samples;
E_FORMAT format;
VkExtent3D extent;
uint32_t mipLevels;
uint32_t arrayLayers;
core::bitflag<E_CREATE_FLAGS> flags = ECF_NONE;
core::bitflag<E_USAGE_FLAGS> usage = EUF_NONE;

inline bool operator==(const SCreationParams& rhs) const
{
Expand Down Expand Up @@ -329,6 +328,8 @@ class IImage : public IDescriptor
return false;
if (_params.extent.width != _params.extent.height)
return false;
if (_params.extent.depth > 1u)
return false;
if (_params.arrayLayers < 6u)
return false;
if (_params.samples != ESCF_1_BIT)
Expand Down Expand Up @@ -587,7 +588,7 @@ class IImage : public IDescriptor
//if (!formatHasAspects(m_creationParams.format,subresource.aspectMask))
//return false;
// The aspectMask member of imageSubresource must only have a single bit set
if (!core::bitCount(static_cast<uint32_t>(subresource.aspectMask)) == 1u)
if (!core::bitCount<uint32_t>(subresource.aspectMask.value) == 1u)
return false;
if (subresource.mipLevel >= m_creationParams.mipLevels)
return false;
Expand Down Expand Up @@ -716,7 +717,7 @@ class IImage : public IDescriptor
for (auto it2=it+1u; it2!=pRegionsEnd; it2++)
{
const auto& subresource2 = it2->getDstSubresource();
if (!(subresource2.aspectMask&subresource.aspectMask))
if (!(subresource2.aspectMask&subresource.aspectMask).value)
continue;
if (subresource2.mipLevel!=subresource.mipLevel)
continue;
Expand Down
195 changes: 99 additions & 96 deletions include/nbl/asset/IImageView.h
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// Copyright (C) 2018-2020 - DevSH Graphics Programming Sp. z O.O.
// Copyright (C) 2018-2023 - DevSH Graphics Programming Sp. z O.O.
// This file is part of the "Nabla Engine".
// For conditions of distribution and use, see copyright notice in nabla.h

#ifndef __NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED__
#define __NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED__
#ifndef _NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED_
#define _NBL_ASSET_I_IMAGE_VIEW_H_INCLUDED_

#include "nbl/asset/IImage.h"

Expand All @@ -16,8 +15,8 @@ template<class ImageType>
class IImageView : public IDescriptor
{
public:
_NBL_STATIC_INLINE_CONSTEXPR size_t remaining_mip_levels = ~static_cast<size_t>(0u);
_NBL_STATIC_INLINE_CONSTEXPR size_t remaining_array_layers = ~static_cast<size_t>(0u);
static inline constexpr uint32_t remaining_mip_levels = ~static_cast<uint32_t>(0u);
static inline constexpr uint32_t remaining_array_layers = ~static_cast<uint32_t>(0u);

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

const auto& imgParams = _params.image->getCreationParameters();
bool mutableFormat = imgParams.flags.hasFlags(IImage::ECF_MUTABLE_FORMAT_BIT);
/* TODO: LAter
image must have been created with a usage value containing at least one of VK_IMAGE_USAGE_SAMPLED_BIT,
VK_IMAGE_USAGE_STORAGE_BIT, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT, VK_IMAGE_USAGE_SHADING_RATE_IMAGE_BIT_NV, or VK_IMAGE_USAGE_FRAGMENT_DENSITY_MAP_BIT_EXT
if (imgParams.)
return false;
*/

// declared usages that are not a subset
if (!imgParams.usage.hasFlags(_params.subUsages))
return false;

const bool mutableFormat = imgParams.flags.hasFlags(IImage::ECF_MUTABLE_FORMAT_BIT);
const bool blockTexelViewCompatible = imgParams.flags.hasFlags(IImage::ECF_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT);
const auto& subresourceRange = _params.subresourceRange;
if (mutableFormat)
{
//if (!isFormatCompatible(_params.format,imgParams.format))
//return false;
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageCreateInfo-flags-01573
// BlockTexelViewCompatible implies MutableFormat
if (blockTexelViewCompatible)
{
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-01583
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag, format must be compatible with,
// or must be an uncompressed format that is size-compatible with, the format used to create image
if (getTexelOrBlockBytesize(_params.format)!=getTexelOrBlockBytesize(imgParams.format))
return false;
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-07072
// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag and
// format is a non-compressed format, the levelCount and layerCount members of subresourceRange must both be 1
if (!asset::isBlockCompressionFormat(_params.format) && (subresourceRange.levelCount!=1u || subresourceRange.layerCount!=1u))
return false;
}
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-image-01761
// 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,
// and if the format of the image is not a multi-planar format, format must be compatible with the format used to create image
else if (getFormatClass(_params.format)!=getFormatClass(imgParams.format))
return false;
else if (asset::isBlockCompressionFormat(_params.format)!=asset::isBlockCompressionFormat(imgParams.format))
return false;

/*
TODO: if the format of the image is a multi-planar format, and if subresourceRange.aspectMask
Expand All @@ -110,90 +149,51 @@ class IImageView : public IDescriptor
as defined in Compatible formats of planes of multi-planar formats
*/
}


const auto& subresourceRange = _params.subresourceRange;

if (imgParams.flags.hasFlags(IImage::ECF_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT))
{
/*
TODO: format must be compatible with, or must be an uncompressed format that is size-compatible with,
the format used to create image.
*/
if (subresourceRange.levelCount!=1u || subresourceRange.layerCount!=1u)
return false;
}
else
{
if (mutableFormat)
{
/*
TODO: if the format of the image is not a multi-planar format,
format must be compatible with the format used to create image,
as defined in Format Compatibility Classes
*/
}
}

if (!mutableFormat || asset::isPlanarFormat(imgParams.format))
else if (_params.format!=imgParams.format)
{
/*
TODO: format must be compatible with, or must be an uncompressed format that is size-compatible with,
the format used to create image.
*/
}

/*
image must have been created with a usage value containing at least one of VK_IMAGE_USAGE_SAMPLED_BIT,
VK_IMAGE_USAGE_STORAGE_BIT, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT, VK_IMAGE_USAGE_SHADING_RATE_IMAGE_BIT_NV, or VK_IMAGE_USAGE_FRAGMENT_DENSITY_MAP_BIT_EXT
if (imgParams.)
// TODO: multi-planar exceptions
return false;
*/
}

//! sanity checks

// we have some layers
if (subresourceRange.layerCount==0u)
return false;

// mip level ranges
if (subresourceRange.baseMipLevel>=imgParams.mipLevels)
return false;
if (subresourceRange.levelCount!=remaining_mip_levels &&
(subresourceRange.levelCount==0u ||
subresourceRange.baseMipLevel+subresourceRange.levelCount>imgParams.mipLevels))
return false;

auto mipExtent = _params.image->getMipSize(subresourceRange.baseMipLevel);

if (subresourceRange.layerCount==0u)
return false;
auto actualLayerCount = subresourceRange.layerCount;

bool sourceIs3D = imgParams.type==IImage::ET_3D;
bool sourceIs2DCompat = imgParams.flags.hasFlags(IImage::ECF_2D_ARRAY_COMPATIBLE_BIT) && (_params.viewType==ET_2D||_params.viewType==ET_2D_ARRAY);
auto actualLayerCount = subresourceRange.layerCount!=remaining_array_layers ? subresourceRange.layerCount:
((sourceIs3D&&sourceIs2DCompat ? mipExtent.z:imgParams.arrayLayers)-subresourceRange.baseArrayLayer);
bool checkLayers = true;
auto hasCubemapProporties = [&](bool isItACubemapArray = false)
// the fact that source is 3D is implied by IImage::validateCreationParams
const bool sourceIs2DCompat = imgParams.flags.hasFlags(IImage::ECF_2D_ARRAY_COMPATIBLE_BIT);
if (subresourceRange.layerCount==remaining_array_layers)
{
if (!imgParams.flags.hasFlags(IImage::ECF_CUBE_COMPATIBLE_BIT))
return false;
if (imgParams.samples > 1u)
return false;
if (imgParams.extent.height != imgParams.extent.width)
return false;
if (imgParams.extent.depth > 1u)
return false;
if (actualLayerCount % 6u)
return false;
if (sourceIs2DCompat && _params.viewType!=ET_3D)
actualLayerCount = mipExtent.z;
else
if (isItACubemapArray)
{
if (imgParams.arrayLayers < 6u)
return false;
}
else
if (imgParams.arrayLayers != 6u)
return false;
if (subresourceRange.baseArrayLayer + actualLayerCount > imgParams.arrayLayers)
return false;
return true;
};
actualLayerCount = imgParams.arrayLayers;
actualLayerCount -= subresourceRange.baseArrayLayer;
}

// If image was created with the VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT flag, ... or must be an uncompressed format
if (blockTexelViewCompatible && !asset::isBlockCompressionFormat(_params.format))
{
// 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.
mipExtent = _params.image->getTexelBlockInfo().convertTexelsToBlocks(mipExtent);
if (subresourceRange.layerCount==remaining_array_layers)
actualLayerCount = 1;
}
const auto endLayer = actualLayerCount+subresourceRange.baseArrayLayer;

bool checkLayers = true;
switch (_params.viewType)
{
case ET_1D:
Expand All @@ -203,8 +203,6 @@ class IImageView : public IDescriptor
return false;
[[fallthrough]];
case ET_1D_ARRAY:
if (imgParams.extent.height>1u || imgParams.extent.depth>1u)
return false;
break;
case ET_2D:
if (imgParams.type==IImage::ET_1D)
Expand All @@ -213,7 +211,7 @@ class IImageView : public IDescriptor
return false;
[[fallthrough]];
case ET_2D_ARRAY:
if (sourceIs3D)
if (imgParams.type==IImage::ET_3D)
{
if (!sourceIs2DCompat)
return false;
Expand All @@ -226,16 +224,22 @@ class IImageView : public IDescriptor
return false;
if (subresourceRange.baseArrayLayer>=mipExtent.z)
return false;
if (subresourceRange.baseArrayLayer+actualLayerCount>mipExtent.z)
if (endLayer>mipExtent.z)
return false;
}
break;
case ET_CUBE_MAP:
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02960
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02962
if (actualLayerCount!=6u)
return false;
[[fallthrough]];
case ET_CUBE_MAP_ARRAY:
if (!hasCubemapProporties(true))
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02961
// https://registry.khronos.org/vulkan/specs/1.2/html/vkspec.html#VUID-VkImageViewCreateInfo-viewType-02963
if (actualLayerCount%6u)
return false;
break;
case ET_CUBE_MAP:
if (!hasCubemapProporties(false))
if (!imgParams.flags.hasFlags(IImage::ECF_CUBE_COMPATIBLE_BIT))
return false;
break;
case ET_3D:
Expand All @@ -245,15 +249,14 @@ class IImageView : public IDescriptor
return false;
break;
}

if (checkLayers)
{
if (subresourceRange.baseArrayLayer>=imgParams.arrayLayers)
return false;
if (subresourceRange.layerCount!=remaining_array_layers &&
(subresourceRange.baseArrayLayer+subresourceRange.layerCount>imgParams.arrayLayers))
if (endLayer>imgParams.arrayLayers)
return false;
}

return true;
}

Expand Down
Loading

0 comments on commit 64da18c

Please sign in to comment.