Vulkan: clean up the swapchain implementation

Bits included in this change:
1. Refactor transform supports
2. Clean up error returns
3. Clean up redundant TODOs

Bug: 134185757
Test: CtsDeqpTestCases and CtsGraphicsTestCases
Change-Id: I113839bc294d08e0d5f3e1b6f70539f674605dcf
diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp
index a8949d3..340f48f 100644
--- a/vulkan/libvulkan/swapchain.cpp
+++ b/vulkan/libvulkan/swapchain.cpp
@@ -34,10 +34,6 @@
 
 using android::hardware::graphics::common::V1_0::BufferUsage;
 
-// TODO(jessehall): Currently we don't have a good error code for when a native
-// window operation fails. Just returning INITIALIZATION_FAILED for now. Later
-// versions (post SDK 0.9) of the API/extension have a better error code.
-// When updating to that version, audit all error returns.
 namespace vulkan {
 namespace driver {
 
@@ -48,29 +44,12 @@
     VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR |
     VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR |
     VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR |
-    // TODO(jessehall): See TODO in TranslateNativeToVulkanTransform.
-    // VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR |
-    // VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR |
-    // VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR |
-    // VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR |
+    VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR |
+    VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR |
+    VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR |
+    VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR |
     VK_SURFACE_TRANSFORM_INHERIT_BIT_KHR;
 
-int TranslateVulkanToNativeTransform(VkSurfaceTransformFlagBitsKHR transform) {
-    switch (transform) {
-        // TODO: See TODO in TranslateNativeToVulkanTransform
-        case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR:
-            return NATIVE_WINDOW_TRANSFORM_ROT_90;
-        case VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR:
-            return NATIVE_WINDOW_TRANSFORM_ROT_180;
-        case VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR:
-            return NATIVE_WINDOW_TRANSFORM_ROT_270;
-        case VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR:
-        case VK_SURFACE_TRANSFORM_INHERIT_BIT_KHR:
-        default:
-            return 0;
-    }
-}
-
 VkSurfaceTransformFlagBitsKHR TranslateNativeToVulkanTransform(int native) {
     // Native and Vulkan transforms are isomorphic, but are represented
     // differently. Vulkan transforms are built up of an optional horizontal
@@ -78,27 +57,22 @@
     // transforms are built up from a horizontal flip, vertical flip, and
     // 90-degree rotation, all optional but always in that order.
 
-    // TODO(jessehall): For now, only support pure rotations, not
-    // flip or flip-and-rotate, until I have more time to test them and build
-    // sample code. As far as I know we never actually use anything besides
-    // pure rotations anyway.
-
     switch (native) {
-        case 0:  // 0x0
+        case 0:
             return VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR;
-        // case NATIVE_WINDOW_TRANSFORM_FLIP_H:  // 0x1
-        //     return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR;
-        // case NATIVE_WINDOW_TRANSFORM_FLIP_V:  // 0x2
-        //     return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR;
-        case NATIVE_WINDOW_TRANSFORM_ROT_180:  // FLIP_H | FLIP_V
+        case NATIVE_WINDOW_TRANSFORM_FLIP_H:
+            return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR;
+        case NATIVE_WINDOW_TRANSFORM_FLIP_V:
+            return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR;
+        case NATIVE_WINDOW_TRANSFORM_ROT_180:
             return VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR;
-        case NATIVE_WINDOW_TRANSFORM_ROT_90:  // 0x4
+        case NATIVE_WINDOW_TRANSFORM_ROT_90:
             return VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR;
-        // case NATIVE_WINDOW_TRANSFORM_FLIP_H | NATIVE_WINDOW_TRANSFORM_ROT_90:
-        //     return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR;
-        // case NATIVE_WINDOW_TRANSFORM_FLIP_V | NATIVE_WINDOW_TRANSFORM_ROT_90:
-        //     return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR;
-        case NATIVE_WINDOW_TRANSFORM_ROT_270:  // FLIP_H | FLIP_V | ROT_90
+        case NATIVE_WINDOW_TRANSFORM_FLIP_H | NATIVE_WINDOW_TRANSFORM_ROT_90:
+            return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR;
+        case NATIVE_WINDOW_TRANSFORM_FLIP_V | NATIVE_WINDOW_TRANSFORM_ROT_90:
+            return VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR;
+        case NATIVE_WINDOW_TRANSFORM_ROT_270:
             return VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR;
         case NATIVE_WINDOW_TRANSFORM_INVERSE_DISPLAY:
         default:
@@ -106,6 +80,31 @@
     }
 }
 
+int TranslateVulkanToNativeTransform(VkSurfaceTransformFlagBitsKHR transform) {
+    switch (transform) {
+        case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_ROT_90;
+        case VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_ROT_180;
+        case VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_ROT_270;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_H;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_H |
+                   NATIVE_WINDOW_TRANSFORM_ROT_90;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_V;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_V |
+                   NATIVE_WINDOW_TRANSFORM_ROT_90;
+        case VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR:
+        case VK_SURFACE_TRANSFORM_INHERIT_BIT_KHR:
+        default:
+            return 0;
+    }
+}
+
 int InvertTransformToNative(VkSurfaceTransformFlagBitsKHR transform) {
     switch (transform) {
         case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR:
@@ -114,17 +113,16 @@
             return NATIVE_WINDOW_TRANSFORM_ROT_180;
         case VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR:
             return NATIVE_WINDOW_TRANSFORM_ROT_90;
-        // TODO(jessehall): See TODO in TranslateNativeToVulkanTransform.
-        // case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR:
-        //     return NATIVE_WINDOW_TRANSFORM_FLIP_H;
-        // case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR:
-        //     return NATIVE_WINDOW_TRANSFORM_FLIP_H |
-        //            NATIVE_WINDOW_TRANSFORM_ROT_90;
-        // case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR:
-        //     return NATIVE_WINDOW_TRANSFORM_FLIP_V;
-        // case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR:
-        //     return NATIVE_WINDOW_TRANSFORM_FLIP_V |
-        //            NATIVE_WINDOW_TRANSFORM_ROT_90;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_H;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_90_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_H |
+                   NATIVE_WINDOW_TRANSFORM_ROT_90;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_180_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_V;
+        case VK_SURFACE_TRANSFORM_HORIZONTAL_MIRROR_ROTATE_270_BIT_KHR:
+            return NATIVE_WINDOW_TRANSFORM_FLIP_V |
+                   NATIVE_WINDOW_TRANSFORM_ROT_90;
         case VK_SURFACE_TRANSFORM_IDENTITY_BIT_KHR:
         case VK_SURFACE_TRANSFORM_INHERIT_BIT_KHR:
         default:
@@ -201,8 +199,6 @@
             { NATIVE_WINDOW_TIMESTAMP_PENDING };
 };
 
-// ----------------------------------------------------------------------------
-
 struct Surface {
     android::sp<ANativeWindow> window;
     VkSwapchainKHR swapchain_handle;
@@ -539,15 +535,12 @@
               strerror(-err), err);
         surface->~Surface();
         allocator->pfnFree(allocator->pUserData, surface);
-        return VK_ERROR_INITIALIZATION_FAILED;
+        return VK_ERROR_SURFACE_LOST_KHR;
     }
 
-    // TODO(jessehall): Create and use NATIVE_WINDOW_API_VULKAN.
     err =
         native_window_api_connect(surface->window.get(), NATIVE_WINDOW_API_EGL);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_api_connect() failed: %s (%d)", strerror(-err),
               err);
         surface->~Surface();
@@ -656,7 +649,6 @@
         return VK_ERROR_SURFACE_LOST_KHR;
     }
 
