Extract GPDIFP2 getProducerUsage path to seperate function

Extracting the GPDIFP2 logic to its own function makes it easier to
exit early from the GPDIFP2 path when we need to fall back to the
GetSwapchainGrallocUsage*ANDROID path.

This change also fixes an issue where we would exit getProducerUsage
early instead of falling back to the gralloc paths if GPDIFP2 would
return an error when called with an unsupported format.

Bug: 379230826
Test: atest libvulkan_test
Flag: EXEMPT bugfix
Change-Id: I42f1fe31e5e4a01c472a12854a1f4e989d2786dc

Author: tomnomgoogleuk

vulkan/libvulkan/Android.bp

@@ -22,6 +22,13 @@ package {
     default_applicable_licenses: ["frameworks_native_license"],
 }
 
+// Expose internal header files to test testing binary
+cc_library_headers {
+    name: "libvulkanprivate_headers-testing",
+    export_include_dirs: ["."],
+    visibility: ["//frameworks/native/vulkan/tests"],
+}
+
 ndk_library {
     name: "libvulkan",
     symbol_file: "libvulkan.map.txt",

vulkan/libvulkan/TEST_MAPPING

@@ -0,0 +1,7 @@
+{
+  "presubmit": [
+    {
+      "name": "libvulkan_test"
+    }
+  ]
+}

vulkan/libvulkan/swapchain.cpp

@@ -1413,115 +1413,138 @@ static void DestroySwapchainInternal(VkDevice device,
     allocator->pfnFree(allocator->pUserData, swapchain);
 }
 
-static VkResult getProducerUsage(const VkDevice& device,
-                                 const VkSwapchainCreateInfoKHR* create_info,
-                                 const VkSwapchainImageUsageFlagsANDROID swapchain_image_usage,
-                                 bool create_protected_swapchain,
-                                 uint64_t* producer_usage) {
-    // Get the physical device to query the appropriate producer usage
-    const VkPhysicalDevice& pdev = GetData(device).driver_physical_device;
-    const InstanceData& instance_data = GetData(pdev);
-    const InstanceDriverTable& instance_dispatch = instance_data.driver;
-    if (instance_dispatch.GetPhysicalDeviceImageFormatProperties2 ||
-            instance_dispatch.GetPhysicalDeviceImageFormatProperties2KHR) {
-        // Look through the create_info pNext chain passed to createSwapchainKHR
-        // for an image compression control struct.
-        // if one is found AND the appropriate extensions are enabled, create a
-        // VkImageCompressionControlEXT structure to pass on to
-        // GetPhysicalDeviceImageFormatProperties2
-        void* compression_control_pNext = nullptr;
-        VkImageCompressionControlEXT image_compression = {};
-        const VkSwapchainCreateInfoKHR* create_infos = create_info;
-        while (create_infos->pNext) {
-            create_infos = reinterpret_cast<const VkSwapchainCreateInfoKHR*>(create_infos->pNext);
-            switch (create_infos->sType) {
-                case VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT: {
-                    const VkImageCompressionControlEXT* compression_infos =
-                        reinterpret_cast<const VkImageCompressionControlEXT*>(create_infos);
-                    image_compression = *compression_infos;
-                    image_compression.pNext = nullptr;
-                    compression_control_pNext = &image_compression;
-                } break;
-                default:
-                    // Ignore all other info structs
-                    break;
-            }
+static VkResult getProducerUsageGPDIFP2(
+    const VkPhysicalDevice& pdev,
+    const VkSwapchainCreateInfoKHR* create_info,
+    const VkSwapchainImageUsageFlagsANDROID swapchain_image_usage,
+    bool create_protected_swapchain,
+    uint64_t* producer_usage) {
+    // Look through the create_info pNext chain passed to createSwapchainKHR
+    // for an image compression control struct.
+    // if one is found AND the appropriate extensions are enabled, create a
+    // VkImageCompressionControlEXT structure to pass on to
+    // GetPhysicalDeviceImageFormatProperties2
+    void* compression_control_pNext = nullptr;
+    VkImageCompressionControlEXT image_compression = {};
+    const VkSwapchainCreateInfoKHR* create_infos = create_info;
+    while (create_infos->pNext) {
+        create_infos = reinterpret_cast<const VkSwapchainCreateInfoKHR*>(
+            create_infos->pNext);
+        switch (create_infos->sType) {
+            case VK_STRUCTURE_TYPE_IMAGE_COMPRESSION_CONTROL_EXT: {
+                const VkImageCompressionControlEXT* compression_infos =
+                    reinterpret_cast<const VkImageCompressionControlEXT*>(
+                        create_infos);
+                image_compression = *compression_infos;
+                image_compression.pNext = nullptr;
+                compression_control_pNext = &image_compression;
+            } break;
+            default:
+                // Ignore all other info structs
+                break;
         }
+    }
 
-        // call GetPhysicalDeviceImageFormatProperties2KHR
-        VkPhysicalDeviceExternalImageFormatInfo external_image_format_info = {
-            .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTERNAL_IMAGE_FORMAT_INFO,
-            .pNext = compression_control_pNext,
-            .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
-        };
+    // call GetPhysicalDeviceImageFormatProperties2KHR
+    VkPhysicalDeviceExternalImageFormatInfo external_image_format_info = {
+        .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTERNAL_IMAGE_FORMAT_INFO,
+        .pNext = compression_control_pNext,
+        .handleType =
+            VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
+    };
 
-        // AHB does not have an sRGB format so we can't pass it to GPDIFP
-        // We need to convert the format to unorm if it is srgb
-        VkFormat format = create_info->imageFormat;
-        if (format == VK_FORMAT_R8G8B8A8_SRGB) {
-            format = VK_FORMAT_R8G8B8A8_UNORM;
-        }
+    // AHB does not have an sRGB format so we can't pass it to GPDIFP
+    // We need to convert the format to unorm if it is srgb
+    VkFormat format = create_info->imageFormat;
+    if (format == VK_FORMAT_R8G8B8A8_SRGB) {
+        format = VK_FORMAT_R8G8B8A8_UNORM;
+    }
 
-        VkPhysicalDeviceImageFormatInfo2 image_format_info = {
-            .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_FORMAT_INFO_2,
-            .pNext = &external_image_format_info,
-            .format = format,
-            .type = VK_IMAGE_TYPE_2D,
-            .tiling = VK_IMAGE_TILING_OPTIMAL,
-            .usage = create_info->imageUsage,
-            .flags = create_protected_swapchain ? VK_IMAGE_CREATE_PROTECTED_BIT : 0u,
-        };
+    VkPhysicalDeviceImageFormatInfo2 image_format_info = {
+        .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_FORMAT_INFO_2,
+        .pNext = &external_image_format_info,
+        .format = format,
+        .type = VK_IMAGE_TYPE_2D,
+        .tiling = VK_IMAGE_TILING_OPTIMAL,
+        .usage = create_info->imageUsage,
+        .flags =
+            create_protected_swapchain ? VK_IMAGE_CREATE_PROTECTED_BIT : 0u,
+    };
 
-        // If supporting mutable format swapchain add the mutable format flag
-        if (create_info->flags & VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR) {
-            image_format_info.flags |= VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
-            image_format_info.flags |= VK_IMAGE_CREATE_EXTENDED_USAGE_BIT_KHR;
-        }
+    // If supporting mutable format swapchain add the mutable format flag
+    if (create_info->flags & VK_SWAPCHAIN_CREATE_MUTABLE_FORMAT_BIT_KHR) {
+        image_format_info.flags |= VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
+        image_format_info.flags |= VK_IMAGE_CREATE_EXTENDED_USAGE_BIT_KHR;
+    }
 
-        VkAndroidHardwareBufferUsageANDROID ahb_usage;
-        ahb_usage.sType = VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_USAGE_ANDROID;
-        ahb_usage.pNext = nullptr;
+    VkAndroidHardwareBufferUsageANDROID ahb_usage;
+    ahb_usage.sType = VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_USAGE_ANDROID;
+    ahb_usage.pNext = nullptr;
 
-        VkImageFormatProperties2 image_format_properties;
-        image_format_properties.sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_PROPERTIES_2;
-        image_format_properties.pNext = &ahb_usage;
+    VkImageFormatProperties2 image_format_properties;
+    image_format_properties.sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_PROPERTIES_2;
+    image_format_properties.pNext = &ahb_usage;
 
-        VkResult result = GetPhysicalDeviceImageFormatProperties2(
-            pdev, &image_format_info, &image_format_properties);
-        if (result != VK_SUCCESS) {
-            ALOGE(
-                "VkGetPhysicalDeviceImageFormatProperties2 for AHB usage "
-                "failed: %d",
-                result);
-            return VK_ERROR_SURFACE_LOST_KHR;
-        }
+    VkResult result = GetPhysicalDeviceImageFormatProperties2(
+        pdev, &image_format_info, &image_format_properties);
+    if (result != VK_SUCCESS) {
+        ALOGE(
+            "VkGetPhysicalDeviceImageFormatProperties2 for AHB usage "
+            "failed: %d",
+            result);
+        return VK_ERROR_SURFACE_LOST_KHR;
+    }
+    // Determine if USAGE_FRONT_BUFFER is needed.
+    // GPDIFP2 has no means of using VkSwapchainImageUsageFlagsANDROID when
+    // querying for producer_usage. So androidHardwareBufferUsage will not
+    // contain USAGE_FRONT_BUFFER. We need to manually check for usage here.
+    if (!(swapchain_image_usage &
+          VK_SWAPCHAIN_IMAGE_USAGE_SHARED_BIT_ANDROID)) {
+        *producer_usage = ahb_usage.androidHardwareBufferUsage;
+        return VK_SUCCESS;
+    }
 
-        // Determine if USAGE_FRONT_BUFFER is needed.
-        // GPDIFP2 has no means of using VkSwapchainImageUsageFlagsANDROID when
-        // querying for producer_usage. So androidHardwareBufferUsage will not
-        // contain USAGE_FRONT_BUFFER. We need to manually check for usage here.
-        if (!(swapchain_image_usage & VK_SWAPCHAIN_IMAGE_USAGE_SHARED_BIT_ANDROID)) {
-            *producer_usage = ahb_usage.androidHardwareBufferUsage;
-            return VK_SUCCESS;
-        }
+    // Check if USAGE_FRONT_BUFFER is supported for this swapchain
+    AHardwareBuffer_Desc ahb_desc = {
+        .width = create_info->imageExtent.width,
+        .height = create_info->imageExtent.height,
+        .layers = create_info->imageArrayLayers,
+        .format = create_info->imageFormat,
+        .usage = ahb_usage.androidHardwareBufferUsage |
+                 AHARDWAREBUFFER_USAGE_FRONT_BUFFER,
+        .stride = 0,  // stride is always ignored when calling isSupported()
+    };
 
-        // Check if USAGE_FRONT_BUFFER is supported for this swapchain
-        AHardwareBuffer_Desc ahb_desc = {
-            .width = create_info->imageExtent.width,
-            .height = create_info->imageExtent.height,
-            .layers = create_info->imageArrayLayers,
-            .format = create_info->imageFormat,
-            .usage = ahb_usage.androidHardwareBufferUsage | AHARDWAREBUFFER_USAGE_FRONT_BUFFER,
-            .stride = 0, // stride is always ignored when calling isSupported()
-        };
+    // If FRONT_BUFFER is not supported in the GPDIFP2 path
+    // then we need to fallback to GetSwapchainGrallocUsageXAndroid
+    if (AHardwareBuffer_isSupported(&ahb_desc)) {
+        *producer_usage = ahb_usage.androidHardwareBufferUsage;
+        *producer_usage |= AHARDWAREBUFFER_USAGE_FRONT_BUFFER;
+        return VK_SUCCESS;
+    }
+
+    return VK_ERROR_FORMAT_NOT_SUPPORTED;
+}
+
+static VkResult getProducerUsage(const VkDevice& device,
+                                 const VkSwapchainCreateInfoKHR* create_info,
+                                 const VkSwapchainImageUsageFlagsANDROID swapchain_image_usage,
+                                 bool create_protected_swapchain,
+                                 uint64_t* producer_usage) {
+    // Get the physical device to query the appropriate producer usage
+    const VkPhysicalDevice& pdev = GetData(device).driver_physical_device;
+    const InstanceData& instance_data = GetData(pdev);
+    const InstanceDriverTable& instance_dispatch = instance_data.driver;
 
-        // If FRONT_BUFFER is not supported,
-        // then we need to call GetSwapchainGrallocUsageXAndroid below
-        if (AHardwareBuffer_isSupported(&ahb_desc)) {
-            *producer_usage = ahb_usage.androidHardwareBufferUsage;
-            *producer_usage |= AHARDWAREBUFFER_USAGE_FRONT_BUFFER;
+    if (instance_dispatch.GetPhysicalDeviceImageFormatProperties2 ||
+            instance_dispatch.GetPhysicalDeviceImageFormatProperties2KHR) {
+        VkResult result =
+            getProducerUsageGPDIFP2(pdev, create_info, swapchain_image_usage,
+                                    create_protected_swapchain, producer_usage);
+        if (result == VK_SUCCESS) {
             return VK_SUCCESS;
         }
+        // Fall through to gralloc path on error
     }
 
     uint64_t native_usage = 0;

vulkan/tests/Android.bp

@@ -27,6 +27,7 @@ cc_test {
 
     header_libs: [
         "hwvulkan_headers",
+        "libvulkanprivate_headers-testing",
         "vulkan_headers",
     ],
 

vulkan/tests/libvulkan_test.cpp

@@ -15,6 +15,7 @@
  */
 
 #include <android/log.h>
+#include <driver.h>
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <media/NdkImageReader.h>
@@ -29,6 +30,8 @@
 
 namespace android {
 
+namespace libvulkantest {
+
 class AImageReaderVulkanSwapchainTest : public ::testing::Test {
    public:
     AImageReaderVulkanSwapchainTest() {}
@@ -271,17 +274,20 @@ class AImageReaderVulkanSwapchainTest : public ::testing::Test {
 
         VkResult res =
             vkCreateSwapchainKHR(mDevice, &swapchainInfo, nullptr, &mSwapchain);
-        VK_CHECK(res);
-        LOGI("Swapchain created successfully");
+        if (res == VK_SUCCESS) {
+            LOGI("Swapchain created successfully");
 
-        uint32_t swapchainImageCount = 0;
-        vkGetSwapchainImagesKHR(mDevice, mSwapchain, &swapchainImageCount,
-                                nullptr);
-        std::vector<VkImage> swapchainImages(swapchainImageCount);
-        vkGetSwapchainImagesKHR(mDevice, mSwapchain, &swapchainImageCount,
-                                swapchainImages.data());
+            uint32_t swapchainImageCount = 0;
+            vkGetSwapchainImagesKHR(mDevice, mSwapchain, &swapchainImageCount,
+                                    nullptr);
+            std::vector<VkImage> swapchainImages(swapchainImageCount);
+            vkGetSwapchainImagesKHR(mDevice, mSwapchain, &swapchainImageCount,
+                                    swapchainImages.data());
 
-        LOGI("Swapchain has %u images", swapchainImageCount);
+            LOGI("Swapchain has %u images", swapchainImageCount);
+        } else {
+            LOGI("Swapchain creation failed");
+        }
     }
 
     // Image available callback (AImageReader)
@@ -357,4 +363,79 @@ TEST_F(AImageReaderVulkanSwapchainTest, TestHelperMethods) {
     cleanUpSwapchainForTest();
 }
 
+// Passing state in these tests requires global state. Wrap each test in an
+// anonymous namespace to prevent conflicting names.
+namespace {
+
+VKAPI_ATTR VkResult VKAPI_CALL hookedGetPhysicalDeviceImageFormatProperties2KHR(
+    VkPhysicalDevice,
+    const VkPhysicalDeviceImageFormatInfo2*,
+    VkImageFormatProperties2*) {
+    return VK_ERROR_SURFACE_LOST_KHR;
+}
+
+static PFN_vkGetSwapchainGrallocUsage2ANDROID
+    pfnNextGetSwapchainGrallocUsage2ANDROID = nullptr;
+
+static bool g_grallocCalled = false;
+
+VKAPI_ATTR VkResult VKAPI_CALL hookGetSwapchainGrallocUsage2ANDROID(
+    VkDevice device,
+    VkFormat format,
+    VkImageUsageFlags imageUsage,
+    VkSwapchainImageUsageFlagsANDROID swapchainImageUsage,
+    uint64_t* grallocConsumerUsage,
+    uint64_t* grallocProducerUsage) {
+    g_grallocCalled = true;
+    if (pfnNextGetSwapchainGrallocUsage2ANDROID) {
+        return pfnNextGetSwapchainGrallocUsage2ANDROID(
+            device, format, imageUsage, swapchainImageUsage,
+            grallocConsumerUsage, grallocProducerUsage);
+    }
+
+    return VK_ERROR_INITIALIZATION_FAILED;
+}
+
+TEST_F(AImageReaderVulkanSwapchainTest, getProducerUsageFallbackTest1) {
+    // BUG: 379230826
+    // Verify that getProducerUsage falls back to
+    // GetSwapchainGrallocUsage*ANDROID if GPDIFP2 fails
+    std::vector<const char*> instanceLayers = {};
+    std::vector<const char*> deviceLayers = {};
+    createVulkanInstance(instanceLayers);
+
+    createAImageReader(640, 480, AIMAGE_FORMAT_PRIVATE, 3);
+    getANativeWindowFromReader();
+    createVulkanSurface();
+    pickPhysicalDeviceAndQueueFamily();
+
+    createDeviceAndGetQueue(deviceLayers);
+    auto& pdev = vulkan::driver::GetData(mDevice).driver_physical_device;
+    auto& pdevDispatchTable = vulkan::driver::GetData(pdev).driver;
+    auto& deviceDispatchTable = vulkan::driver::GetData(mDevice).driver;
+
+    ASSERT_NE(deviceDispatchTable.GetSwapchainGrallocUsage2ANDROID, nullptr);
+
+    pdevDispatchTable.GetPhysicalDeviceImageFormatProperties2 =
+        hookedGetPhysicalDeviceImageFormatProperties2KHR;
+    deviceDispatchTable.GetSwapchainGrallocUsage2ANDROID =
+        hookGetSwapchainGrallocUsage2ANDROID;
+
+    ASSERT_FALSE(g_grallocCalled);
+
+    createSwapchain();
+
+    ASSERT_TRUE(g_grallocCalled);
+
+    ASSERT_NE(mVkInstance, (VkInstance)VK_NULL_HANDLE);
+    ASSERT_NE(mPhysicalDev, (VkPhysicalDevice)VK_NULL_HANDLE);
+    ASSERT_NE(mDevice, (VkDevice)VK_NULL_HANDLE);
+    ASSERT_NE(mSurface, (VkSurfaceKHR)VK_NULL_HANDLE);
+    cleanUpSwapchainForTest();
+}
+
+}  // namespace
+
+}  // namespace libvulkantest
+
 }  // namespace android