Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 10fbabfb authored by Vishnu Nair's avatar Vishnu Nair
Browse files

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
parent 799de38b

TEST_MAPPING

+0 −4
Original line number Diff line number Diff line
@@ -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

+25 −17
Original line number Diff line number Diff line
@@ -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,9 +96,11 @@ public: 
        if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) {

            ASSERT_GE(latchTime, 0) << "bad latch time";

            ASSERT_NE(presentFence, nullptr);

            if (SyncFeatures::getInstance().useNativeFenceSync()) {

                if (mExpectedPresentTime >= 0) {

                    ASSERT_EQ(presentFence->wait(3000), NO_ERROR);

                ASSERT_GE(presentFence->getSignalTime(), mExpectedPresentTime - nsecs_t(5 * 1e6));

                    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(),
@@ -108,6 +111,7 @@ public: 
                    ASSERT_GE(presentFence->getSignalTime(),

                              mExpectedPresentTimeForVsyncId - 4'000'000);

                }

            }

        } else {

            ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented";

            ASSERT_EQ(latchTime, -1) << "unpresented transactions shouldn't be latched";
@@ -150,12 +154,16 @@ private: 
                }

            }



            // 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);

            }



            if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) {

                ASSERT_NE(previousReleaseFence, nullptr)

services/surfaceflinger/tests/BufferGeneratorShader.h

+3 −3
Original line number Diff line number Diff line
@@ -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

+9 −3
Original line number Diff line number Diff line
@@ -52,7 +52,10 @@ public: 
    }



    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 @@ public: 
                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));

    }

}