layers: GH535 Add checks for Clear image errors
authorTobin Ehlis <tobine@google.com>
Thu, 19 May 2016 15:59:30 +0000 (09:59 -0600)
committerTobin Ehlis <tobine@google.com>
Thu, 19 May 2016 19:09:50 +0000 (13:09 -0600)
This adds two new checks to make sure that CmdClearColorImage is
not called with depth/stencil format image and that
CmdClearDepthStencilImage is not called with color format image.

Also add getImageState() helper function to image layer to
simplify/unify code for lookup of IMAGE_STATE from imageMap.

layers/image.cpp

index 840c543..43b6452 100644 (file)
@@ -76,6 +76,14 @@ static void init_image(layer_data *my_data, const VkAllocationCallbacks *pAlloca
     layer_debug_actions(my_data->report_data, my_data->logging_callback, pAllocator, "lunarg_image");
 }
 
+static IMAGE_STATE const *getImageState(layer_data const *dev_data, VkImage image) {
+    auto it = dev_data->imageMap.find(image);
+    if (it == dev_data->imageMap.end()) {
+        return nullptr;
+    }
+    return &it->second;
+}
+
 VKAPI_ATTR VkResult VKAPI_CALL
 CreateDebugReportCallbackEXT(VkInstance instance, const VkDebugReportCallbackCreateInfoEXT *pCreateInfo,
                              const VkAllocationCallbacks *pAllocator, VkDebugReportCallbackEXT *pMsgCallback) {
@@ -477,19 +485,19 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateImageView(VkDevice device, const VkImageVie
                                                const VkAllocationCallbacks *pAllocator, VkImageView *pView) {
     bool skipCall = false;
     layer_data *device_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map);
-    auto imageEntry = device_data->imageMap.find(pCreateInfo->image);
-    if (imageEntry != device_data->imageMap.end()) {
-        if (pCreateInfo->subresourceRange.baseMipLevel >= imageEntry->second.mipLevels) {
+    auto imageEntry = getImageState(device_data, pCreateInfo->image);
+    if (imageEntry) {
+        if (pCreateInfo->subresourceRange.baseMipLevel >= imageEntry->mipLevels) {
             std::stringstream ss;
             ss << "vkCreateImageView called with baseMipLevel " << pCreateInfo->subresourceRange.baseMipLevel << " for image "
-               << pCreateInfo->image << " that only has " << imageEntry->second.mipLevels << " mip levels.";
+               << pCreateInfo->image << " that only has " << imageEntry->mipLevels << " mip levels.";
             skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__,
                                 IMAGE_VIEW_CREATE_ERROR, "IMAGE", "%s", ss.str().c_str());
         }
-        if (pCreateInfo->subresourceRange.baseArrayLayer >= imageEntry->second.arraySize) {
+        if (pCreateInfo->subresourceRange.baseArrayLayer >= imageEntry->arraySize) {
             std::stringstream ss;
             ss << "vkCreateImageView called with baseArrayLayer " << pCreateInfo->subresourceRange.baseArrayLayer << " for image "
-               << pCreateInfo->image << " that only has " << imageEntry->second.arraySize << " array layers.";
+               << pCreateInfo->image << " that only has " << imageEntry->arraySize << " array layers.";
             skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, (VkDebugReportObjectTypeEXT)0, 0, __LINE__,
                                 IMAGE_VIEW_CREATE_ERROR, "IMAGE", "%s", ss.str().c_str());
         }
@@ -506,8 +514,8 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateImageView(VkDevice device, const VkImageVie
                                 IMAGE_VIEW_CREATE_ERROR, "IMAGE", "%s", ss.str().c_str());
         }
 
-        VkImageCreateFlags imageFlags = imageEntry->second.flags;
-        VkFormat imageFormat = imageEntry->second.format;
+        VkImageCreateFlags imageFlags = imageEntry->flags;
+        VkFormat imageFormat = imageEntry->format;
         VkFormat ivciFormat = pCreateInfo->format;
         VkImageAspectFlags aspectMask = pCreateInfo->subresourceRange.aspectMask;
 
@@ -644,6 +652,19 @@ VKAPI_ATTR void VKAPI_CALL CmdClearColorImage(VkCommandBuffer commandBuffer, VkI
         }
     }
 
+    auto image_state = getImageState(device_data, image);
+    if (image_state) {
+        if (vk_format_is_depth_or_stencil(image_state->format)) {
+            char const str[] = "vkCmdClearColorImage called with depth/stencil image.";
+            skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT,
+                                reinterpret_cast<uint64_t &>(image), __LINE__, IMAGE_INVALID_FORMAT, "IMAGE", str);
+        } else if (vk_format_is_compressed(image_state->format)) {
+            char const str[] = "vkCmdClearColorImage called with compressed image.";
+            skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT,
+                                reinterpret_cast<uint64_t &>(image), __LINE__, IMAGE_INVALID_FORMAT, "IMAGE", str);
+        }
+    }
+
     if (!skipCall) {
         device_data->device_dispatch_table->CmdClearColorImage(commandBuffer, image, imageLayout, pColor, rangeCount, pRanges);
     }
