Merge "Revert "Turn on vk unit tests"" into udc-dev

libs/hwui/Properties.cpp

@@ -17,7 +17,6 @@
 #include "Properties.h"
 
 #include "Debug.h"
-#include "log/log_main.h"
 #ifdef __ANDROID__
 #include "HWUIProperties.sysprop.h"
 #endif
@@ -220,12 +219,15 @@ RenderPipelineType Properties::getRenderPipelineType() {
     return sRenderPipelineType;
 }
 
-void Properties::overrideRenderPipelineType(RenderPipelineType type, bool inUnitTest) {
+void Properties::overrideRenderPipelineType(RenderPipelineType type) {
     // If we're doing actual rendering then we can't change the renderer after it's been set.
-    // Unit tests can freely change this as often as it wants.
+    // Unit tests can freely change this as often as it wants, though, as there's no actual
-    LOG_ALWAYS_FATAL_IF(sRenderPipelineType != RenderPipelineType::NotInitialized &&
+    // GL rendering happening
-                                sRenderPipelineType != type && !inUnitTest,
+    if (sRenderPipelineType != RenderPipelineType::NotInitialized) {
-                        "Trying to change pipeline but it's already set.");
+        LOG_ALWAYS_FATAL_IF(sRenderPipelineType != type,
+                            "Trying to change pipeline but it's already set");
+        return;
+    }
     sRenderPipelineType = type;
 }
 

libs/hwui/Properties.h

@@ -303,7 +303,7 @@ public:
     static bool enableRTAnimations;
 
     // Used for testing only to change the render pipeline.
-    static void overrideRenderPipelineType(RenderPipelineType, bool inUnitTest = false);
+    static void overrideRenderPipelineType(RenderPipelineType);
 
     static bool runningInEmulator;
 

libs/hwui/renderthread/VulkanManager.cpp

@@ -23,13 +23,11 @@
 #include <GrDirectContext.h>
 #include <GrTypes.h>
 #include <android/sync.h>
+#include <gui/TraceUtils.h>
 #include <ui/FatVector.h>
 #include <vk/GrVkExtensions.h>
 #include <vk/GrVkTypes.h>
 
-#include <cstring>
-
-#include <gui/TraceUtils.h>
 #include "Properties.h"
 #include "RenderThread.h"
 #include "pipeline/skia/ShaderCache.h"
@@ -90,19 +88,6 @@ static void free_features_extensions_structs(const VkPhysicalDeviceFeatures2& fe
     }
 }
 
-GrVkGetProc VulkanManager::sSkiaGetProp = [](const char* proc_name, VkInstance instance,
-                                             VkDevice device) {
-    if (device != VK_NULL_HANDLE) {
-        if (strcmp("vkQueueSubmit", proc_name) == 0) {
-            return (PFN_vkVoidFunction)VulkanManager::interceptedVkQueueSubmit;
-        } else if (strcmp("vkQueueWaitIdle", proc_name) == 0) {
-            return (PFN_vkVoidFunction)VulkanManager::interceptedVkQueueWaitIdle;
-        }
-        return vkGetDeviceProcAddr(device, proc_name);
-    }
-    return vkGetInstanceProcAddr(instance, proc_name);
-};
-
 #define GET_PROC(F) m##F = (PFN_vk##F)vkGetInstanceProcAddr(VK_NULL_HANDLE, "vk" #F)
 #define GET_INST_PROC(F) m##F = (PFN_vk##F)vkGetInstanceProcAddr(mInstance, "vk" #F)
 #define GET_DEV_PROC(F) m##F = (PFN_vk##F)vkGetDeviceProcAddr(mDevice, "vk" #F)
@@ -138,6 +123,7 @@ VulkanManager::~VulkanManager() {
     }
 
     mGraphicsQueue = VK_NULL_HANDLE;
+    mAHBUploadQueue = VK_NULL_HANDLE;
     mDevice = VK_NULL_HANDLE;
     mPhysicalDevice = VK_NULL_HANDLE;
     mInstance = VK_NULL_HANDLE;
@@ -231,7 +217,7 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
     mDriverVersion = physDeviceProperties.driverVersion;
 
     // query to get the initial queue props size
-    uint32_t queueCount;
+    uint32_t queueCount = 0;
     mGetPhysicalDeviceQueueFamilyProperties(mPhysicalDevice, &queueCount, nullptr);
     LOG_ALWAYS_FATAL_IF(!queueCount);
 
@@ -239,11 +225,14 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
     std::unique_ptr<VkQueueFamilyProperties[]> queueProps(new VkQueueFamilyProperties[queueCount]);
     mGetPhysicalDeviceQueueFamilyProperties(mPhysicalDevice, &queueCount, queueProps.get());
 
+    constexpr auto kRequestedQueueCount = 2;
+
     // iterate to find the graphics queue
     mGraphicsQueueIndex = queueCount;
     for (uint32_t i = 0; i < queueCount; i++) {
         if (queueProps[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) {
             mGraphicsQueueIndex = i;
+            LOG_ALWAYS_FATAL_IF(queueProps[i].queueCount < kRequestedQueueCount);
             break;
         }
     }
@@ -273,7 +262,14 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
         LOG_ALWAYS_FATAL_IF(!hasKHRSwapchainExtension);
     }
 