-    // TODO(jessehall): Figure out what the min/max values should be.
     int max_buffer_count;
     err = window->query(window, NATIVE_WINDOW_MAX_BUFFER_COUNT, &max_buffer_count);
     if (err != 0) {
@@ -670,8 +662,7 @@
     capabilities->currentExtent =
         VkExtent2D{static_cast<uint32_t>(width), static_cast<uint32_t>(height)};
 
-    // TODO(jessehall): Figure out what the max extent should be. Maximum
-    // texture dimension maybe?
+    // TODO(http://b/134182502): Figure out what the max extent should be.
     capabilities->minImageExtent = VkExtent2D{1, 1};
     capabilities->maxImageExtent = VkExtent2D{4096, 4096};
 
@@ -685,11 +676,6 @@
     // associated with the bufferqueue. It can't be changed from here.
     capabilities->supportedCompositeAlpha = VK_COMPOSITE_ALPHA_INHERIT_BIT_KHR;
 
-    // TODO(jessehall): I think these are right, but haven't thought hard about
-    // it. Do we need to query the driver for support of any of these?
-    // Currently not included:
-    // - VK_IMAGE_USAGE_DEPTH_STENCIL_BIT: definitely not
-    // - VK_IMAGE_USAGE_TRANSIENT_ATTACHMENT_BIT: definitely not
     capabilities->supportedUsageFlags =
         VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT |
         VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_STORAGE_BIT |
@@ -729,8 +715,7 @@
     int err = native_window_get_wide_color_support(surface.window.get(),
                                                    &wide_color_support);
     if (err) {
-        // Not allowed to return a more sensible error code, so do this
-        return VK_ERROR_OUT_OF_HOST_MEMORY;
+        return VK_ERROR_SURFACE_LOST_KHR;
     }
     ALOGV("wide_color_support is: %d", wide_color_support);
     wide_color_support =
@@ -1052,6 +1037,8 @@
     // non-FREE state at any given time. Disconnecting and re-connecting
     // orphans the previous buffers, getting us back to the state where we can
     // dequeue all buffers.
+    //
+    // TODO(http://b/134186185) recycle swapchain images more efficiently
     err = native_window_api_disconnect(surface.window.get(),
                                        NATIVE_WINDOW_API_EGL);
     ALOGW_IF(err != 0, "native_window_api_disconnect failed: %s (%d)",
@@ -1072,8 +1059,6 @@
         create_info->presentMode == VK_PRESENT_MODE_MAILBOX_KHR ? 0 : 1;
     err = surface.window->setSwapInterval(surface.window.get(), swap_interval);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window->setSwapInterval(1) failed: %s (%d)",
               strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1100,8 +1085,6 @@
     err = native_window_set_buffers_format(surface.window.get(),
                                            native_pixel_format);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_buffers_format(%d) failed: %s (%d)",
               native_pixel_format, strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1109,8 +1092,6 @@
     err = native_window_set_buffers_data_space(surface.window.get(),
                                                native_dataspace);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_buffers_data_space(%d) failed: %s (%d)",
               native_dataspace, strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1120,8 +1101,6 @@
         surface.window.get(), static_cast<int>(create_info->imageExtent.width),
         static_cast<int>(create_info->imageExtent.height));
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_buffers_dimensions(%d,%d) failed: %s (%d)",
               create_info->imageExtent.width, create_info->imageExtent.height,
               strerror(-err), err);
