Merge changes from topic "surfaceflinger-arc"
* changes:
SF: Cleanups to use std::atomic/std::mutex
SF: Allow SurfaceFlinger creation to be altered
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index 744bdba..4f12d3e 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -4,6 +4,7 @@
"-DLOG_TAG=\"SurfaceFlinger\"",
"-Wall",
"-Werror",
+ "-Wformat",
"-Wthread-safety",
"-Wunused",
"-Wunreachable-code",
@@ -18,6 +19,10 @@
"-DGL_GLEXT_PROTOTYPES",
"-DEGL_EGLEXT_PROTOTYPES",
],
+ include_dirs: [
+ "frameworks/native/vulkan/vkjson",
+ "frameworks/native/vulkan/include",
+ ],
shared_libs: [
"android.frameworks.vr.composer@1.0",
"android.hardware.configstore-utils",
@@ -84,6 +89,18 @@
],
}
+cc_defaults {
+ name: "libsurfaceflinger_production_defaults",
+ defaults: ["libsurfaceflinger_defaults"],
+ cflags: [
+ "-fvisibility=hidden",
+ "-fwhole-program-vtables", // requires ThinLTO
+ ],
+ lto: {
+ thin: true,
+ },
+}
+
cc_library_headers {
name: "libsurfaceflinger_headers",
export_include_dirs: ["."],
@@ -137,33 +154,23 @@
}
cc_library_shared {
+ // Please use libsurfaceflinger_defaults to configure how the sources are
+ // built, so the same settings can be used elsewhere.
name: "libsurfaceflinger",
- defaults: ["libsurfaceflinger_defaults"],
- cflags: [
- "-fvisibility=hidden",
- "-Werror=format",
- ],
+ defaults: ["libsurfaceflinger_production_defaults"],
srcs: [
":libsurfaceflinger_sources",
+
+ // Note: SurfaceFlingerFactory is not in the default sources so that it
+ // can be easily replaced.
+ "SurfaceFlingerFactory.cpp",
],
logtags: ["EventLog/EventLogTags.logtags"],
- include_dirs: [
- "frameworks/native/vulkan/vkjson",
- "frameworks/native/vulkan/include",
- ],
- cppflags: [
- "-fwhole-program-vtables", // requires ThinLTO
- ],
- lto: {
- thin: true,
- },
}
-cc_binary {
- name: "surfaceflinger",
+cc_defaults {
+ name: "libsurfaceflinger_binary",
defaults: ["surfaceflinger_defaults"],
- init_rc: ["surfaceflinger.rc"],
- srcs: ["main_surfaceflinger.cpp"],
whole_static_libs: [
"libsigchain",
],
@@ -180,7 +187,6 @@
"libhidltransport",
"liblayers_proto",
"liblog",
- "libsurfaceflinger",
"libtimestats_proto",
"libutils",
],
@@ -189,19 +195,19 @@
"libtrace_proto",
],
ldflags: ["-Wl,--export-dynamic"],
+}
- // TODO(b/71715793): These version-scripts are required due to the use of
- // whole_static_libs to pull in libsigchain. To work, the files had to be
- // locally duplicated from their original location
- // $ANDROID_ROOT/art/sigchainlib/
- multilib: {
- lib32: {
- version_script: "version-script32.txt",
- },
- lib64: {
- version_script: "version-script64.txt",
- },
- },
+filegroup {
+ name: "surfaceflinger_binary_sources",
+ srcs: ["main_surfaceflinger.cpp"],
+}
+
+cc_binary {
+ name: "surfaceflinger",
+ defaults: ["libsurfaceflinger_binary"],
+ init_rc: ["surfaceflinger.rc"],
+ srcs: [":surfaceflinger_binary_sources"],
+ shared_libs: ["libsurfaceflinger"],
}
cc_library_shared {
diff --git a/services/surfaceflinger/Barrier.h b/services/surfaceflinger/Barrier.h
index 3e9d443..97028a8 100644
--- a/services/surfaceflinger/Barrier.h
+++ b/services/surfaceflinger/Barrier.h
@@ -18,46 +18,40 @@
#define ANDROID_BARRIER_H
#include <stdint.h>
-#include <sys/types.h>
-#include <utils/threads.h>
+#include <condition_variable>
+#include <mutex>
namespace android {
class Barrier
{
public:
- inline Barrier() : state(CLOSED) { }
- inline ~Barrier() { }
-
// Release any threads waiting at the Barrier.
// Provides release semantics: preceding loads and stores will be visible
// to other threads before they wake up.
void open() {
- Mutex::Autolock _l(lock);
- state = OPENED;
- cv.broadcast();
+ std::lock_guard<std::mutex> lock(mMutex);
+ mIsOpen = true;
+ mCondition.notify_all();
}
// Reset the Barrier, so wait() will block until open() has been called.
void close() {
- Mutex::Autolock _l(lock);
- state = CLOSED;
+ std::lock_guard<std::mutex> lock(mMutex);
+ mIsOpen = false;
}
// Wait until the Barrier is OPEN.
// Provides acquire semantics: no subsequent loads or stores will occur
// until wait() returns.
void wait() const {
- Mutex::Autolock _l(lock);
- while (state == CLOSED) {
- cv.wait(lock);
- }
+ std::unique_lock<std::mutex> lock(mMutex);
+ mCondition.wait(lock, [this]() NO_THREAD_SAFETY_ANALYSIS { return mIsOpen; });
}
private:
- enum { OPENED, CLOSED };
- mutable Mutex lock;
- mutable Condition cv;
- volatile int state;
+ mutable std::mutex mMutex;
+ mutable std::condition_variable mCondition;
+ int mIsOpen GUARDED_BY(mMutex){false};
};
}; // namespace android
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 4839bfb..521cdbf 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -207,8 +207,9 @@
}
std::optional<Region> BufferQueueLayer::latchSidebandStream(bool& recomputeVisibleRegions) {
- if (android_atomic_acquire_cas(true, false, &mSidebandStreamChanged) == 0) {
- // mSidebandStreamChanged was true
+ bool sidebandStreamChanged = true;
+ if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, false)) {
+ // mSidebandStreamChanged was changed to false
// replicated in LayerBE until FE/BE is ready to be synchronized
getBE().compositionInfo.hwc.sidebandStream = mConsumer->getSidebandStream();
if (getBE().compositionInfo.hwc.sidebandStream != nullptr) {
@@ -259,7 +260,7 @@
Mutex::Autolock lock(mQueueItemLock);
mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
mQueueItems.removeAt(0);
- android_atomic_dec(&mQueuedFrames);
+ mQueuedFrames--;
}
return BAD_VALUE;
} else if (updateResult != NO_ERROR || mUpdateTexImageFailed) {
@@ -270,7 +271,7 @@
if (queuedBuffer) {
Mutex::Autolock lock(mQueueItemLock);
mQueueItems.clear();
- android_atomic_and(0, &mQueuedFrames);
+ mQueuedFrames = 0;
mTimeStats.clearLayerRecord(getName().c_str());
}
@@ -294,7 +295,7 @@
while (mQueueItems[0].mFrameNumber != currentFrameNumber) {
mTimeStats.removeTimeRecord(getName().c_str(), mQueueItems[0].mFrameNumber);
mQueueItems.removeAt(0);
- android_atomic_dec(&mQueuedFrames);
+ mQueuedFrames--;
}
const std::string layerName(getName().c_str());
@@ -306,7 +307,7 @@
// Decrement the queued-frames count. Signal another event if we
// have more frames pending.
- if ((queuedBuffer && android_atomic_dec(&mQueuedFrames) > 1) || mAutoRefresh) {
+ if ((queuedBuffer && mQueuedFrames.fetch_sub(1) > 1) || mAutoRefresh) {
mFlinger->signalLayerUpdate();
}
@@ -367,7 +368,7 @@
}
mQueueItems.push_back(item);
- android_atomic_inc(&mQueuedFrames);
+ mQueuedFrames++;
// Wake up any pending callbacks
mLastFrameNumberReceived = item.mFrameNumber;
@@ -404,8 +405,9 @@
}
void BufferQueueLayer::onSidebandStreamChanged() {
- if (android_atomic_release_cas(false, true, &mSidebandStreamChanged) == 0) {
- // mSidebandStreamChanged was false
+ bool sidebandStreamChanged = false;
+ if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, true)) {
+ // mSidebandStreamChanged was changed to true
mFlinger->signalLayerUpdate();
}
}
diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h
index 579ed81..400f412 100644
--- a/services/surfaceflinger/BufferQueueLayer.h
+++ b/services/surfaceflinger/BufferQueueLayer.h
@@ -137,8 +137,8 @@
int mActiveBufferSlot;
// thread-safe
- volatile int32_t mQueuedFrames;
- volatile int32_t mSidebandStreamChanged; // used like an atomic boolean
+ std::atomic<int32_t> mQueuedFrames{0};
+ std::atomic<bool> mSidebandStreamChanged{false};
};
} // namespace android
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 03b63bd..19c84d0 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -62,12 +62,11 @@
namespace android {
-int32_t Layer::sSequence = 1;
+std::atomic<int32_t> Layer::sSequence{1};
Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& name, uint32_t w,
uint32_t h, uint32_t flags)
: contentDirty(false),
- sequence(uint32_t(android_atomic_inc(&sSequence))),
mFlinger(flinger),
mPremultipliedAlpha(true),
mName(name),
@@ -1019,11 +1018,11 @@
}
uint32_t Layer::getTransactionFlags(uint32_t flags) {
- return android_atomic_and(~flags, &mTransactionFlags) & flags;
+ return mTransactionFlags.fetch_and(~flags) & flags;
}
uint32_t Layer::setTransactionFlags(uint32_t flags) {
- return android_atomic_or(flags, &mTransactionFlags);
+ return mTransactionFlags.fetch_or(flags);
}
bool Layer::setPosition(float x, float y, bool immediate) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 3845b22..74f0a63 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -73,7 +73,7 @@
// ---------------------------------------------------------------------------
class Layer : public virtual RefBase {
- static int32_t sSequence;
+ static std::atomic<int32_t> sSequence;
public:
friend class LayerBE;
@@ -89,7 +89,7 @@
// Layer serial number. This gives layers an explicit ordering, so we
// have a stable sort order when their layer stack and Z-order are
// the same.
- int32_t sequence;
+ int32_t sequence{sSequence++};
enum { // flags for doTransaction()
eDontUpdateGeometryState = 0x00000001,
@@ -271,6 +271,7 @@
virtual void useSurfaceDamage() {}
virtual void useEmptyDamage() {}
+ uint32_t getTransactionFlags() const { return mTransactionFlags; }
uint32_t getTransactionFlags(uint32_t flags);
uint32_t setTransactionFlags(uint32_t flags);
@@ -696,7 +697,7 @@
// these are protected by an external lock
State mCurrentState;
State mDrawingState;
- volatile int32_t mTransactionFlags;
+ std::atomic<uint32_t> mTransactionFlags{0};
// Accessed from main thread and binder threads
Mutex mPendingStateMutex;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index eb04563..157cbea 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -259,12 +259,10 @@
SurfaceFlinger::SurfaceFlinger(SurfaceFlinger::SkipInitializationTag)
: BnSurfaceComposer(),
- mTransactionFlags(0),
mTransactionPending(false),
mAnimTransactionPending(false),
mLayersRemoved(false),
mLayersAdded(false),
- mRepaintEverything(0),
mBootTime(systemTime()),
mDisplayTokens(),
mVisibleRegionsDirty(false),
@@ -1468,7 +1466,7 @@
resyncToHardwareVsync(false);
- android_atomic_or(1, &mRepaintEverything);
+ mRepaintEverything = true;
setTransactionFlags(eDisplayTransactionNeeded);
}
@@ -1527,7 +1525,7 @@
mRefreshPending = false;
- const bool repaintEverything = android_atomic_and(0, &mRepaintEverything);
+ const bool repaintEverything = mRepaintEverything.exchange(false);
preComposition();
rebuildLayerStacks();
calculateWorkingSet();
@@ -3355,11 +3353,11 @@
}
uint32_t SurfaceFlinger::peekTransactionFlags() {
- return android_atomic_release_load(&mTransactionFlags);
+ return mTransactionFlags;
}
uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags) {
- return android_atomic_and(~flags, &mTransactionFlags) & flags;
+ return mTransactionFlags.fetch_and(~flags) & flags;
}
uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags) {
@@ -3368,7 +3366,7 @@
uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags,
VSyncModulator::TransactionStart transactionStart) {
- uint32_t old = android_atomic_or(flags, &mTransactionFlags);
+ uint32_t old = mTransactionFlags.fetch_or(flags);
mVsyncModulator.setTransactionStart(transactionStart);
if ((old & flags)==0) { // wake the server up
signalTransaction();
@@ -4663,7 +4661,7 @@
}
result.appendFormat(" transaction-flags : %08x\n"
" gpu_to_cpu_unsupported : %d\n",
- mTransactionFlags, !mGpuToCpuSupported);
+ mTransactionFlags.load(), !mGpuToCpuSupported);
if (display) {
const auto activeConfig = getHwComposer().getActiveConfig(display->getId());
@@ -5180,7 +5178,7 @@
}
void SurfaceFlinger::repaintEverything() {
- android_atomic_or(1, &mRepaintEverything);
+ mRepaintEverything = true;
signalTransaction();
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 01d18d9..e2be544 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -793,7 +793,7 @@
// access must be protected by mStateLock
mutable Mutex mStateLock;
State mCurrentState{LayerVector::StateSet::Current};
- volatile int32_t mTransactionFlags;
+ std::atomic<int32_t> mTransactionFlags{0};
Condition mTransactionCV;
bool mTransactionPending;
bool mAnimTransactionPending;
@@ -812,8 +812,7 @@
bool mLayersRemoved;
bool mLayersAdded;
- // access must be protected by mInvalidateLock
- volatile int32_t mRepaintEverything;
+ std::atomic<bool> mRepaintEverything{false};
// helper methods
void configureHwcCommonData(const CompositionInfo& compositionInfo) const;
diff --git a/services/surfaceflinger/SurfaceFlingerFactory.cpp b/services/surfaceflinger/SurfaceFlingerFactory.cpp
new file mode 100644
index 0000000..b37d6f0
--- /dev/null
+++ b/services/surfaceflinger/SurfaceFlingerFactory.cpp
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "SurfaceFlingerFactory.h"
+#include "SurfaceFlinger.h"
+
+namespace android::surfaceflinger {
+
+sp<SurfaceFlinger> createSurfaceFlinger() {
+ return new SurfaceFlinger();
+}
+
+} // namespace android::surfaceflinger
diff --git a/services/surfaceflinger/SurfaceFlingerFactory.h b/services/surfaceflinger/SurfaceFlingerFactory.h
new file mode 100644
index 0000000..65eb797
--- /dev/null
+++ b/services/surfaceflinger/SurfaceFlingerFactory.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <cutils/compiler.h>
+#include <utils/StrongPointer.h>
+
+namespace android {
+
+class SurfaceFlinger;
+
+namespace surfaceflinger {
+
+ANDROID_API sp<SurfaceFlinger> createSurfaceFlinger();
+
+} // namespace surfaceflinger
+} // namespace android
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 0b4c6fc..57bda5a 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -96,8 +96,9 @@
const sp<const Layer>& layer)
{
Transaction* transaction(increment->mutable_transaction());
- transaction->set_synchronous(layer->mTransactionFlags & BnSurfaceComposer::eSynchronous);
- transaction->set_animation(layer->mTransactionFlags & BnSurfaceComposer::eAnimation);
+ const uint32_t layerFlags = layer->getTransactionFlags();
+ transaction->set_synchronous(layerFlags & BnSurfaceComposer::eSynchronous);
+ transaction->set_animation(layerFlags & BnSurfaceComposer::eAnimation);
const int32_t layerId(getLayerId(layer));
addPositionLocked(transaction, layerId, layer->mCurrentState.active_legacy.transform.tx(),
diff --git a/services/surfaceflinger/main_surfaceflinger.cpp b/services/surfaceflinger/main_surfaceflinger.cpp
index b1ff522..9c2edca 100644
--- a/services/surfaceflinger/main_surfaceflinger.cpp
+++ b/services/surfaceflinger/main_surfaceflinger.cpp
@@ -21,16 +21,16 @@
#include <android/frameworks/displayservice/1.0/IDisplayService.h>
#include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
#include <android/hardware/graphics/allocator/2.0/IAllocator.h>
-#include <cutils/sched_policy.h>
-#include <binder/IServiceManager.h>
#include <binder/IPCThreadState.h>
-#include <binder/ProcessState.h>
#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
+#include <configstore/Utils.h>
+#include <cutils/sched_policy.h>
#include <displayservice/DisplayService.h>
#include <hidl/LegacySupport.h>
-#include <configstore/Utils.h>
#include "GpuService.h"
#include "SurfaceFlinger.h"
+#include "SurfaceFlingerFactory.h"
using namespace android;
@@ -85,7 +85,7 @@
ps->startThreadPool();
// instantiate surfaceflinger
- sp<SurfaceFlinger> flinger = new SurfaceFlinger();
+ sp<SurfaceFlinger> flinger = surfaceflinger::createSurfaceFlinger();
setpriority(PRIO_PROCESS, 0, PRIORITY_URGENT_DISPLAY);