diff options
45 files changed, 407 insertions, 341 deletions
diff --git a/include/input/Input.h b/include/input/Input.h index 2e326cb102..dce6ccb3d5 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -1015,6 +1015,25 @@ private: std::queue<std::unique_ptr<DragEvent>> mDragEventPool; }; +/* + * Describes a unique request to enable or disable Pointer Capture. + */ +struct PointerCaptureRequest { +public: + inline PointerCaptureRequest() : enable(false), seq(0) {} + inline PointerCaptureRequest(bool enable, uint32_t seq) : enable(enable), seq(seq) {} + inline bool operator==(const PointerCaptureRequest& other) const { + return enable == other.enable && seq == other.seq; + } + explicit inline operator bool() const { return enable; } + + // True iff this is a request to enable Pointer Capture. + bool enable; + + // The sequence number for the request. + uint32_t seq; +}; + } // namespace android #endif // _LIBINPUT_INPUT_H diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index 1dcb94c80f..6471aa6d6b 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -36,7 +36,8 @@ namespace android { // --------------------------------------------------------------------------- Mutex BpBinder::sTrackingLock; -std::unordered_map<int32_t,uint32_t> BpBinder::sTrackingMap; +std::unordered_map<int32_t, uint32_t> BpBinder::sTrackingMap; +std::unordered_map<int32_t, uint32_t> sLastLimitCallbackMap; int BpBinder::sNumTrackedUids = 0; std::atomic_bool BpBinder::sCountByUidEnabled(false); binder_proxy_limit_callback BpBinder::sLimitCallback; @@ -117,12 +118,24 @@ sp<BpBinder> BpBinder::create(int32_t handle) { if (sBinderProxyThrottleCreate) { return nullptr; } + trackedValue = trackedValue & COUNTING_VALUE_MASK; + uint32_t lastLimitCallbackAt = sLastLimitCallbackMap[trackedUid]; + + if (trackedValue > lastLimitCallbackAt && + (trackedValue - lastLimitCallbackAt > sBinderProxyCountHighWatermark)) { + ALOGE("Still too many binder proxy objects sent to uid %d from uid %d (%d proxies " + "held)", + getuid(), trackedUid, trackedValue); + if (sLimitCallback) sLimitCallback(trackedUid); + sLastLimitCallbackMap[trackedUid] = trackedValue; + } } else { if ((trackedValue & COUNTING_VALUE_MASK) >= sBinderProxyCountHighWatermark) { ALOGE("Too many binder proxy objects sent to uid %d from uid %d (%d proxies held)", getuid(), trackedUid, trackedValue); sTrackingMap[trackedUid] |= LIMIT_REACHED_MASK; if (sLimitCallback) sLimitCallback(trackedUid); + sLastLimitCallbackMap[trackedUid] = trackedValue & COUNTING_VALUE_MASK; if (sBinderProxyThrottleCreate) { ALOGI("Throttling binder proxy creates from uid %d in uid %d until binder proxy" " count drops below %d", @@ -452,8 +465,9 @@ BpBinder::~BpBinder() ((trackedValue & COUNTING_VALUE_MASK) <= sBinderProxyCountLowWatermark) )) { ALOGI("Limit reached bit reset for uid %d (fewer than %d proxies from uid %d held)", - getuid(), mTrackedUid, sBinderProxyCountLowWatermark); + getuid(), sBinderProxyCountLowWatermark, mTrackedUid); sTrackingMap[mTrackedUid] &= ~LIMIT_REACHED_MASK; + sLastLimitCallbackMap.erase(mTrackedUid); } if (--sTrackingMap[mTrackedUid] == 0) { sTrackingMap.erase(mTrackedUid); diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 445df9eeff..c415ea02e2 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -1398,6 +1398,25 @@ status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, bool *sync_received, bo return ret; } +#ifndef __ANDROID_VNDK__ +status_t IPCThreadState::getProcessFreezeInfo(pid_t pid, uint32_t *sync_received, + uint32_t *async_received) +{ + int ret = 0; + binder_frozen_status_info info; + info.pid = pid; + +#if defined(__ANDROID__) + if (ioctl(self()->mProcess->mDriverFD, BINDER_GET_FROZEN_INFO, &info) < 0) + ret = -errno; +#endif + *sync_received = info.sync_recv; + *async_received = info.async_recv; + + return ret; +} +#endif + status_t IPCThreadState::freeze(pid_t pid, bool enable, uint32_t timeout_ms) { struct binder_freeze_info info; int ret = 0; diff --git a/libs/binder/binder_module.h b/libs/binder/binder_module.h index 9dea3b448b..793795e1d4 100644 --- a/libs/binder/binder_module.h +++ b/libs/binder/binder_module.h @@ -74,6 +74,8 @@ struct binder_frozen_status_info { // // Indicates whether the process has received any sync calls since last // freeze (cleared at freeze/unfreeze) + // bit 0: received sync transaction after being frozen + // bit 1: new pending sync transaction during freezing // __u32 sync_recv; // diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h index 196a41ba55..b401ea96cb 100644 --- a/libs/binder/include/binder/IPCThreadState.h +++ b/libs/binder/include/binder/IPCThreadState.h @@ -53,6 +53,13 @@ public: // Provide information about the state of a frozen process static status_t getProcessFreezeInfo(pid_t pid, bool *sync_received, bool *async_received); + + // TODO: Remove the above legacy duplicated function in next version +#ifndef __ANDROID_VNDK__ + static status_t getProcessFreezeInfo(pid_t pid, uint32_t *sync_received, + uint32_t *async_received); +#endif + sp<ProcessState> process(); status_t clearLastError(); diff --git a/libs/binder/libbinder.arm32.map b/libs/binder/libbinder.arm32.map index f26c33dff5..9f10b677bd 100644 --- a/libs/binder/libbinder.arm32.map +++ b/libs/binder/libbinder.arm32.map @@ -149,6 +149,7 @@ LIBBINDER { _ZN7android14IPCThreadState20clearCallingIdentityEv; _ZN7android14IPCThreadState20getAndExecuteCommandEv; _ZN7android14IPCThreadState20getProcessFreezeInfoEiPbS1_; + _ZN7android14IPCThreadState20getProcessFreezeInfoEiPjS1_; _ZN7android14IPCThreadState20handlePolledCommandsEv; _ZN7android14IPCThreadState20processPendingDerefsEv; _ZN7android14IPCThreadState20writeTransactionDataEijijRKNS_6ParcelEPi; diff --git a/libs/binder/libbinder.arm64.map b/libs/binder/libbinder.arm64.map index 62112502ab..1d3da4c81a 100644 --- a/libs/binder/libbinder.arm64.map +++ b/libs/binder/libbinder.arm64.map @@ -150,6 +150,7 @@ LIBBINDER { _ZN7android14IPCThreadState20clearCallingIdentityEv; _ZN7android14IPCThreadState20getAndExecuteCommandEv; _ZN7android14IPCThreadState20getProcessFreezeInfoEiPbS1_; + _ZN7android14IPCThreadState20getProcessFreezeInfoEiPjS1_; _ZN7android14IPCThreadState20handlePolledCommandsEv; _ZN7android14IPCThreadState20processPendingDerefsEv; _ZN7android14IPCThreadState20writeTransactionDataEijijRKNS_6ParcelEPi; diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 5612d1db66..64196ba30f 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -425,31 +425,30 @@ TEST_F(BinderLibTest, NopTransactionClear) { TEST_F(BinderLibTest, Freeze) { Parcel data, reply, replypid; - std::ifstream freezer_file("/sys/fs/cgroup/freezer/cgroup.freeze"); + std::ifstream freezer_file("/sys/fs/cgroup/uid_0/cgroup.freeze"); - //Pass test on devices where the freezer is not supported + // Pass test on devices where the cgroup v2 freezer is not supported if (freezer_file.fail()) { GTEST_SKIP(); return; } - std::string freezer_enabled; - std::getline(freezer_file, freezer_enabled); - - //Pass test on devices where the freezer is disabled - if (freezer_enabled != "1") { - GTEST_SKIP(); - return; - } - EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_GETPID, data, &replypid), StatusEq(NO_ERROR)); int32_t pid = replypid.readInt32(); for (int i = 0; i < 10; i++) { EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION_WAIT, data, &reply, TF_ONE_WAY)); } - EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, 1, 0)); - EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, 1, 0)); - EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, 1, 1000)); + + // Pass test on devices where BINDER_FREEZE ioctl is not supported + int ret = IPCThreadState::self()->freeze(pid, false, 0); + if (ret != 0) { + GTEST_SKIP(); + return; + } + + EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, true, 0)); + EXPECT_EQ(-EAGAIN, IPCThreadState::self()->freeze(pid, true, 0)); + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, true, 1000)); EXPECT_EQ(FAILED_TRANSACTION, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); bool sync_received, async_received; @@ -460,6 +459,14 @@ TEST_F(BinderLibTest, Freeze) { EXPECT_EQ(sync_received, 1); EXPECT_EQ(async_received, 0); + uint32_t sync_received2, async_received2; + + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->getProcessFreezeInfo(pid, &sync_received2, + &async_received2)); + + EXPECT_EQ(sync_received2, 1); + EXPECT_EQ(async_received2, 0); + EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, 0, 0)); EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); } diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index d54de4999c..7f0cac5d4f 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -343,80 +343,6 @@ void* GraphicsEnv::loadLibrary(std::string name) { return nullptr; } -bool GraphicsEnv::checkAngleRules(void* so) { - auto manufacturer = base::GetProperty("ro.product.manufacturer", "UNSET"); - auto model = base::GetProperty("ro.product.model", "UNSET"); - - auto ANGLEGetFeatureSupportUtilAPIVersion = - (fpANGLEGetFeatureSupportUtilAPIVersion)dlsym(so, - "ANGLEGetFeatureSupportUtilAPIVersion"); - - if (!ANGLEGetFeatureSupportUtilAPIVersion) { - ALOGW("Cannot find ANGLEGetFeatureSupportUtilAPIVersion function"); - return false; - } - - // Negotiate the interface version by requesting most recent known to the platform - unsigned int versionToUse = CURRENT_ANGLE_API_VERSION; - if (!(ANGLEGetFeatureSupportUtilAPIVersion)(&versionToUse)) { - ALOGW("Cannot use ANGLE feature-support library, it is older than supported by EGL, " - "requested version %u", - versionToUse); - return false; - } - - // Add and remove versions below as needed - bool useAngle = false; - switch (versionToUse) { - case 2: { - ALOGV("Using version %d of ANGLE feature-support library", versionToUse); - void* rulesHandle = nullptr; - int rulesVersion = 0; - void* systemInfoHandle = nullptr; - - // Get the symbols for the feature-support-utility library: -#define GET_SYMBOL(symbol) \ - fp##symbol symbol = (fp##symbol)dlsym(so, #symbol); \ - if (!symbol) { \ - ALOGW("Cannot find " #symbol " in ANGLE feature-support library"); \ - break; \ - } - GET_SYMBOL(ANGLEAndroidParseRulesString); - GET_SYMBOL(ANGLEGetSystemInfo); - GET_SYMBOL(ANGLEAddDeviceInfoToSystemInfo); - GET_SYMBOL(ANGLEShouldBeUsedForApplication); - GET_SYMBOL(ANGLEFreeRulesHandle); - GET_SYMBOL(ANGLEFreeSystemInfoHandle); - - // Parse the rules, obtain the SystemInfo, and evaluate the - // application against the rules: - if (!(ANGLEAndroidParseRulesString)(mRulesBuffer.data(), &rulesHandle, &rulesVersion)) { - ALOGW("ANGLE feature-support library cannot parse rules file"); - break; - } - if (!(ANGLEGetSystemInfo)(&systemInfoHandle)) { - ALOGW("ANGLE feature-support library cannot obtain SystemInfo"); - break; - } - if (!(ANGLEAddDeviceInfoToSystemInfo)(manufacturer.c_str(), model.c_str(), - systemInfoHandle)) { - ALOGW("ANGLE feature-support library cannot add device info to SystemInfo"); - break; - } - useAngle = (ANGLEShouldBeUsedForApplication)(rulesHandle, rulesVersion, - systemInfoHandle, mAngleAppName.c_str()); - (ANGLEFreeRulesHandle)(rulesHandle); - (ANGLEFreeSystemInfoHandle)(systemInfoHandle); - } break; - - default: - ALOGW("Version %u of ANGLE feature-support library is NOT supported.", versionToUse); - } - - ALOGV("Close temporarily-loaded ANGLE opt-in/out logic"); - return useAngle; -} - bool GraphicsEnv::shouldUseAngle(std::string appName) { if (appName != mAngleAppName) { // Make sure we are checking the app we were init'ed for @@ -444,31 +370,20 @@ void GraphicsEnv::updateUseAngle() { const char* ANGLE_PREFER_ANGLE = "angle"; const char* ANGLE_PREFER_NATIVE = "native"; + mUseAngle = NO; if (mAngleDeveloperOptIn == ANGLE_PREFER_ANGLE) { ALOGV("User set \"Developer Options\" to force the use of ANGLE"); mUseAngle = YES; } else if (mAngleDeveloperOptIn == ANGLE_PREFER_NATIVE) { ALOGV("User set \"Developer Options\" to force the use of Native"); - mUseAngle = NO; } else { - // The "Developer Options" value wasn't set to force the use of ANGLE. Need to temporarily - // load ANGLE and call the updatable opt-in/out logic: - void* featureSo = loadLibrary("feature_support"); - if (featureSo) { - ALOGV("loaded ANGLE's opt-in/out logic from namespace"); - mUseAngle = checkAngleRules(featureSo) ? YES : NO; - dlclose(featureSo); - featureSo = nullptr; - } else { - ALOGV("Could not load the ANGLE opt-in/out logic, cannot use ANGLE."); - } + ALOGV("User set invalid \"Developer Options\": '%s'", mAngleDeveloperOptIn.c_str()); } } void GraphicsEnv::setAngleInfo(const std::string path, const std::string appName, const std::string developerOptIn, - const std::vector<std::string> eglFeatures, const int rulesFd, - const long rulesOffset, const long rulesLength) { + const std::vector<std::string> eglFeatures) { if (mUseAngle != UNKNOWN) { // We've already figured out an answer for this app, so just return. ALOGV("Already evaluated the rules file for '%s': use ANGLE = %s", appName.c_str(), @@ -485,22 +400,6 @@ void GraphicsEnv::setAngleInfo(const std::string path, const std::string appName ALOGV("setting ANGLE application opt-in to '%s'", developerOptIn.c_str()); mAngleDeveloperOptIn = developerOptIn; - lseek(rulesFd, rulesOffset, SEEK_SET); - mRulesBuffer = std::vector<char>(rulesLength + 1); - ssize_t numBytesRead = read(rulesFd, mRulesBuffer.data(), rulesLength); - if (numBytesRead < 0) { - ALOGE("Cannot read rules file: numBytesRead = %zd", numBytesRead); - numBytesRead = 0; - } else if (numBytesRead == 0) { - ALOGW("Empty rules file"); - } - if (numBytesRead != rulesLength) { - ALOGW("Did not read all of the necessary bytes from the rules file." - "expected: %ld, got: %zd", - rulesLength, numBytesRead); - } - mRulesBuffer[numBytesRead] = '\0'; - // Update the current status of whether we should use ANGLE or not updateUseAngle(); } diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index 900fc49b59..56d1139f57 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -97,8 +97,7 @@ public: // in the search path must have a '!' after the zip filename, e.g. // /system/app/ANGLEPrebuilt/ANGLEPrebuilt.apk!/lib/arm64-v8a void setAngleInfo(const std::string path, const std::string appName, std::string devOptIn, - const std::vector<std::string> eglFeatures, const int rulesFd, - const long rulesOffset, const long rulesLength); + const std::vector<std::string> eglFeatures); // Get the ANGLE driver namespace. android_namespace_t* getAngleNamespace(); // Get the app name for ANGLE debug message. @@ -129,8 +128,6 @@ private: // Load requested ANGLE library. void* loadLibrary(std::string name); - // Check ANGLE support with the rules. - bool checkAngleRules(void* so); // Update whether ANGLE should be used. void updateUseAngle(); // Link updatable driver namespace with llndk and vndk-sp libs. @@ -159,8 +156,6 @@ private: std::string mAngleDeveloperOptIn; // ANGLE EGL features; std::vector<std::string> mAngleEglFeatures; - // ANGLE rules. - std::vector<char> mRulesBuffer; // Use ANGLE flag. UseAngle mUseAngle = UNKNOWN; // Vulkan debug layers libs. diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 56a9683773..94e1ae1c74 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -118,12 +118,12 @@ void BLASTBufferItemConsumer::getConnectionEvents(uint64_t frameNumber, bool* ne } void BLASTBufferItemConsumer::setBlastBufferQueue(BLASTBufferQueue* blastbufferqueue) { - Mutex::Autolock lock(mMutex); + std::scoped_lock lock(mBufferQueueMutex); mBLASTBufferQueue = blastbufferqueue; } void BLASTBufferItemConsumer::onSidebandStreamChanged() { - Mutex::Autolock lock(mMutex); + std::scoped_lock lock(mBufferQueueMutex); if (mBLASTBufferQueue != nullptr) { sp<NativeHandle> stream = getSidebandStream(); mBLASTBufferQueue->setSidebandStream(stream); @@ -630,7 +630,10 @@ bool BLASTBufferQueue::maxBuffersAcquired(bool includeExtraAcquire) const { class BBQSurface : public Surface { private: + std::mutex mMutex; sp<BLASTBufferQueue> mBbq; + bool mDestroyed = false; + public: BBQSurface(const sp<IGraphicBufferProducer>& igbp, bool controlledByApp, const sp<IBinder>& scHandle, const sp<BLASTBufferQueue>& bbq) @@ -650,6 +653,10 @@ public: status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } if (!ValidateFrameRate(frameRate, compatibility, changeFrameRateStrategy, "BBQSurface::setFrameRate")) { return BAD_VALUE; @@ -658,8 +665,20 @@ public: } status_t setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } return mBbq->setFrameTimelineInfo(frameTimelineInfo); } + + void destroy() override { + Surface::destroy(); + + std::unique_lock _lock{mMutex}; + mDestroyed = true; + mBbq = nullptr; + } }; // TODO: Can we coalesce this with frame updates? Need to confirm diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 2edb4e4ba4..353a91d062 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -2622,4 +2622,14 @@ status_t Surface::setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInf return composerService()->setFrameTimelineInfo(mGraphicBufferProducer, frameTimelineInfo); } +sp<IBinder> Surface::getSurfaceControlHandle() const { + Mutex::Autolock lock(mMutex); + return mSurfaceControlHandle; +} + +void Surface::destroy() { + Mutex::Autolock lock(mMutex); + mSurfaceControlHandle = nullptr; +} + }; // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index ea9b1c68af..6c5b2aa53c 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -62,11 +62,12 @@ private: uint64_t mCurrentFrameNumber = 0; Mutex mMutex; + std::mutex mBufferQueueMutex; ConsumerFrameEventHistory mFrameEventHistory GUARDED_BY(mMutex); std::queue<uint64_t> mDisconnectEvents GUARDED_BY(mMutex); bool mCurrentlyConnected GUARDED_BY(mMutex); bool mPreviouslyConnected GUARDED_BY(mMutex); - BLASTBufferQueue* mBLASTBufferQueue GUARDED_BY(mMutex); + BLASTBufferQueue* mBLASTBufferQueue GUARDED_BY(mBufferQueueMutex); }; class BLASTBufferQueue diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h index 7e4143b1f3..e5403512a9 100644 --- a/libs/gui/include/gui/Surface.h +++ b/libs/gui/include/gui/Surface.h @@ -99,7 +99,7 @@ public: */ sp<IGraphicBufferProducer> getIGraphicBufferProducer() const; - sp<IBinder> getSurfaceControlHandle() const { return mSurfaceControlHandle; } + sp<IBinder> getSurfaceControlHandle() const; /* convenience function to check that the given surface is non NULL as * well as its IGraphicBufferProducer */ @@ -333,6 +333,7 @@ public: virtual int connect( int api, bool reportBufferRemoval, const sp<SurfaceListener>& sListener); + virtual void destroy(); // When client connects to Surface with reportBufferRemoval set to true, any buffers removed // from this Surface will be collected and returned here. Once this method returns, these diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index 3c59f11395..94023e6247 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -424,14 +424,28 @@ base::unique_fd SkiaGLRenderEngine::flush() { return fenceFd; } -bool SkiaGLRenderEngine::waitFence(base::unique_fd fenceFd) { +void SkiaGLRenderEngine::waitFence(base::borrowed_fd fenceFd) { + if (fenceFd.get() >= 0 && !waitGpuFence(fenceFd)) { + ATRACE_NAME("SkiaGLRenderEngine::waitFence"); + sync_wait(fenceFd.get(), -1); + } +} + +bool SkiaGLRenderEngine::waitGpuFence(base::borrowed_fd fenceFd) { if (!gl::GLExtensions::getInstance().hasNativeFenceSync() || !gl::GLExtensions::getInstance().hasWaitSync()) { return false; } + // Duplicate the fence for passing to eglCreateSyncKHR. + base::unique_fd fenceDup(dup(fenceFd.get())); + if (fenceDup.get() < 0) { + ALOGE("failed to create duplicate fence fd: %d", fenceDup.get()); + return false; + } + // release the fd and transfer the ownership to EGLSync - EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fenceFd.release(), EGL_NONE}; + EGLint attribs[] = {EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fenceDup.release(), EGL_NONE}; EGLSyncKHR sync = eglCreateSyncKHR(mEGLDisplay, EGL_SYNC_NATIVE_FENCE_ANDROID, attribs); if (sync == EGL_NO_SYNC_KHR) { ALOGE("failed to create EGL native fence sync: %#x", eglGetError()); @@ -726,14 +740,6 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, return NO_ERROR; } - if (bufferFence.get() >= 0) { - // Duplicate the fence for passing to waitFence. - base::unique_fd bufferFenceDup(dup(bufferFence.get())); - if (bufferFenceDup < 0 || !waitFence(std::move(bufferFenceDup))) { - ATRACE_NAME("Waiting before draw"); - sync_wait(bufferFence.get(), -1); - } - } if (buffer == nullptr) { ALOGE("No output buffer provided. Aborting GPU composition."); return BAD_VALUE; @@ -758,6 +764,9 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, true, mTextureCleanupMgr); } + // wait on the buffer to be ready to use prior to using it + waitFence(bufferFence); + const ui::Dataspace dstDataspace = mUseColorManagement ? display.outputDataspace : ui::Dataspace::V0_SRGB_LINEAR; sk_sp<SkSurface> dstSurface = surfaceTextureRef->getOrCreateSurface(dstDataspace, grContext); @@ -1014,6 +1023,12 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, false, mTextureCleanupMgr); } + // if the layer's buffer has a fence, then we must must respect the fence prior to using + // the buffer. + if (layer->source.buffer.fence != nullptr) { + waitFence(layer->source.buffer.fence->get()); + } + // isOpaque means we need to ignore the alpha in the image, // replacing it with the alpha specified by the LayerSettings. See // https://developer.android.com/reference/android/view/SurfaceControl.Builder#setOpaque(boolean) diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h index a852bbcaf8..238ad8f452 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.h +++ b/libs/renderengine/skia/SkiaGLRenderEngine.h @@ -99,7 +99,10 @@ private: inline GrDirectContext* getActiveGrContext() const; base::unique_fd flush(); - bool waitFence(base::unique_fd fenceFd); + // waitFence attempts to wait in the GPU, and if unable to waits on the CPU instead. + void waitFence(base::borrowed_fd fenceFd); + bool waitGpuFence(base::borrowed_fd fenceFd); + void initCanvas(SkCanvas* canvas, const DisplaySettings& display); void drawShadow(SkCanvas* canvas, const SkRRect& casterRRect, const ShadowSettings& shadowSettings); diff --git a/services/inputflinger/InputListener.cpp b/services/inputflinger/InputListener.cpp index 33b3e1ee2f..71b0f5fec9 100644 --- a/services/inputflinger/InputListener.cpp +++ b/services/inputflinger/InputListener.cpp @@ -287,16 +287,16 @@ void NotifyDeviceResetArgs::notify(const sp<InputListenerInterface>& listener) c // --- NotifyPointerCaptureChangedArgs --- -NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, - bool enabled) - : NotifyArgs(id, eventTime), enabled(enabled) {} +NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( + int32_t id, nsecs_t eventTime, const PointerCaptureRequest& request) + : NotifyArgs(id, eventTime), request(request) {} NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( const NotifyPointerCaptureChangedArgs& other) - : NotifyArgs(other.id, other.eventTime), enabled(other.enabled) {} + : NotifyArgs(other.id, other.eventTime), request(other.request) {} bool NotifyPointerCaptureChangedArgs::operator==(const NotifyPointerCaptureChangedArgs& rhs) const { - return id == rhs.id && eventTime == rhs.eventTime && enabled == rhs.enabled; + return id == rhs.id && eventTime == rhs.eventTime && request == rhs.request; } void NotifyPointerCaptureChangedArgs::notify(const sp<InputListenerInterface>& listener) const { diff --git a/services/inputflinger/InputReaderBase.cpp b/services/inputflinger/InputReaderBase.cpp index 9cc777d450..d34482f506 100644 --- a/services/inputflinger/InputReaderBase.cpp +++ b/services/inputflinger/InputReaderBase.cpp @@ -67,6 +67,9 @@ std::string InputReaderConfiguration::changesToString(uint32_t changes) { if (changes & CHANGE_EXTERNAL_STYLUS_PRESENCE) { result += "EXTERNAL_STYLUS_PRESENCE | "; } + if (changes & CHANGE_POINTER_CAPTURE) { + result += "POINTER_CAPTURE | "; + } if (changes & CHANGE_ENABLED_STATE) { result += "ENABLED_STATE | "; } diff --git a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp index bc77b8aef4..aa8cc302c7 100644 --- a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp +++ b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp @@ -113,7 +113,7 @@ private: void onPointerDownOutsideFocus(const sp<IBinder>& newToken) override {} - void setPointerCapture(bool enabled) override {} + void setPointerCapture(const PointerCaptureRequest&) override {} void notifyDropWindow(const sp<IBinder>&, float x, float y) override {} diff --git a/services/inputflinger/dispatcher/Entry.cpp b/services/inputflinger/dispatcher/Entry.cpp index 881024fc6e..5c3747e2ec 100644 --- a/services/inputflinger/dispatcher/Entry.cpp +++ b/services/inputflinger/dispatcher/Entry.cpp @@ -119,15 +119,15 @@ std::string FocusEntry::getDescription() const { // PointerCaptureChanged notifications always go to apps, so set the flag POLICY_FLAG_PASS_TO_USER // for all entries. PointerCaptureChangedEntry::PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, - bool hasPointerCapture) + const PointerCaptureRequest& request) : EventEntry(id, Type::POINTER_CAPTURE_CHANGED, eventTime, POLICY_FLAG_PASS_TO_USER), - pointerCaptureEnabled(hasPointerCapture) {} + pointerCaptureRequest(request) {} PointerCaptureChangedEntry::~PointerCaptureChangedEntry() {} std::string PointerCaptureChangedEntry::getDescription() const { return StringPrintf("PointerCaptureChangedEvent(pointerCaptureEnabled=%s)", - pointerCaptureEnabled ? "true" : "false"); + pointerCaptureRequest.enable ? "true" : "false"); } // --- DragEntry --- @@ -324,8 +324,7 @@ CommandEntry::CommandEntry(Command command) keyEntry(nullptr), userActivityEventType(0), seq(0), - handled(false), - enabled(false) {} + handled(false) {} CommandEntry::~CommandEntry() {} diff --git a/services/inputflinger/dispatcher/Entry.h b/services/inputflinger/dispatcher/Entry.h index ebbd8e93bf..6f1dfadf59 100644 --- a/services/inputflinger/dispatcher/Entry.h +++ b/services/inputflinger/dispatcher/Entry.h @@ -104,9 +104,9 @@ struct FocusEntry : EventEntry { }; struct PointerCaptureChangedEntry : EventEntry { - bool pointerCaptureEnabled; + const PointerCaptureRequest pointerCaptureRequest; - PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, bool hasPointerCapture); + PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); std::string getDescription() const override; ~PointerCaptureChangedEntry() override; @@ -284,7 +284,7 @@ struct CommandEntry { sp<IBinder> oldToken; sp<IBinder> newToken; std::string obscuringPackage; - bool enabled; + PointerCaptureRequest pointerCaptureRequest; int32_t pid; nsecs_t consumeTime; // time when the event was consumed by InputConsumer int32_t displayId; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 1899c5f29d..ce1f266cfb 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -523,7 +523,6 @@ InputDispatcher::InputDispatcher(const sp<InputDispatcherPolicyInterface>& polic mInTouchMode(true), mMaximumObscuringOpacityForTouch(1.0f), mFocusedDisplayId(ADISPLAY_ID_DEFAULT), - mFocusedWindowRequestedPointerCapture(false), mWindowTokenWithPointerCapture(nullptr), mLatencyAggregator(), mLatencyTracker(&mLatencyAggregator), @@ -1306,36 +1305,51 @@ void InputDispatcher::dispatchFocusLocked(nsecs_t currentTime, std::shared_ptr<F void InputDispatcher::dispatchPointerCaptureChangedLocked( nsecs_t currentTime, const std::shared_ptr<PointerCaptureChangedEntry>& entry, DropReason& dropReason) { - const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; - if (entry->pointerCaptureEnabled && haveWindowWithPointerCapture) { - LOG_ALWAYS_FATAL("Pointer Capture has already been enabled for the window."); - } - if (!entry->pointerCaptureEnabled && !haveWindowWithPointerCapture) { - // Pointer capture was already forcefully disabled because of focus change. - dropReason = DropReason::NOT_DROPPED; - return; - } - - // Set drop reason for early returns - dropReason = DropReason::NO_POINTER_CAPTURE; + dropReason = DropReason::NOT_DROPPED; + const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; sp<IBinder> token; - if (entry->pointerCaptureEnabled) { - // Enable Pointer Capture - if (!mFocusedWindowRequestedPointerCapture) { + + if (entry->pointerCaptureRequest.enable) { + // Enable Pointer Capture. + if (haveWindowWithPointerCapture && + (entry->pointerCaptureRequest == mCurrentPointerCaptureRequest)) { + LOG_ALWAYS_FATAL("This request to enable Pointer Capture has already been dispatched " + "to the window."); + } + if (!mCurrentPointerCaptureRequest.enable) { // This can happen if a window requests capture and immediately releases capture. ALOGW("No window requested Pointer Capture."); + dropReason = DropReason::NO_POINTER_CAPTURE; + return; + } + if (entry->pointerCaptureRequest.seq != mCurrentPointerCaptureRequest.seq) { + ALOGI("Skipping dispatch of Pointer Capture being enabled: sequence number mismatch."); return; } + token = mFocusResolver.getFocusedWindowToken(mFocusedDisplayId); LOG_ALWAYS_FATAL_IF(!token, "Cannot find focused window for Pointer Capture."); mWindowTokenWithPointerCapture = token; } else { - // Disable Pointer Capture + // Disable Pointer Capture. + // We do not check if the sequence number matches for requests to disable Pointer Capture + // for two reasons: + // 1. Pointer Capture can be disabled by a focus change, which means we can get two entries + // to disable capture with the same sequence number: one generated by + // disablePointerCaptureForcedLocked() and another as an acknowledgement of Pointer + // Capture being disabled in InputReader. + // 2. We respect any request to disable Pointer Capture generated by InputReader, since the + // actual Pointer Capture state that affects events being generated by input devices is + // in InputReader. + if (!haveWindowWithPointerCapture) { + // Pointer capture was already forcefully disabled because of focus change. + dropReason = DropReason::NOT_DROPPED; + return; + } token = mWindowTokenWithPointerCapture; mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } } @@ -1344,8 +1358,7 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( if (channel == nullptr) { // Window has gone away, clean up Pointer Capture state. mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } return; @@ -3180,7 +3193,7 @@ void InputDispatcher::startDispatchCycleLocked(nsecs_t currentTime, static_cast<const PointerCaptureChangedEntry&>(eventEntry); status = connection->inputPublisher .publishCaptureEvent(dispatchEntry->seq, captureEntry.id, - captureEntry.pointerCaptureEnabled); + captureEntry.pointerCaptureRequest.enable); break; } @@ -3978,14 +3991,14 @@ void InputDispatcher::notifyDeviceReset(const NotifyDeviceResetArgs* args) { void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) { #if DEBUG_INBOUND_EVENT_DETAILS ALOGD("notifyPointerCaptureChanged - eventTime=%" PRId64 ", enabled=%s", args->eventTime, - args->enabled ? "true" : "false"); + args->request.enable ? "true" : "false"); #endif bool needWake; { // acquire lock std::scoped_lock _l(mLock); auto entry = std::make_unique<PointerCaptureChangedEntry>(args->id, args->eventTime, - args->enabled); + args->request); needWake = enqueueInboundEventLocked(std::move(entry)); } // release lock @@ -4929,8 +4942,8 @@ void InputDispatcher::logDispatchStateLocked() { std::string InputDispatcher::dumpPointerCaptureStateLocked() { std::string dump; - dump += StringPrintf(INDENT "FocusedWindowRequestedPointerCapture: %s\n", - toString(mFocusedWindowRequestedPointerCapture)); + dump += StringPrintf(INDENT "Pointer Capture Requested: %s\n", + toString(mCurrentPointerCaptureRequest.enable)); std::string windowName = "None"; if (mWindowTokenWithPointerCapture) { @@ -4939,7 +4952,7 @@ std::string InputDispatcher::dumpPointerCaptureStateLocked() { windowName = captureWindowHandle ? captureWindowHandle->getName().c_str() : "token has capture without window"; } - dump += StringPrintf(INDENT "CurrentWindowWithPointerCapture: %s\n", windowName.c_str()); + dump += StringPrintf(INDENT "Current Window with Pointer Capture: %s\n", windowName.c_str()); return dump; } @@ -5407,14 +5420,13 @@ void InputDispatcher::requestPointerCapture(const sp<IBinder>& windowToken, bool return; } - if (enabled == mFocusedWindowRequestedPointerCapture) { + if (enabled == mCurrentPointerCaptureRequest.enable) { ALOGW("Ignoring request to %s Pointer Capture: " "window has %s requested pointer capture.", enabled ? "enable" : "disable", enabled ? "already" : "not"); return; } - mFocusedWindowRequestedPointerCapture = enabled; setPointerCaptureLocked(enabled); } // release lock @@ -6173,14 +6185,13 @@ void InputDispatcher::onFocusChangedLocked(const FocusResolver::FocusChanges& ch } void InputDispatcher::disablePointerCaptureForcedLocked() { - if (!mFocusedWindowRequestedPointerCapture && !mWindowTokenWithPointerCapture) { + if (!mCurrentPointerCaptureRequest.enable && !mWindowTokenWithPointerCapture) { return; } ALOGD_IF(DEBUG_FOCUS, "Disabling Pointer Capture because the window lost focus."); - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } @@ -6197,14 +6208,16 @@ void InputDispatcher::disablePointerCaptureForcedLocked() { } auto entry = std::make_unique<PointerCaptureChangedEntry>(mIdGenerator.nextId(), now(), - false /* hasCapture */); + mCurrentPointerCaptureRequest); mInboundQueue.push_front(std::move(entry)); } void InputDispatcher::setPointerCaptureLocked(bool enabled) { + mCurrentPointerCaptureRequest.enable = enabled; + mCurrentPointerCaptureRequest.seq++; std::unique_ptr<CommandEntry> commandEntry = std::make_unique<CommandEntry>( &InputDispatcher::doSetPointerCaptureLockedInterruptible); - commandEntry->enabled = enabled; + commandEntry->pointerCaptureRequest = mCurrentPointerCaptureRequest; postCommandLocked(std::move(commandEntry)); } @@ -6212,7 +6225,7 @@ void InputDispatcher::doSetPointerCaptureLockedInterruptible( android::inputdispatcher::CommandEntry* commandEntry) { mLock.unlock(); - mPolicy->setPointerCapture(commandEntry->enabled); + mPolicy->setPointerCapture(commandEntry->pointerCaptureRequest); mLock.lock(); } diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 9edf41c9c0..30652c65ee 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -357,10 +357,12 @@ private: // Keeps track of the focused window per display and determines focus changes. FocusResolver mFocusResolver GUARDED_BY(mLock); - // Whether the focused window on the focused display has requested Pointer Capture. - // The state of this variable should always be in sync with the state of Pointer Capture in the - // policy, which is updated through setPointerCaptureLocked(enabled). - bool mFocusedWindowRequestedPointerCapture GUARDED_BY(mLock); + + // The enabled state of this request is true iff the focused window on the focused display has + // requested Pointer Capture. This request also contains the sequence number associated with the + // current request. The state of this variable should always be in sync with the state of + // Pointer Capture in the policy, and is only updated through setPointerCaptureLocked(request). + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); // The window token that has Pointer Capture. // This should be in sync with PointerCaptureChangedEvents dispatched to the input channel. @@ -370,7 +372,7 @@ private: void disablePointerCaptureForcedLocked() REQUIRES(mLock); // Set the Pointer Capture state in the Policy. - void setPointerCaptureLocked(bool enabled) REQUIRES(mLock); + void setPointerCaptureLocked(bool enable) REQUIRES(mLock); // Dispatcher state at time of last ANR. std::string mLastAnrState GUARDED_BY(mLock); diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h index 219f45a7c3..fd591e0e1c 100644 --- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h +++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h @@ -156,7 +156,7 @@ public: * * InputDispatcher is solely responsible for updating the Pointer Capture state. */ - virtual void setPointerCapture(bool enabled) = 0; + virtual void setPointerCapture(const PointerCaptureRequest&) = 0; /* Notifies the policy that the drag window has moved over to another window */ virtual void notifyDropWindow(const sp<IBinder>& token, float x, float y) = 0; diff --git a/services/inputflinger/docs/pointer_capture.md b/services/inputflinger/docs/pointer_capture.md index 8da699dc56..0b44187c08 100644 --- a/services/inputflinger/docs/pointer_capture.md +++ b/services/inputflinger/docs/pointer_capture.md @@ -17,6 +17,8 @@ `InputDispatcher` is responsible for controlling the state of Pointer Capture. Since the feature requires changes to how events are generated, Pointer Capture is configured in `InputReader`. +We use a sequence number to synchronize different requests to enable Pointer Capture between InputReader and InputDispatcher. + ### Enabling Pointer Capture There are four key steps that take place when Pointer Capture is enabled: @@ -40,5 +42,5 @@ When an `InputWindow` with Pointer Capture loses focus, Pointer Capture is disab `InputDispatcher` tracks two pieces of state information regarding Pointer Capture: -- `mFocusedWindowRequestedPointerCapture`: Whether or not the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. +- `mCurrentPointerCaptureRequest`: The sequence number of the current Pointer Capture request. This request is enabled iff the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. - `mWindowTokenWithPointerCapture`: The Binder token of the `InputWindow` that currently has Pointer Capture. This is only updated during the dispatch cycle. If it is not `nullptr`, it signifies that the window was notified that it has Pointer Capture. diff --git a/services/inputflinger/include/InputListener.h b/services/inputflinger/include/InputListener.h index 4b7d26df2b..fe74214b36 100644 --- a/services/inputflinger/include/InputListener.h +++ b/services/inputflinger/include/InputListener.h @@ -211,11 +211,12 @@ struct NotifyDeviceResetArgs : public NotifyArgs { /* Describes a change in the state of Pointer Capture. */ struct NotifyPointerCaptureChangedArgs : public NotifyArgs { - bool enabled; + // The sequence number of the Pointer Capture request, if enabled. + PointerCaptureRequest request; inline NotifyPointerCaptureChangedArgs() {} - NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, bool enabled); + NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); NotifyPointerCaptureChangedArgs(const NotifyPointerCaptureChangedArgs& other); diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index 7fdbbfdac4..3c8ac1cf7b 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -279,29 +279,30 @@ struct InputReaderConfiguration { // True to show the location of touches on the touch screen as spots. bool showTouches; - // True if pointer capture is enabled. - bool pointerCapture; + // The latest request to enable or disable Pointer Capture. + PointerCaptureRequest pointerCaptureRequest; // The set of currently disabled input devices. std::set<int32_t> disabledDevices; - InputReaderConfiguration() : - virtualKeyQuietTime(0), + InputReaderConfiguration() + : virtualKeyQuietTime(0), pointerVelocityControlParameters(1.0f, 500.0f, 3000.0f, 3.0f), wheelVelocityControlParameters(1.0f, 15.0f, 50.0f, 4.0f), pointerGesturesEnabled(true), - pointerGestureQuietInterval(100 * 1000000LL), // 100 ms - pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second - pointerGestureTapInterval(150 * 1000000LL), // 150 ms - pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms - pointerGestureTapSlop(10.0f), // 10 pixels + pointerGestureQuietInterval(100 * 1000000LL), // 100 ms + pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second + pointerGestureTapInterval(150 * 1000000LL), // 150 ms + pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms + pointerGestureTapSlop(10.0f), // 10 pixels pointerGestureMultitouchSettleInterval(100 * 1000000LL), // 100 ms - pointerGestureMultitouchMinDistance(15), // 15 pixels - pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees + pointerGestureMultitouchMinDistance(15), // 15 pixels + pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees pointerGestureSwipeMaxWidthRatio(0.25f), pointerGestureMovementSpeedRatio(0.8f), pointerGestureZoomSpeedRatio(0.3f), - showTouches(false), pointerCapture(false) { } + showTouches(false), + pointerCaptureRequest() {} static std::string changesToString(uint32_t changes); diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 10c04f606c..5120860503 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -367,9 +367,15 @@ void InputReader::refreshConfigurationLocked(uint32_t changes) { } if (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE) { - const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, - mConfig.pointerCapture); - mQueuedListener->notifyPointerCaptureChanged(&args); + if (mCurrentPointerCaptureRequest == mConfig.pointerCaptureRequest) { + ALOGV("Skipping notifying pointer capture changes: " + "There was no change in the pointer capture state."); + } else { + mCurrentPointerCaptureRequest = mConfig.pointerCaptureRequest; + const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, + mCurrentPointerCaptureRequest); + mQueuedListener->notifyPointerCaptureChanged(&args); + } } } diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index a00c5af4dc..e44aa0fe27 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -230,6 +230,8 @@ private: uint32_t mConfigurationChangesToRefresh GUARDED_BY(mLock); void refreshConfigurationLocked(uint32_t changes) REQUIRES(mLock); + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); + // state queries typedef int32_t (InputDevice::*GetStateFunc)(uint32_t sourceMask, int32_t code); int32_t getStateLocked(int32_t deviceId, uint32_t sourceMask, int32_t code, diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp index 437902a79b..2ac41b1e67 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp @@ -154,9 +154,9 @@ void CursorInputMapper::configure(nsecs_t when, const InputReaderConfiguration* mHWheelScale = 1.0f; } - if ((!changes && config->pointerCapture) || + if ((!changes && config->pointerCaptureRequest.enable) || (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE)) { - if (config->pointerCapture) { + if (config->pointerCaptureRequest.enable) { if (mParameters.mode == Parameters::MODE_POINTER) { mParameters.mode = Parameters::MODE_POINTER_RELATIVE; mSource = AINPUT_SOURCE_MOUSE_RELATIVE; diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 962d8d2935..8733540e53 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -603,7 +603,7 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // Determine device mode. if (mParameters.deviceType == Parameters::DeviceType::POINTER && - mConfig.pointerGesturesEnabled && !mConfig.pointerCapture) { + mConfig.pointerGesturesEnabled && !mConfig.pointerCaptureRequest.enable) { mSource = AINPUT_SOURCE_MOUSE; mDeviceMode = DeviceMode::POINTER; if (hasStylus()) { @@ -776,11 +776,12 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // preserve the cursor position. if (mDeviceMode == DeviceMode::POINTER || (mDeviceMode == DeviceMode::DIRECT && mConfig.showTouches) || - (mParameters.deviceType == Parameters::DeviceType::POINTER && mConfig.pointerCapture)) { + (mParameters.deviceType == Parameters::DeviceType::POINTER && + mConfig.pointerCaptureRequest.enable)) { if (mPointerController == nullptr) { mPointerController = getContext()->getPointerController(getDeviceId()); } - if (mConfig.pointerCapture) { + if (mConfig.pointerCaptureRequest.enable) { mPointerController->fade(PointerControllerInterface::Transition::IMMEDIATE); } } else { diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 3a9dede89b..50b65cad5d 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -239,19 +239,22 @@ public: mConfig.keyRepeatDelay = delay; } - void waitForSetPointerCapture(bool enabled) { + PointerCaptureRequest assertSetPointerCaptureCalled(bool enabled) { std::unique_lock lock(mLock); base::ScopedLockAssertion assumeLocked(mLock); if (!mPointerCaptureChangedCondition.wait_for(lock, 100ms, [this, enabled]() REQUIRES(mLock) { - return mPointerCaptureEnabled && - *mPointerCaptureEnabled == + return mPointerCaptureRequest->enable == enabled; })) { - FAIL() << "Timed out waiting for setPointerCapture(" << enabled << ") to be called."; + ADD_FAILURE() << "Timed out waiting for setPointerCapture(" << enabled + << ") to be called."; + return {}; } - mPointerCaptureEnabled.reset(); + auto request = *mPointerCaptureRequest; + mPointerCaptureRequest.reset(); + return request; } void assertSetPointerCaptureNotCalled() { @@ -259,11 +262,11 @@ public: base::ScopedLockAssertion assumeLocked(mLock); if (mPointerCaptureChangedCondition.wait_for(lock, 100ms) != std::cv_status::timeout) { - FAIL() << "Expected setPointerCapture(enabled) to not be called, but was called. " + FAIL() << "Expected setPointerCapture(request) to not be called, but was called. " "enabled = " - << *mPointerCaptureEnabled; + << std::to_string(mPointerCaptureRequest->enable); } - mPointerCaptureEnabled.reset(); + mPointerCaptureRequest.reset(); } void assertDropTargetEquals(const sp<IBinder>& targetToken) { @@ -281,7 +284,8 @@ private: std::optional<NotifySwitchArgs> mLastNotifySwitch GUARDED_BY(mLock); std::condition_variable mPointerCaptureChangedCondition; - std::optional<bool> mPointerCaptureEnabled GUARDED_BY(mLock); + + std::optional<PointerCaptureRequest> mPointerCaptureRequest GUARDED_BY(mLock); // ANR handling std::queue<std::shared_ptr<InputApplicationHandle>> mAnrApplications GUARDED_BY(mLock); @@ -398,9 +402,9 @@ private: mOnPointerDownToken = newToken; } - void setPointerCapture(bool enabled) override { + void setPointerCapture(const PointerCaptureRequest& request) override { std::scoped_lock lock(mLock); - mPointerCaptureEnabled = {enabled}; + mPointerCaptureRequest = {request}; mPointerCaptureChangedCondition.notify_all(); } @@ -1379,8 +1383,9 @@ static NotifyMotionArgs generateMotionArgs(int32_t action, int32_t source, int32 return generateMotionArgs(action, source, displayId, {PointF{100, 200}}); } -static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs(bool enabled) { - return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), enabled); +static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs( + const PointerCaptureRequest& request) { + return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), request); } TEST_F(InputDispatcherTest, SetInputWindow_SingleWindowTouch) { @@ -4591,16 +4596,18 @@ protected: mWindow->consumeFocusEvent(true); } - void notifyPointerCaptureChanged(bool enabled) { - const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(enabled); + void notifyPointerCaptureChanged(const PointerCaptureRequest& request) { + const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(request); mDispatcher->notifyPointerCaptureChanged(&args); } - void requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, bool enabled) { + PointerCaptureRequest requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, + bool enabled) { mDispatcher->requestPointerCapture(window->getToken(), enabled); - mFakePolicy->waitForSetPointerCapture(enabled); - notifyPointerCaptureChanged(enabled); + auto request = mFakePolicy->assertSetPointerCaptureCalled(enabled); + notifyPointerCaptureChanged(request); window->consumeCaptureEvent(enabled); + return request; } }; @@ -4622,7 +4629,7 @@ TEST_F(InputDispatcherPointerCaptureTests, EnablePointerCaptureWhenFocused) { } TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLosesFocus) { - requestAndVerifyPointerCapture(mWindow, true); + auto request = requestAndVerifyPointerCapture(mWindow, true); setFocusedWindow(mSecondWindow); @@ -4630,26 +4637,26 @@ TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLose mWindow->consumeCaptureEvent(false); mWindow->consumeFocusEvent(false); mSecondWindow->consumeFocusEvent(true); - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); // Ensure that additional state changes from InputReader are not sent to the window. - notifyPointerCaptureChanged(false); - notifyPointerCaptureChanged(true); - notifyPointerCaptureChanged(false); + notifyPointerCaptureChanged({}); + notifyPointerCaptureChanged(request); + notifyPointerCaptureChanged({}); mWindow->assertNoEvents(); mSecondWindow->assertNoEvents(); mFakePolicy->assertSetPointerCaptureNotCalled(); } TEST_F(InputDispatcherPointerCaptureTests, UnexpectedStateChangeDisablesPointerCapture) { - requestAndVerifyPointerCapture(mWindow, true); + auto request = requestAndVerifyPointerCapture(mWindow, true); // InputReader unexpectedly disables and enables pointer capture. - notifyPointerCaptureChanged(false); - notifyPointerCaptureChanged(true); + notifyPointerCaptureChanged({}); + notifyPointerCaptureChanged(request); // Ensure that Pointer Capture is disabled. - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); mWindow->consumeCaptureEvent(false); mWindow->assertNoEvents(); } @@ -4659,24 +4666,43 @@ TEST_F(InputDispatcherPointerCaptureTests, OutOfOrderRequests) { // The first window loses focus. setFocusedWindow(mSecondWindow); - mFakePolicy->waitForSetPointerCapture(false); + mFakePolicy->assertSetPointerCaptureCalled(false); mWindow->consumeCaptureEvent(false); // Request Pointer Capture from the second window before the notification from InputReader // arrives. mDispatcher->requestPointerCapture(mSecondWindow->getToken(), true); - mFakePolicy->waitForSetPointerCapture(true); + auto request = mFakePolicy->assertSetPointerCaptureCalled(true); // InputReader notifies Pointer Capture was disabled (because of the focus change). - notifyPointerCaptureChanged(false); + notifyPointerCaptureChanged({}); // InputReader notifies Pointer Capture was enabled (because of mSecondWindow's request). - notifyPointerCaptureChanged(true); + notifyPointerCaptureChanged(request); mSecondWindow->consumeFocusEvent(true); mSecondWindow->consumeCaptureEvent(true); } +TEST_F(InputDispatcherPointerCaptureTests, EnableRequestFollowsSequenceNumbers) { + // App repeatedly enables and disables capture. + mDispatcher->requestPointerCapture(mWindow->getToken(), true); + auto firstRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + mDispatcher->requestPointerCapture(mWindow->getToken(), false); + mFakePolicy->assertSetPointerCaptureCalled(false); + mDispatcher->requestPointerCapture(mWindow->getToken(), true); + auto secondRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + + // InputReader notifies that PointerCapture has been enabled for the first request. Since the + // first request is now stale, this should do nothing. + notifyPointerCaptureChanged(firstRequest); + mWindow->assertNoEvents(); + + // InputReader notifies that the second request was enabled. + notifyPointerCaptureChanged(secondRequest); + mWindow->consumeCaptureEvent(true); +} + class InputDispatcherUntrustedTouchesTest : public InputDispatcherTest { protected: constexpr static const float MAXIMUM_OBSCURING_OPACITY = 0.8; diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 997cbe88a1..38dfe4041f 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -299,8 +299,9 @@ public: transform = t; } - void setPointerCapture(bool enabled) { - mConfig.pointerCapture = enabled; + PointerCaptureRequest setPointerCapture(bool enabled) { + mConfig.pointerCaptureRequest = {enabled, mNextPointerCaptureSequenceNumber++}; + return mConfig.pointerCaptureRequest; } void setShowTouches(bool enabled) { @@ -314,6 +315,8 @@ public: float getPointerGestureMovementSpeedRatio() { return mConfig.pointerGestureMovementSpeedRatio; } private: + uint32_t mNextPointerCaptureSequenceNumber = 0; + DisplayViewport createDisplayViewport(int32_t displayId, int32_t width, int32_t height, int32_t orientation, bool isActive, const std::string& uniqueId, @@ -1961,24 +1964,24 @@ TEST_F(InputReaderTest, GetKeyCodeState_ForwardsRequestsToSubdeviceMappers) { TEST_F(InputReaderTest, ChangingPointerCaptureNotifiesInputListener) { NotifyPointerCaptureChangedArgs args; - mFakePolicy->setPointerCapture(true); + auto request = mFakePolicy->setPointerCapture(true); mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_TRUE(args.enabled) << "Pointer Capture should be enabled."; + ASSERT_TRUE(args.request.enable) << "Pointer Capture should be enabled."; + ASSERT_EQ(args.request, request) << "Pointer Capture sequence number should match."; mFakePolicy->setPointerCapture(false); mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled."; + ASSERT_FALSE(args.request.enable) << "Pointer Capture should be disabled."; - // Verify that the Pointer Capture state is re-configured correctly when the configuration value + // Verify that the Pointer Capture state is not updated when the configuration value // does not change. mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE); mReader->loopOnce(); - mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled."; + mFakeListener->assertNotifyCaptureWasNotCalled(); } class FakeVibratorInputMapper : public FakeInputMapper { diff --git a/services/inputflinger/tests/TestInputListener.cpp b/services/inputflinger/tests/TestInputListener.cpp index fb7de97b49..6a26c636d9 100644 --- a/services/inputflinger/tests/TestInputListener.cpp +++ b/services/inputflinger/tests/TestInputListener.cpp @@ -100,6 +100,11 @@ void TestInputListener::assertNotifyCaptureWasCalled( "to have been called.")); } +void TestInputListener::assertNotifyCaptureWasNotCalled() { + ASSERT_NO_FATAL_FAILURE(assertNotCalled<NotifyPointerCaptureChangedArgs>( + "notifyPointerCaptureChanged() should not be called.")); +} + template <class NotifyArgsType> void TestInputListener::assertCalled(NotifyArgsType* outEventArgs, std::string message) { std::unique_lock<std::mutex> lock(mLock); diff --git a/services/inputflinger/tests/TestInputListener.h b/services/inputflinger/tests/TestInputListener.h index 0ffcaaa527..0a1dc4b0b9 100644 --- a/services/inputflinger/tests/TestInputListener.h +++ b/services/inputflinger/tests/TestInputListener.h @@ -55,6 +55,7 @@ public: void assertNotifySwitchWasCalled(NotifySwitchArgs* outEventArgs = nullptr); void assertNotifyCaptureWasCalled(NotifyPointerCaptureChangedArgs* outEventArgs = nullptr); + void assertNotifyCaptureWasNotCalled(); void assertNotifySensorWasCalled(NotifySensorArgs* outEventArgs = nullptr); void assertNotifyVibratorStateWasCalled(NotifyVibratorStateArgs* outEventArgs = nullptr); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index e4a777f3c7..6ee13ce508 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -819,11 +819,7 @@ void Layer::setZOrderRelativeOf(const wp<Layer>& relativeOf) { } bool Layer::setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ) { - sp<Handle> handle = static_cast<Handle*>(relativeToHandle.get()); - if (handle == nullptr) { - return false; - } - sp<Layer> relative = handle->owner.promote(); + sp<Layer> relative = fromHandle(relativeToHandle).promote(); if (relative == nullptr) { return false; } @@ -1609,8 +1605,7 @@ void Layer::setChildrenDrawingParent(const sp<Layer>& newParent) { bool Layer::reparent(const sp<IBinder>& newParentHandle) { sp<Layer> newParent; if (newParentHandle != nullptr) { - auto handle = static_cast<Handle*>(newParentHandle.get()); - newParent = handle->owner.promote(); + newParent = fromHandle(newParentHandle).promote(); if (newParent == nullptr) { ALOGE("Unable to promote Layer handle"); return false; @@ -1985,24 +1980,10 @@ void Layer::commitChildList() { mDrawingParent = mCurrentParent; } -static wp<Layer> extractLayerFromBinder(const wp<IBinder>& weakBinderHandle) { - if (weakBinderHandle == nullptr) { - return nullptr; - } - sp<IBinder> binderHandle = weakBinderHandle.promote(); - if (binderHandle == nullptr) { - return nullptr; - } - sp<Layer::Handle> handle = static_cast<Layer::Handle*>(binderHandle.get()); - if (handle == nullptr) { - return nullptr; - } - return handle->owner; -} void Layer::setInputInfo(const InputWindowInfo& info) { mDrawingState.inputInfo = info; - mDrawingState.touchableRegionCrop = extractLayerFromBinder(info.touchableRegionCropHandle); + mDrawingState.touchableRegionCrop = fromHandle(info.touchableRegionCropHandle.promote()); mDrawingState.modified = true; mFlinger->mInputInfoChanged = true; setTransactionFlags(eTransactionNeeded); @@ -2561,6 +2542,23 @@ void Layer::setClonedChild(const sp<Layer>& clonedChild) { mFlinger->mNumClones++; } +const String16 Layer::Handle::kDescriptor = String16("android.Layer.Handle"); + +wp<Layer> Layer::fromHandle(const sp<IBinder>& handleBinder) { + if (handleBinder == nullptr) { + return nullptr; + } + + BBinder* b = handleBinder->localBinder(); + if (b == nullptr || b->getInterfaceDescriptor() != Handle::kDescriptor) { + return nullptr; + } + + // We can safely cast this binder since its local and we verified its interface descriptor. + sp<Handle> handle = static_cast<Handle*>(handleBinder.get()); + return handle->owner; +} + // --------------------------------------------------------------------------- std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 59f5b0dc73..8905548b64 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -289,16 +289,17 @@ public: class LayerCleaner { sp<SurfaceFlinger> mFlinger; sp<Layer> mLayer; + BBinder* mHandle; protected: ~LayerCleaner() { // destroy client resources - mFlinger->onHandleDestroyed(mLayer); + mFlinger->onHandleDestroyed(mHandle, mLayer); } public: - LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer) - : mFlinger(flinger), mLayer(layer) {} + LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer, BBinder* handle) + : mFlinger(flinger), mLayer(layer), mHandle(handle) {} }; /* @@ -312,11 +313,15 @@ public: class Handle : public BBinder, public LayerCleaner { public: Handle(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer) - : LayerCleaner(flinger, layer), owner(layer) {} + : LayerCleaner(flinger, layer, this), owner(layer) {} + const String16& getInterfaceDescriptor() const override { return kDescriptor; } + static const String16 kDescriptor; wp<Layer> owner; }; + static wp<Layer> fromHandle(const sp<IBinder>& handle); + explicit Layer(const LayerCreationArgs& args); virtual ~Layer(); diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp index 0334d70bd5..73b7b6365c 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp @@ -911,7 +911,10 @@ RefreshRateConfigs::KernelIdleTimerAction RefreshRateConfigs::getIdleTimerAction int RefreshRateConfigs::getFrameRateDivider(Fps displayFrameRate, Fps layerFrameRate) { // This calculation needs to be in sync with the java code // in DisplayManagerService.getDisplayInfoForFrameRateOverride - constexpr float kThreshold = 0.1f; + + // The threshold must be smaller than 0.001 in order to differentiate + // between the fractional pairs (e.g. 59.94 and 60). + constexpr float kThreshold = 0.0009f; const auto numPeriods = displayFrameRate.getValue() / layerFrameRate.getValue(); const auto numPeriodsRounded = std::round(numPeriods); if (std::abs(numPeriods - numPeriodsRounded) > kThreshold) { diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index e0b364020b..64ad178b26 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -386,6 +386,13 @@ void Scheduler::dispatchCachedReportedMode() { } const auto modeId = *mFeatures.modeId; + // If the modeId is not the current mode, this means that a + // mode change is in progress. In that case we shouldn't dispatch an event + // as it will be dispatched when the current mode changes. + if (mRefreshRateConfigs.getCurrentRefreshRate().getModeId() != modeId) { + return; + } + const auto vsyncPeriod = mRefreshRateConfigs.getRefreshRateFromModeId(modeId).getVsyncPeriod(); // If there is no change from cached mode, there is no need to dispatch an event diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 230810c936..fe43a2091d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3611,7 +3611,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( sp<Layer> layer = nullptr; if (s.surface) { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } else if (s.hasBufferChanges()) { ALOGW("Transaction with buffer, but no Layer?"); continue; @@ -3778,7 +3778,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp, postTime, permissions, listenerCallbacksWithSurfaces); if ((flags & eAnimation) && state.state.surface) { - if (const auto layer = fromHandleLocked(state.state.surface).promote(); layer) { + if (const auto layer = fromHandle(state.state.surface).promote(); layer) { mScheduler->recordLayerHistory(layer.get(), isAutoTimestamp ? 0 : desiredPresentTime, LayerHistory::LayerUpdateType::AnimationTX); @@ -3933,13 +3933,11 @@ uint32_t SurfaceFlinger::setClientStateLocked( if (what & layer_state_t::eLayerCreated) { layer = handleLayerCreatedLocked(s.surface); if (layer) { - // put the created layer into mLayersByLocalBinderToken. - mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer); flags |= eTransactionNeeded | eTraversalNeeded; mLayersAdded = true; } } else { - layer = fromHandleLocked(s.surface).promote(); + layer = fromHandle(s.surface).promote(); } } else { // The client may provide us a null handle. Treat it as if the layer was removed. @@ -4267,7 +4265,7 @@ status_t SurfaceFlinger::mirrorLayer(const sp<Client>& client, const sp<IBinder> { Mutex::Autolock _l(mStateLock); - mirrorFrom = fromHandleLocked(mirrorFromHandle).promote(); + mirrorFrom = fromHandle(mirrorFromHandle).promote(); if (!mirrorFrom) { return NAME_NOT_FOUND; } @@ -4465,7 +4463,7 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const sp<Layer>& layer) { setTransactionFlags(eTransactionNeeded); } -void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) { +void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) { Mutex::Autolock lock(mStateLock); // If a layer has a parent, we allow it to out-live it's handle // with the idea that the parent holds a reference and will eventually @@ -4476,17 +4474,7 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) { mCurrentState.layersSortedByZ.remove(layer); } markLayerPendingRemovalLocked(layer); - - auto it = mLayersByLocalBinderToken.begin(); - while (it != mLayersByLocalBinderToken.end()) { - if (it->second == layer) { - mBufferCountTracker.remove(it->first->localBinder()); - it = mLayersByLocalBinderToken.erase(it); - } else { - it++; - } - } - + mBufferCountTracker.remove(handle); layer.clear(); } @@ -6089,7 +6077,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, { Mutex::Autolock lock(mStateLock); - parent = fromHandleLocked(args.layerHandle).promote(); + parent = fromHandle(args.layerHandle).promote(); if (parent == nullptr || parent->isRemovedFromCurrentState()) { ALOGE("captureLayers called with an invalid or removed parent"); return NAME_NOT_FOUND; @@ -6120,7 +6108,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY); for (const auto& handle : args.excludeHandles) { - sp<Layer> excludeLayer = fromHandleLocked(handle).promote(); + sp<Layer> excludeLayer = fromHandle(handle).promote(); if (excludeLayer != nullptr) { excludeLayers.emplace(excludeLayer); } else { @@ -6616,24 +6604,8 @@ status_t SurfaceFlinger::getDesiredDisplayModeSpecs(const sp<IBinder>& displayTo } } -wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) { - Mutex::Autolock _l(mStateLock); - return fromHandleLocked(handle); -} - -wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) const { - BBinder* b = nullptr; - if (handle) { - b = handle->localBinder(); - } - if (b == nullptr) { - return nullptr; - } - auto it = mLayersByLocalBinderToken.find(b); - if (it != mLayersByLocalBinderToken.end()) { - return it->second; - } - return nullptr; +wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) const { + return Layer::fromHandle(handle); } void SurfaceFlinger::onLayerFirstRef(Layer* layer) { @@ -6938,7 +6910,7 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) { sp<Layer> parent; bool allowAddRoot = state->addToRoot; if (state->initialParent != nullptr) { - parent = fromHandleLocked(state->initialParent.promote()).promote(); + parent = fromHandle(state->initialParent.promote()).promote(); if (parent == nullptr) { ALOGE("Invalid parent %p", state->initialParent.unsafe_get()); allowAddRoot = false; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 380f444221..c3d837e86c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -327,8 +327,7 @@ public: // Returns nullptr if the handle does not point to an existing layer. // Otherwise, returns a weak reference so that callers off the main-thread // won't accidentally hold onto the last strong reference. - wp<Layer> fromHandle(const sp<IBinder>& handle); - wp<Layer> fromHandleLocked(const sp<IBinder>& handle) const REQUIRES(mStateLock); + wp<Layer> fromHandle(const sp<IBinder>& handle) const; // If set, disables reusing client composition buffers. This can be set by // debug.sf.disable_client_composition_cache @@ -898,7 +897,7 @@ private: // called when all clients have released all their references to // this layer meaning it is entirely safe to destroy all // resources associated to this layer. - void onHandleDestroyed(sp<Layer>& layer); + void onHandleDestroyed(BBinder* handle, sp<Layer>& layer); void markLayerPendingRemovalLocked(const sp<Layer>& layer); // add a layer to SurfaceFlinger @@ -1290,8 +1289,6 @@ private: std::optional<DisplayIdGenerator<HalVirtualDisplayId>> hal; } mVirtualDisplayIdGenerators; - std::unordered_map<BBinder*, wp<Layer>> mLayersByLocalBinderToken GUARDED_BY(mStateLock); - // don't use a lock for these, we don't care int mDebugRegion = 0; bool mDebugDisableHWC = false; diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index 23ab7c8bab..837b2e8924 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -183,12 +183,9 @@ status_t SurfaceInterceptor::writeProtoFileLocked() { return NO_ERROR; } -const sp<const Layer> SurfaceInterceptor::getLayer(const wp<const IBinder>& weakHandle) const { - const sp<const IBinder>& handle(weakHandle.promote()); - const auto layerHandle(static_cast<const Layer::Handle*>(handle.get())); - const sp<const Layer> layer(layerHandle->owner.promote()); - // layer could be a nullptr at this point - return layer; +const sp<const Layer> SurfaceInterceptor::getLayer(const wp<IBinder>& weakHandle) const { + sp<IBinder> handle = weakHandle.promote(); + return Layer::fromHandle(handle).promote(); } int32_t SurfaceInterceptor::getLayerId(const sp<const Layer>& layer) const { @@ -203,12 +200,11 @@ int32_t SurfaceInterceptor::getLayerIdFromWeakRef(const wp<const Layer>& layer) return strongLayer == nullptr ? -1 : getLayerId(strongLayer); } -int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<const IBinder>& handle) const { +int32_t SurfaceInterceptor::getLayerIdFromHandle(const sp<IBinder>& handle) const { if (handle == nullptr) { return -1; } - const auto layerHandle(static_cast<const Layer::Handle*>(handle.get())); - const sp<const Layer> layer(layerHandle->owner.promote()); + const sp<const Layer> layer = Layer::fromHandle(handle).promote(); return layer == nullptr ? -1 : getLayerId(layer); } diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index 673f9e789d..c9555969dc 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -133,10 +133,10 @@ private: void addInitialDisplayStateLocked(Increment* increment, const DisplayDeviceState& display); status_t writeProtoFileLocked(); - const sp<const Layer> getLayer(const wp<const IBinder>& weakHandle) const; + const sp<const Layer> getLayer(const wp<IBinder>& weakHandle) const; int32_t getLayerId(const sp<const Layer>& layer) const; int32_t getLayerIdFromWeakRef(const wp<const Layer>& layer) const; - int32_t getLayerIdFromHandle(const sp<const IBinder>& weakHandle) const; + int32_t getLayerIdFromHandle(const sp<IBinder>& weakHandle) const; Increment* createTraceIncrementLocked(); void addSurfaceCreationLocked(Increment* increment, const sp<const Layer>& layer); diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp index 3423bd590b..3b2bd818b7 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp @@ -2123,7 +2123,11 @@ TEST_F(RefreshRateConfigsTest, getFrameRateDivider) { refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_90); displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps(); EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.5f))); - EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.6f))); + + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(24.f), Fps(25.f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(24.f), Fps(23.976f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(30.f), Fps(29.97f))); + EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(Fps(60.f), Fps(59.94f))); } TEST_F(RefreshRateConfigsTest, getFrameRateOverrides_noLayers) { diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index 271558784e..3ed3eba767 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -1231,6 +1231,12 @@ VkResult CreateSwapchainKHR(VkDevice device, return VK_ERROR_SURFACE_LOST_KHR; } + // In shared mode the num_images must be one regardless of how many + // buffers were allocated for the buffer queue. + if (swapchain_image_usage & VK_SWAPCHAIN_IMAGE_USAGE_SHARED_BIT_ANDROID) { + num_images = 1; + } + int32_t legacy_usage = 0; if (dispatch.GetSwapchainGrallocUsage2ANDROID) { uint64_t consumer_usage, producer_usage; |