@@ -667,6 +688,13 @@ CmdClearDepthStencilImage(VkCommandBuffer commandBuffer, VkImage image, VkImageL
         }
     }
 
+    auto image_state = getImageState(device_data, image);
+    if (image_state && !vk_format_is_depth_or_stencil(image_state->format)) {
+        char const str[] = "vkCmdClearDepthStencilImage called without a depth/stencil image.";
+        skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT,
+                            reinterpret_cast<uint64_t &>(image), __LINE__, IMAGE_INVALID_FORMAT, "IMAGE", str);
+    }
+
     if (!skipCall) {
         device_data->device_dispatch_table->CmdClearDepthStencilImage(commandBuffer, image, imageLayout, pDepthStencil, rangeCount,
                                                                       pRanges);
@@ -711,7 +739,7 @@ static bool region_intersects(const VkImageCopy *src, const VkImageCopy *dst, Vk
 }
 
 // Returns true if offset and extent exceed image extents
-static bool exceeds_bounds(const VkOffset3D *offset, const VkExtent3D *extent, IMAGE_STATE *image) {
+static bool exceeds_bounds(const VkOffset3D *offset, const VkExtent3D *extent, const IMAGE_STATE *image) {
     bool result = false;
     // Extents/depths cannot be negative but checks left in for clarity
 
@@ -723,12 +751,12 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
 
     bool skipCall = false;
     layer_data *device_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
-    auto srcImageEntry = device_data->imageMap.find(srcImage);
-    auto dstImageEntry = device_data->imageMap.find(dstImage);
+    auto srcImageEntry = getImageState(device_data, srcImage);
+    auto dstImageEntry = getImageState(device_data, dstImage);
 
     // TODO: This does not cover swapchain-created images. This should fall out when this layer is moved
     // into the core_validation layer
-    if ((srcImageEntry != device_data->imageMap.end()) && (dstImageEntry != device_data->imageMap.end())) {
+    if (srcImageEntry && dstImageEntry) {
 
         for (uint32_t i = 0; i < regionCount; i++) {
 
@@ -788,7 +816,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
 
             // If either of the calling command's srcImage or dstImage parameters are of VkImageType VK_IMAGE_TYPE_3D,
             // the baseArrayLayer and layerCount members of both srcSubresource and dstSubresource must be 0 and 1, respectively
-            if (((srcImageEntry->second.imageType == VK_IMAGE_TYPE_3D) || (dstImageEntry->second.imageType == VK_IMAGE_TYPE_3D)) &&
+            if (((srcImageEntry->imageType == VK_IMAGE_TYPE_3D) || (dstImageEntry->imageType == VK_IMAGE_TYPE_3D)) &&
                 ((pRegions[i].srcSubresource.baseArrayLayer != 0) || (pRegions[i].srcSubresource.layerCount != 1) ||
                  (pRegions[i].dstSubresource.baseArrayLayer != 0) || (pRegions[i].dstSubresource.layerCount != 1))) {
                 std::stringstream ss;
@@ -800,7 +828,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
             }
 
             // MipLevel must be less than the mipLevels specified in VkImageCreateInfo when the image was created
-            if (pRegions[i].srcSubresource.mipLevel >= srcImageEntry->second.mipLevels) {
+            if (pRegions[i].srcSubresource.mipLevel >= srcImageEntry->mipLevels) {
                 std::stringstream ss;
                 ss << "vkCmdCopyImage: pRegions[" << i
                    << "] specifies a src mipLevel greater than the number specified when the srcImage was created.";
@@ -808,7 +836,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
                                     VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast<uint64_t &>(commandBuffer),
                                     __LINE__, IMAGE_INVALID_EXTENTS, "IMAGE", "%s", ss.str().c_str());
             }
-            if (pRegions[i].dstSubresource.mipLevel >= dstImageEntry->second.mipLevels) {
+            if (pRegions[i].dstSubresource.mipLevel >= dstImageEntry->mipLevels) {
                 std::stringstream ss;
                 ss << "vkCmdCopyImage: pRegions[" << i
                    << "] specifies a dst mipLevel greater than the number specified when the dstImage was created.";
@@ -819,20 +847,18 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
 
             // (baseArrayLayer + layerCount) must be less than or equal to the arrayLayers specified in VkImageCreateInfo when the
             // image was created
-            if ((pRegions[i].srcSubresource.baseArrayLayer + pRegions[i].srcSubresource.layerCount) >
-                srcImageEntry->second.arraySize) {
+            if ((pRegions[i].srcSubresource.baseArrayLayer + pRegions[i].srcSubresource.layerCount) > srcImageEntry->arraySize) {
                 std::stringstream ss;
-                ss << "vkCmdCopyImage: srcImage arrayLayers was " << srcImageEntry->second.arraySize << " but subRegion[" << i
+                ss << "vkCmdCopyImage: srcImage arrayLayers was " << srcImageEntry->arraySize << " but subRegion[" << i
                    << "] baseArrayLayer + layerCount is "
                    << (pRegions[i].srcSubresource.baseArrayLayer + pRegions[i].srcSubresource.layerCount);
                 skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
                                     VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, reinterpret_cast<uint64_t &>(commandBuffer),
                                     __LINE__, IMAGE_INVALID_EXTENTS, "IMAGE", "%s", ss.str().c_str());
             }
-            if ((pRegions[i].dstSubresource.baseArrayLayer + pRegions[i].dstSubresource.layerCount) >
-                dstImageEntry->second.arraySize) {
+            if ((pRegions[i].dstSubresource.baseArrayLayer + pRegions[i].dstSubresource.layerCount) > dstImageEntry->arraySize) {
                 std::stringstream ss;
-                ss << "vkCmdCopyImage: dstImage arrayLayers was " << dstImageEntry->second.arraySize << " but subRegion[" << i
+                ss << "vkCmdCopyImage: dstImage arrayLayers was " << dstImageEntry->arraySize << " but subRegion[" << i
                    << "] baseArrayLayer + layerCount is "
                    << (pRegions[i].dstSubresource.baseArrayLayer + pRegions[i].dstSubresource.layerCount);
                 skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
@@ -841,7 +867,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
             }
 
             // The source region specified by a given element of pRegions must be a region that is contained within srcImage
-            if (exceeds_bounds(&pRegions[i].srcOffset, &pRegions[i].extent, &srcImageEntry->second)) {
+            if (exceeds_bounds(&pRegions[i].srcOffset, &pRegions[i].extent, srcImageEntry)) {
                 std::stringstream ss;
                 ss << "vkCmdCopyImage: srcSubResource in pRegions[" << i << "] exceeds extents srcImage was created with";
                 skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
@@ -850,7 +876,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
             }
 
             // The destination region specified by a given element of pRegions must be a region that is contained within dstImage
-            if (exceeds_bounds(&pRegions[i].dstOffset, &pRegions[i].extent, &dstImageEntry->second)) {
+            if (exceeds_bounds(&pRegions[i].dstOffset, &pRegions[i].extent, dstImageEntry)) {
                 std::stringstream ss;
                 ss << "vkCmdCopyImage: dstSubResource in pRegions[" << i << "] exceeds extents dstImage was created with";
                 skipCall |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
@@ -862,7 +888,7 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
             // must not overlap in memory
             if (srcImage == dstImage) {
                 for (uint32_t j = 0; j < regionCount; j++) {
-                    if (region_intersects(&pRegions[i], &pRegions[j], srcImageEntry->second.imageType)) {
+                    if (region_intersects(&pRegions[i], &pRegions[j], srcImageEntry->imageType)) {
                         std::stringstream ss;
                         ss << "vkCmdCopyImage: pRegions[" << i << "] src overlaps with pRegions[" << j << "].";
                         skipCall |=
@@ -877,16 +903,16 @@ bool cmd_copy_image_valid_usage(VkCommandBuffer commandBuffer, VkImage srcImage,
         // The formats of srcImage and dstImage must be compatible. Formats are considered compatible if their texel size in bytes
         // is the same between both formats. For example, VK_FORMAT_R8G8B8A8_UNORM is compatible with VK_FORMAT_R32_UINT because
         // because both texels are 4 bytes in size. Depth/stencil formats must match exactly.
-        if (is_depth_format(srcImageEntry->second.format) || is_depth_format(dstImageEntry->second.format)) {
-            if (srcImageEntry->second.format != dstImageEntry->second.format) {
+        if (is_depth_format(srcImageEntry->format) || is_depth_format(dstImageEntry->format)) {
+            if (srcImageEntry->format != dstImageEntry->format) {
                 char const str[] = "vkCmdCopyImage called with unmatched source and dest image depth/stencil formats.";
                 skipCall |=
                     log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
                             reinterpret_cast<uint64_t &>(commandBuffer), __LINE__, IMAGE_MISMATCHED_IMAGE_FORMAT, "IMAGE", str);
             }
         } else {
-            size_t srcSize = vk_format_get_size(srcImageEntry->second.format);
-            size_t destSize = vk_format_get_size(dstImageEntry->second.format);
+            size_t srcSize = vk_format_get_size(srcImageEntry->format);
+            size_t destSize = vk_format_get_size(dstImageEntry->format);
             if (srcSize != destSize) {
                 char const str[] = "vkCmdCopyImage called with unmatched source and dest image format sizes.";
                 skipCall |=
@@ -1019,13 +1045,13 @@ CmdBlitImage(VkCommandBuffer commandBuffer, VkImage srcImage, VkImageLayout srcI
     bool skipCall = false;
     layer_data *device_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
 
-    auto srcImageEntry = device_data->imageMap.find(srcImage);
-    auto dstImageEntry = device_data->imageMap.find(dstImage);
+    auto srcImageEntry = getImageState(device_data, srcImage);
+    auto dstImageEntry = getImageState(device_data, dstImage);
 
-    if ((srcImageEntry != device_data->imageMap.end()) && (dstImageEntry != device_data->imageMap.end())) {
+    if (srcImageEntry && dstImageEntry) {
 
-        VkFormat srcFormat = srcImageEntry->second.format;
-        VkFormat dstFormat = dstImageEntry->second.format;
+        VkFormat srcFormat = srcImageEntry->format;
+        VkFormat dstFormat = dstImageEntry->format;
 
         // Validate consistency for signed and unsigned formats
         if ((vk_format_is_sint(srcFormat) && !vk_format_is_sint(dstFormat)) ||
@@ -1171,8 +1197,8 @@ CmdResolveImage(VkCommandBuffer commandBuffer, VkImage srcImage, VkImageLayout s
                 VkImageLayout dstImageLayout, uint32_t regionCount, const VkImageResolve *pRegions) {
     bool skipCall = false;
     layer_data *device_data = get_my_data_ptr(get_dispatch_key(commandBuffer), layer_data_map);
-    auto srcImageEntry = device_data->imageMap.find(srcImage);
-    auto dstImageEntry = device_data->imageMap.find(dstImage);
+    auto srcImageEntry = getImageState(device_data, srcImage);
+    auto dstImageEntry = getImageState(device_data, dstImage);
 
     // For each region, the number of layers in the image subresource should not be zero
     // For each region, src and dest image aspect must be color only
@@ -1204,26 +1230,26 @@ CmdResolveImage(VkCommandBuffer commandBuffer, VkImage srcImage, VkImageLayout s
         }
     }
 
-    if ((srcImageEntry != device_data->imageMap.end()) && (dstImageEntry != device_data->imageMap.end())) {
-        if (srcImageEntry->second.format != dstImageEntry->second.format) {
+    if (srcImageEntry && dstImageEntry) {
+        if (srcImageEntry->format != dstImageEntry->format) {
             char const str[] = "vkCmdResolveImage called with unmatched source and dest formats.";
             skipCall |=
                 log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
                         (uint64_t)commandBuffer, __LINE__, IMAGE_MISMATCHED_IMAGE_FORMAT, "IMAGE", str);
         }
-        if (srcImageEntry->second.imageType != dstImageEntry->second.imageType) {
+        if (srcImageEntry->imageType != dstImageEntry->imageType) {
             char const str[] = "vkCmdResolveImage called with unmatched source and dest image types.";
             skipCall |=
                 log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
                         (uint64_t)commandBuffer, __LINE__, IMAGE_MISMATCHED_IMAGE_TYPE, "IMAGE", str);
         }
-        if (srcImageEntry->second.samples == VK_SAMPLE_COUNT_1_BIT) {
+        if (srcImageEntry->samples == VK_SAMPLE_COUNT_1_BIT) {
             char const str[] = "vkCmdResolveImage called with source sample count less than 2.";
             skipCall |=
                 log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
                         (uint64_t)commandBuffer, __LINE__, IMAGE_INVALID_RESOLVE_SAMPLES, "IMAGE", str);
         }
-        if (dstImageEntry->second.samples != VK_SAMPLE_COUNT_1_BIT) {
+        if (dstImageEntry->samples != VK_SAMPLE_COUNT_1_BIT) {
             char const str[] = "vkCmdResolveImage called with dest sample count greater than 1.";
             skipCall |=
                 log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
@@ -1243,11 +1269,11 @@ GetImageSubresourceLayout(VkDevice device, VkImage image, const VkImageSubresour
     layer_data *device_data = get_my_data_ptr(get_dispatch_key(device), layer_data_map);
     VkFormat format;
 
-    auto imageEntry = device_data->imageMap.find(image);
+    auto imageEntry = getImageState(device_data, image);
 
     // Validate that image aspects match formats
-    if (imageEntry != device_data->imageMap.end()) {
-        format = imageEntry->second.format;
+    if (imageEntry) {
+        format = imageEntry->format;
         if (vk_format_is_color(format)) {
             if (pSubresource->aspectMask != VK_IMAGE_ASPECT_COLOR_BIT) {
                 std::stringstream ss;