Fix LayerCallback tests

On cuttlefish the tests were skipped because of a shader compilation
error. This masked real failues in the tests because Cuttlefish does not
support native android fences, and results in a signal time of -1.

This cl fixes the shader compilation error, and skips fence signal time
checks if the device does not support native fences.

Fixes: 328119950
Test: atest LayerCallbackTest
Flag: EXEMPT tests

Change-Id: I7b85844f240e8c3795c794426b87b201ab099c68

Author: nairvis-beep

TEST_MAPPING

@@ -28,10 +28,6 @@
       "name": "SurfaceFlinger_test",
       "keywords": [ "primary-device" ],
       "options": [
-	// TODO(b/328119950) Known to be broken.
-        {
-          "exclude-filter": "LayerCallbackTest#SetNullBuffer"
-        },
 	// TODO(b/398306512) Flaky on real device.
         {
           "exclude-filter": "LayerRenderTypeTransactionTests/LayerRenderTypeTransactionTest#SetRelativeZBasic_BufferQueue/*"

libs/gui/include/gui/test/CallbackUtils.h

@@ -18,6 +18,7 @@
 #include <gtest/gtest.h>
 #include <gui/SurfaceComposerClient.h>
 #include <gui/SurfaceControl.h>
+#include <private/gui/SyncFeatures.h>
 #include <ui/Fence.h>
 #include <utils/Timers.h>
 #include <chrono>
@@ -95,18 +96,21 @@ class ExpectedResult {
         if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) {
             ASSERT_GE(latchTime, 0) << "bad latch time";
             ASSERT_NE(presentFence, nullptr);
-            if (mExpectedPresentTime >= 0) {
-                ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
-                ASSERT_GE(presentFence->getSignalTime(), mExpectedPresentTime - nsecs_t(5 * 1e6));
-                // if the panel is running at 30 hz, at the worst case, our expected time just
-                // misses vsync and we have to wait another 33.3ms
-                ASSERT_LE(presentFence->getSignalTime(),
-                          mExpectedPresentTime + nsecs_t(66.666666 * 1e6));
-            } else if (mExpectedPresentTimeForVsyncId >= 0) {
-                ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
-                // We give 4ms for prediction error
-                ASSERT_GE(presentFence->getSignalTime(),
-                          mExpectedPresentTimeForVsyncId - 4'000'000);
+            if (SyncFeatures::getInstance().useNativeFenceSync()) {
+                if (mExpectedPresentTime >= 0) {
+                    ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
+                    ASSERT_GE(presentFence->getSignalTime(),
+                            mExpectedPresentTime - nsecs_t(5 * 1e6));
+                    // if the panel is running at 30 hz, at the worst case, our expected time just
+                    // misses vsync and we have to wait another 33.3ms
+                    ASSERT_LE(presentFence->getSignalTime(),
+                              mExpectedPresentTime + nsecs_t(66.666666 * 1e6));
+                } else if (mExpectedPresentTimeForVsyncId >= 0) {
+                    ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
+                    // We give 4ms for prediction error
+                    ASSERT_GE(presentFence->getSignalTime(),
+                              mExpectedPresentTimeForVsyncId - 4'000'000);
+                }
             }
         } else {
             ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented";
@@ -150,12 +154,16 @@ class ExpectedResult {
                 }
             }
 
-            if (mBufferResult == ExpectedResult::Buffer::ACQUIRED) {
-                ASSERT_GT(acquireTime, 0) << "acquire time should be valid";
-            } else {
-                ASSERT_LE(acquireTime, 0) << "acquire time should not be valid";
+            // If the device doesn't have native fence support, skip validating the acquire time.
+            // Cuttlefish uses a fence with a signal time of -1 to indicate completion.
+            if (SyncFeatures::getInstance().useNativeFenceSync()) {
+                if (mBufferResult == ExpectedResult::Buffer::ACQUIRED) {
+                    ASSERT_GT(acquireTime, 0) << "acquire time should be valid";
+                } else {
+                    ASSERT_LE(acquireTime, 0) << "acquire time should not be valid";
+                }
+                ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED);
             }
-            ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED);
 
             if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) {
                 ASSERT_NE(previousReleaseFence, nullptr)

services/surfaceflinger/tests/BufferGeneratorShader.h

@@ -34,9 +34,9 @@ void main() {
 static const char* FRAGMENT_SHADER = R"SHADER__(#version 300 es
 precision highp float;
 
-layout(location = 0) uniform vec4 resolution;
-layout(location = 1) uniform float time;
-layout(location = 2) uniform vec3[4] SPHERICAL_HARMONICS;
+uniform vec4 resolution;
+uniform float time;
+uniform vec3[4] SPHERICAL_HARMONICS;
 
 layout(location = 0) out vec4 fragColor;
 

services/surfaceflinger/tests/LayerCallback_test.cpp

@@ -52,7 +52,10 @@ class LayerCallbackTest : public LayerTransactionTest {
     }
 
     virtual sp<SurfaceControl> createLayerWithBuffer() {
-        return createLayer(mClient, "test", 0, 0, ISurfaceComposerClient::eFXSurfaceBufferState);
+        const std::string test_name = std::string("Test layer for ") +
+                ::testing::UnitTest::GetInstance()->current_test_info()->name();
+        return createLayer(mClient, test_name.c_str(), 0, 0,
+                           ISurfaceComposerClient::eFXSurfaceBufferState);
     }
 
     static int fillBuffer(Transaction& transaction, const sp<SurfaceControl>& layer,
@@ -85,7 +88,6 @@ class LayerCallbackTest : public LayerTransactionTest {
                 return err;
             }
         }
-
         transaction.addTransactionCompletedCallback(callbackHelper->function,
                                                     callbackHelper->getContext());
         return NO_ERROR;
@@ -1268,10 +1270,14 @@ TEST_F(LayerCallbackTest, SetNullBuffer) {
     transaction.apply();
 
     {
+        // TODO(b/294915480) Fix this as part of release buffer cleanup. We should not be passing a
+        // release fence in this case, because there is no buffer to release. We currently pass a
+        // release fence because of defensive code used to track screenshot work. Passing a fence
+        // here is odd but harmless.
         ExpectedResult expected;
         expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                             ExpectedResult::Buffer::ACQUIRED,
-                            ExpectedResult::PreviousBuffer::NOT_RELEASED);
+                            ExpectedResult::PreviousBuffer::RELEASED);
         EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
     }
 }