-    grExtensions.init(sSkiaGetProp, mInstance, mPhysicalDevice, mInstanceExtensions.size(),
+    auto getProc = [](const char* proc_name, VkInstance instance, VkDevice device) {
+        if (device != VK_NULL_HANDLE) {
+            return vkGetDeviceProcAddr(device, proc_name);
+        }
+        return vkGetInstanceProcAddr(instance, proc_name);
+    };
+
+    grExtensions.init(getProc, mInstance, mPhysicalDevice, mInstanceExtensions.size(),
                       mInstanceExtensions.data(), mDeviceExtensions.size(),
                       mDeviceExtensions.data());
 
@@ -312,7 +308,7 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
     // and we can't depend on it on all platforms
     features.features.robustBufferAccess = VK_FALSE;
 
-    float queuePriorities[1] = {0.0};
+    float queuePriorities[kRequestedQueueCount] = {0.0};
 
     void* queueNextPtr = nullptr;
 
@@ -345,7 +341,7 @@ void VulkanManager::setupDevice(GrVkExtensions& grExtensions, VkPhysicalDeviceFe
             queueNextPtr,                                // pNext
             0,                                           // VkDeviceQueueCreateFlags
             mGraphicsQueueIndex,                         // queueFamilyIndex
-            1,                                           // queueCount
+            kRequestedQueueCount,                        // queueCount
             queuePriorities,                             // pQueuePriorities
     };
 
@@ -403,6 +399,7 @@ void VulkanManager::initialize() {
     this->setupDevice(mExtensions, mPhysicalDeviceFeatures2);
 
     mGetDeviceQueue(mDevice, mGraphicsQueueIndex, 0, &mGraphicsQueue);
+    mGetDeviceQueue(mDevice, mGraphicsQueueIndex, 1, &mAHBUploadQueue);
 
     if (Properties::enablePartialUpdates && Properties::useBufferAge) {
         mSwapBehavior = SwapBehavior::BufferAge;
@@ -416,16 +413,24 @@ static void onGrContextReleased(void* context) {
 
 sk_sp<GrDirectContext> VulkanManager::createContext(GrContextOptions& options,
                                                     ContextType contextType) {
+    auto getProc = [](const char* proc_name, VkInstance instance, VkDevice device) {
+        if (device != VK_NULL_HANDLE) {
+            return vkGetDeviceProcAddr(device, proc_name);
+        }
+        return vkGetInstanceProcAddr(instance, proc_name);
+    };
+
     GrVkBackendContext backendContext;
     backendContext.fInstance = mInstance;
     backendContext.fPhysicalDevice = mPhysicalDevice;
     backendContext.fDevice = mDevice;
-    backendContext.fQueue = mGraphicsQueue;
+    backendContext.fQueue =
+            (contextType == ContextType::kRenderThread) ? mGraphicsQueue : mAHBUploadQueue;
     backendContext.fGraphicsQueueIndex = mGraphicsQueueIndex;
     backendContext.fMaxAPIVersion = mAPIVersion;
     backendContext.fVkExtensions = &mExtensions;
     backendContext.fDeviceFeatures2 = &mPhysicalDeviceFeatures2;
-    backendContext.fGetProc = sSkiaGetProp;
+    backendContext.fGetProc = std::move(getProc);
 
     LOG_ALWAYS_FATAL_IF(options.fContextDeleteProc != nullptr, "Conflicting fContextDeleteProcs!");
     this->incStrong((void*)onGrContextReleased);
@@ -636,8 +641,6 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
         ALOGE_IF(VK_SUCCESS != err, "VulkanManager::swapBuffers(): Failed to get semaphore Fd");
     } else {
         ALOGE("VulkanManager::swapBuffers(): Semaphore submission failed");
-
-        std::lock_guard<std::mutex> lock(mGraphicsQueueMutex);
         mQueueWaitIdle(mGraphicsQueue);
     }
     if (mDestroySemaphoreContext) {
@@ -652,7 +655,6 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect)
 void VulkanManager::destroySurface(VulkanSurface* surface) {
     // Make sure all submit commands have finished before starting to destroy objects.
     if (VK_NULL_HANDLE != mGraphicsQueue) {
-        std::lock_guard<std::mutex> lock(mGraphicsQueueMutex);
         mQueueWaitIdle(mGraphicsQueue);
     }
     mDeviceWaitIdle(mDevice);

libs/hwui/renderthread/VulkanManager.h

@@ -17,10 +17,6 @@
 #ifndef VULKANMANAGER_H
 #define VULKANMANAGER_H
 
-#include <functional>
-#include <mutex>
-
-#include "vulkan/vulkan_core.h"
 #if !defined(VK_USE_PLATFORM_ANDROID_KHR)
 #define VK_USE_PLATFORM_ANDROID_KHR
 #endif
@@ -186,25 +182,8 @@ private:
     VkDevice mDevice = VK_NULL_HANDLE;
 
     uint32_t mGraphicsQueueIndex;
-
-    std::mutex mGraphicsQueueMutex;
     VkQueue mGraphicsQueue = VK_NULL_HANDLE;
-
+    VkQueue mAHBUploadQueue = VK_NULL_HANDLE;
-    static VKAPI_ATTR VkResult interceptedVkQueueSubmit(VkQueue queue, uint32_t submitCount,
-                                                        const VkSubmitInfo* pSubmits,
-                                                        VkFence fence) {
-        sp<VulkanManager> manager = VulkanManager::getInstance();
-        std::lock_guard<std::mutex> lock(manager->mGraphicsQueueMutex);
-        return manager->mQueueSubmit(queue, submitCount, pSubmits, fence);
-    }
-
-    static VKAPI_ATTR VkResult interceptedVkQueueWaitIdle(VkQueue queue) {
-        sp<VulkanManager> manager = VulkanManager::getInstance();
-        std::lock_guard<std::mutex> lock(manager->mGraphicsQueueMutex);
-        return manager->mQueueWaitIdle(queue);
-    }
-
-    static GrVkGetProc sSkiaGetProp;
 
     // Variables saved to populate VkFunctorInitParams.
     static const uint32_t mAPIVersion = VK_MAKE_VERSION(1, 1, 0);

libs/hwui/tests/common/TestUtils.h

@@ -61,12 +61,12 @@ namespace uirenderer {
         ADD_FAILURE() << "ClipState not a rect";                                     \
     }
 
-#define INNER_PIPELINE_TEST(test_case_name, test_name, pipeline, functionCall)      \
+#define INNER_PIPELINE_TEST(test_case_name, test_name, pipeline, functionCall) \
-    TEST(test_case_name, test_name##_##pipeline) {                                  \
+    TEST(test_case_name, test_name##_##pipeline) {                             \
-        RenderPipelineType oldType = Properties::getRenderPipelineType();           \
+        RenderPipelineType oldType = Properties::getRenderPipelineType();      \
-        Properties::overrideRenderPipelineType(RenderPipelineType::pipeline, true); \
+        Properties::overrideRenderPipelineType(RenderPipelineType::pipeline);  \
-        functionCall;                                                               \
+        functionCall;                                                          \
-        Properties::overrideRenderPipelineType(oldType, true);                      \
+        Properties::overrideRenderPipelineType(oldType);                       \
     };
 
 #define INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, pipeline) \
@@ -78,27 +78,29 @@ namespace uirenderer {
  * Like gtest's TEST, but runs on the RenderThread, and 'renderThread' is passed, in top level scope
  * (for e.g. accessing its RenderState)
  */
-#define RENDERTHREAD_TEST(test_case_name, test_name)                         \
+#define RENDERTHREAD_TEST(test_case_name, test_name)                                        \
-    class test_case_name##_##test_name##_RenderThreadTest {                  \
+    class test_case_name##_##test_name##_RenderThreadTest {                                 \
-    public:                                                                  \
+    public:                                                                                 \
-        static void doTheThing(renderthread::RenderThread& renderThread);    \
+        static void doTheThing(renderthread::RenderThread& renderThread);                   \
-    };                                                                       \
+    };                                                                                      \
-    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaGL);     \
+    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaGL);                    \
-    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaVulkan); \
+    /* Temporarily disabling Vulkan until we can figure out a way to stub out the driver */ \
-    void test_case_name##_##test_name##_RenderThreadTest::doTheThing(        \
+    /* INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaVulkan); */          \
+    void test_case_name##_##test_name##_RenderThreadTest::doTheThing(                       \
             renderthread::RenderThread& renderThread)
 
 /**
  * Like RENDERTHREAD_TEST, but only runs with the Skia RenderPipelineTypes
  */
-#define RENDERTHREAD_SKIA_PIPELINE_TEST(test_case_name, test_name)           \
+#define RENDERTHREAD_SKIA_PIPELINE_TEST(test_case_name, test_name)                          \
-    class test_case_name##_##test_name##_RenderThreadTest {                  \
+    class test_case_name##_##test_name##_RenderThreadTest {                                 \
-    public:                                                                  \
+    public:                                                                                 \
-        static void doTheThing(renderthread::RenderThread& renderThread);    \
+        static void doTheThing(renderthread::RenderThread& renderThread);                   \
-    };                                                                       \
+    };                                                                                      \
-    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaGL);     \
+    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaGL);                    \
-    INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaVulkan); \
+    /* Temporarily disabling Vulkan until we can figure out a way to stub out the driver */ \
-    void test_case_name##_##test_name##_RenderThreadTest::doTheThing(        \
+    /* INNER_PIPELINE_RENDERTHREAD_TEST(test_case_name, test_name, SkiaVulkan); */          \
+    void test_case_name##_##test_name##_RenderThreadTest::doTheThing(                       \
             renderthread::RenderThread& renderThread)
 
 /**