@@ -1140,8 +1119,6 @@
         surface.window.get(),
         InvertTransformToNative(create_info->preTransform));
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_buffers_transform(%d) failed: %s (%d)",
               InvertTransformToNative(create_info->preTransform),
               strerror(-err), err);
@@ -1151,8 +1128,6 @@
     err = native_window_set_scaling_mode(
         surface.window.get(), NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_scaling_mode(SCALE_TO_WINDOW) failed: %s (%d)",
               strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1182,8 +1157,6 @@
                                 NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS,
                                 &query_value);
     if (err != 0 || query_value < 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("window->query failed: %s (%d) value=%d", strerror(-err), err,
               query_value);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1201,8 +1174,6 @@
     // can't actually use!).
     err = native_window_set_buffer_count(surface.window.get(), std::max(2u, num_images));
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_buffer_count(%d) failed: %s (%d)", num_images,
               strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
@@ -1242,8 +1213,6 @@
     }
     err = native_window_set_usage(surface.window.get(), native_usage);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("native_window_set_usage failed: %s (%d)", strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
     }
@@ -1301,8 +1270,6 @@
         err = surface.window->dequeueBuffer(surface.window.get(), &buffer,
                                             &img.dequeue_fence);
         if (err != 0) {
-            // TODO(jessehall): Improve error reporting. Can we enumerate
-            // possible errors and translate them to valid Vulkan result codes?
             ALOGE("dequeueBuffer[%u] failed: %s (%d)", i, strerror(-err), err);
             result = VK_ERROR_SURFACE_LOST_KHR;
             break;
@@ -1457,8 +1424,6 @@
     int fence_fd;
     err = window->dequeueBuffer(window, &buffer, &fence_fd);
     if (err != 0) {
-        // TODO(jessehall): Improve error reporting. Can we enumerate possible
-        // errors and translate them to valid Vulkan result codes?
         ALOGE("dequeueBuffer failed: %s (%d)", strerror(-err), err);
         return VK_ERROR_SURFACE_LOST_KHR;
     }
@@ -1513,8 +1478,6 @@
                               uint32_t* pImageIndex) {
     ATRACE_CALL();
 
-    // TODO: this should actually be the other way around and this function
-    // should handle any additional structures that get passed in
     return AcquireNextImageKHR(device, pAcquireInfo->swapchain,
                                pAcquireInfo->timeout, pAcquireInfo->semaphore,
                                pAcquireInfo->fence, pImageIndex);