Merge "Extract GPDIFP2 getProducerUsage path to seperate function" into main

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