Skip to content

Commit 302cd56

Browse files
Treehugger RobotAndroid (Google) Code Review
authored andcommitted
Merge "Fix LayerCallback tests" into main
2 parents 6e578e5 + 10fbabf commit 302cd56

File tree

4 files changed

+37
-27
lines changed

4 files changed

+37
-27
lines changed

TEST_MAPPING

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@
2828
"name": "SurfaceFlinger_test",
2929
"keywords": [ "primary-device" ],
3030
"options": [
31-
// TODO(b/328119950) Known to be broken.
32-
{
33-
"exclude-filter": "LayerCallbackTest#SetNullBuffer"
34-
},
3531
// TODO(b/398306512) Flaky on real device.
3632
{
3733
"exclude-filter": "LayerRenderTypeTransactionTests/LayerRenderTypeTransactionTest#SetRelativeZBasic_BufferQueue/*"

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

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <gtest/gtest.h>
1919
#include <gui/SurfaceComposerClient.h>
2020
#include <gui/SurfaceControl.h>
21+
#include <private/gui/SyncFeatures.h>
2122
#include <ui/Fence.h>
2223
#include <utils/Timers.h>
2324
#include <chrono>
@@ -95,18 +96,21 @@ class ExpectedResult {
9596
if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) {
9697
ASSERT_GE(latchTime, 0) << "bad latch time";
9798
ASSERT_NE(presentFence, nullptr);
98-
if (mExpectedPresentTime >= 0) {
99-
ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
100-
ASSERT_GE(presentFence->getSignalTime(), mExpectedPresentTime - nsecs_t(5 * 1e6));
101-
// if the panel is running at 30 hz, at the worst case, our expected time just
102-
// misses vsync and we have to wait another 33.3ms
103-
ASSERT_LE(presentFence->getSignalTime(),
104-
mExpectedPresentTime + nsecs_t(66.666666 * 1e6));
105-
} else if (mExpectedPresentTimeForVsyncId >= 0) {
106-
ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
107-
// We give 4ms for prediction error
108-
ASSERT_GE(presentFence->getSignalTime(),
109-
mExpectedPresentTimeForVsyncId - 4'000'000);
99+
if (SyncFeatures::getInstance().useNativeFenceSync()) {
100+
if (mExpectedPresentTime >= 0) {
101+
ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
102+
ASSERT_GE(presentFence->getSignalTime(),
103+
mExpectedPresentTime - nsecs_t(5 * 1e6));
104+
// if the panel is running at 30 hz, at the worst case, our expected time just
105+
// misses vsync and we have to wait another 33.3ms
106+
ASSERT_LE(presentFence->getSignalTime(),
107+
mExpectedPresentTime + nsecs_t(66.666666 * 1e6));
108+
} else if (mExpectedPresentTimeForVsyncId >= 0) {
109+
ASSERT_EQ(presentFence->wait(3000), NO_ERROR);
110+
// We give 4ms for prediction error
111+
ASSERT_GE(presentFence->getSignalTime(),
112+
mExpectedPresentTimeForVsyncId - 4'000'000);
113+
}
110114
}
111115
} else {
112116
ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented";
@@ -150,12 +154,16 @@ class ExpectedResult {
150154
}
151155
}
152156

153-
if (mBufferResult == ExpectedResult::Buffer::ACQUIRED) {
154-
ASSERT_GT(acquireTime, 0) << "acquire time should be valid";
155-
} else {
156-
ASSERT_LE(acquireTime, 0) << "acquire time should not be valid";
157+
// If the device doesn't have native fence support, skip validating the acquire time.
158+
// Cuttlefish uses a fence with a signal time of -1 to indicate completion.
159+
if (SyncFeatures::getInstance().useNativeFenceSync()) {
160+
if (mBufferResult == ExpectedResult::Buffer::ACQUIRED) {
161+
ASSERT_GT(acquireTime, 0) << "acquire time should be valid";
162+
} else {
163+
ASSERT_LE(acquireTime, 0) << "acquire time should not be valid";
164+
}
165+
ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED);
157166
}
158-
ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED);
159167

160168
if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) {
161169
ASSERT_NE(previousReleaseFence, nullptr)

services/surfaceflinger/tests/BufferGeneratorShader.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ void main() {
3434
static const char* FRAGMENT_SHADER = R"SHADER__(#version 300 es
3535
precision highp float;
3636

37-
layout(location = 0) uniform vec4 resolution;
38-
layout(location = 1) uniform float time;
39-
layout(location = 2) uniform vec3[4] SPHERICAL_HARMONICS;
37+
uniform vec4 resolution;
38+
uniform float time;
39+
uniform vec3[4] SPHERICAL_HARMONICS;
4040

4141
layout(location = 0) out vec4 fragColor;
4242

services/surfaceflinger/tests/LayerCallback_test.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ class LayerCallbackTest : public LayerTransactionTest {
5252
}
5353

5454
virtual sp<SurfaceControl> createLayerWithBuffer() {
55-
return createLayer(mClient, "test", 0, 0, ISurfaceComposerClient::eFXSurfaceBufferState);
55+
const std::string test_name = std::string("Test layer for ") +
56+
::testing::UnitTest::GetInstance()->current_test_info()->name();
57+
return createLayer(mClient, test_name.c_str(), 0, 0,
58+
ISurfaceComposerClient::eFXSurfaceBufferState);
5659
}
5760

5861
static int fillBuffer(Transaction& transaction, const sp<SurfaceControl>& layer,
@@ -85,7 +88,6 @@ class LayerCallbackTest : public LayerTransactionTest {
8588
return err;
8689
}
8790
}
88-
8991
transaction.addTransactionCompletedCallback(callbackHelper->function,
9092
callbackHelper->getContext());
9193
return NO_ERROR;
@@ -1268,10 +1270,14 @@ TEST_F(LayerCallbackTest, SetNullBuffer) {
12681270
transaction.apply();
12691271

12701272
{
1273+
// TODO(b/294915480) Fix this as part of release buffer cleanup. We should not be passing a
1274+
// release fence in this case, because there is no buffer to release. We currently pass a
1275+
// release fence because of defensive code used to track screenshot work. Passing a fence
1276+
// here is odd but harmless.
12711277
ExpectedResult expected;
12721278
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
12731279
ExpectedResult::Buffer::ACQUIRED,
1274-
ExpectedResult::PreviousBuffer::NOT_RELEASED);
1280+
ExpectedResult::PreviousBuffer::RELEASED);
12751281
EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
12761282
}
12771283
}

0 commit comments

Comments
 (0)