diff options
39 files changed, 551 insertions, 188 deletions
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 3a2c769c66..a999c9f22e 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -2414,7 +2414,9 @@ static void SendBroadcast(const std::string& action, const std::vector<std::stri static void Vibrate(int duration_ms) { // clang-format off - RunCommand("", {"cmd", "vibrator", "vibrate", "-f", std::to_string(duration_ms), "dumpstate"}, + std::vector<std::string> args = {"cmd", "vibrator_manager", "synced", "-f", "-d", "dumpstate", + "oneshot", std::to_string(duration_ms)}; + RunCommand("", args, CommandOptions::WithTimeout(10) .Log("Vibrate: '%s'\n") .Always() @@ -2909,6 +2911,10 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, // own activity pushes out interesting data from the trace ring buffer. // The trace file is added to the zip by MaybeAddSystemTraceToZip(). MaybeSnapshotSystemTrace(); + + // If a winscope trace is running, snapshot it now. It will be pulled into bugreport later + // from WMTRACE_DATA_DIR. + MaybeSnapshotWinTrace(); } onUiIntensiveBugreportDumpsFinished(calling_uid); MaybeCheckUserConsent(calling_uid, calling_package); @@ -3020,6 +3026,14 @@ void Dumpstate::MaybeSnapshotSystemTrace() { // file in the later stages. } +void Dumpstate::MaybeSnapshotWinTrace() { + RunCommand( + // Empty name because it's not intended to be classified as a bugreport section. + // Actual tracing files can be found in "/data/misc/wmtrace/" in the bugreport. + "", {"cmd", "window", "tracing", "save-for-bugreport"}, + CommandOptions::WithTimeout(10).Always().DropRoot().RedirectStderr().Build()); +} + void Dumpstate::onUiIntensiveBugreportDumpsFinished(int32_t calling_uid) { if (calling_uid == AID_SHELL || !CalledByApi()) { return; diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index f83968b59e..83e6787ebf 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -549,6 +549,7 @@ class Dumpstate { void MaybeTakeEarlyScreenshot(); void MaybeSnapshotSystemTrace(); + void MaybeSnapshotWinTrace(); void onUiIntensiveBugreportDumpsFinished(int32_t calling_uid); diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp index cefcf6404c..ed31ad9cfa 100644 --- a/cmds/installd/otapreopt.cpp +++ b/cmds/installd/otapreopt.cpp @@ -349,9 +349,6 @@ private: } } - // Clear cached artifacts. - ClearDirectory(isa_path); - // Check whether we have a boot image. // TODO: check that the files are correct wrt/ jars. std::string preopted_boot_art_path = @@ -395,37 +392,6 @@ private: return false; } - static void ClearDirectory(const std::string& dir) { - DIR* c_dir = opendir(dir.c_str()); - if (c_dir == nullptr) { - PLOG(WARNING) << "Unable to open " << dir << " to delete it's contents"; - return; - } - - for (struct dirent* de = readdir(c_dir); de != nullptr; de = readdir(c_dir)) { - const char* name = de->d_name; - if (strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { - continue; - } - // We only want to delete regular files and symbolic links. - std::string file = StringPrintf("%s/%s", dir.c_str(), name); - if (de->d_type != DT_REG && de->d_type != DT_LNK) { - LOG(WARNING) << "Unexpected file " - << file - << " of type " - << std::hex - << de->d_type - << " encountered."; - } else { - // Try to unlink the file. - if (unlink(file.c_str()) != 0) { - PLOG(ERROR) << "Unable to unlink " << file; - } - } - } - CHECK_EQ(0, closedir(c_dir)) << "Unable to close directory."; - } - static const char* ParseNull(const char* arg) { return (strcmp(arg, "!") == 0) ? nullptr : arg; } diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp index fb07840730..379cf92043 100644 --- a/cmds/installd/otapreopt_chroot.cpp +++ b/cmds/installd/otapreopt_chroot.cpp @@ -20,15 +20,19 @@ #include <sys/stat.h> #include <sys/wait.h> +#include <fstream> #include <sstream> +#include <android-base/file.h> #include <android-base/logging.h> #include <android-base/macros.h> #include <android-base/stringprintf.h> +#include <android-base/unique_fd.h> #include <libdm/dm.h> #include <selinux/android.h> #include <apex_file_repository.h> +#include <apex_constants.h> #include <apexd.h> #include "installd_constants.h" @@ -77,6 +81,22 @@ static std::vector<apex::ApexFile> ActivateApexPackages() { return apex::GetActivePackages(); } +static void CreateApexInfoList(const std::vector<apex::ApexFile>& apex_files) { + // Setup the apex-info-list.xml file + const std::string apex_info_file = std::string(apex::kApexRoot) + "/" + apex::kApexInfoList; + std::fstream xml(apex_info_file.c_str(), std::ios::out | std::ios::trunc); + if (!xml.is_open()) { + PLOG(ERROR) << "Failed to open " << apex_info_file; + exit(216); + } + + // we do not care about inactive apexs + std::vector<apex::ApexFile> inactive; + apex::CollectApexInfoList(xml, apex_files, inactive); + xml.flush(); + xml.close(); +} + static void DeactivateApexPackages(const std::vector<apex::ApexFile>& active_packages) { for (const apex::ApexFile& apex_file : active_packages) { const std::string& package_path = apex_file.GetPath(); @@ -185,6 +205,13 @@ static int otapreopt_chroot(const int argc, char **arg) { // want it for product APKs. Same notes as vendor above. TryExtraMount("product", arg[2], "/postinstall/product"); + constexpr const char* kPostInstallLinkerconfig = "/postinstall/linkerconfig"; + // Try to mount /postinstall/linkerconfig. we will set it up after performing the chroot + if (mount("tmpfs", kPostInstallLinkerconfig, "tmpfs", 0, nullptr) != 0) { + PLOG(ERROR) << "Failed to mount a tmpfs for " << kPostInstallLinkerconfig; + exit(215); + } + // Setup APEX mount point and its security context. static constexpr const char* kPostinstallApexDir = "/postinstall/apex"; // The following logic is similar to the one in system/core/rootdir/init.rc: @@ -243,17 +270,37 @@ static int otapreopt_chroot(const int argc, char **arg) { // Try to mount APEX packages in "/apex" in the chroot dir. We need at least // the ART APEX, as it is required by otapreopt to run dex2oat. std::vector<apex::ApexFile> active_packages = ActivateApexPackages(); + CreateApexInfoList(active_packages); // Check that an ART APEX has been activated; clean up and exit // early otherwise. - if (std::none_of(active_packages.begin(), - active_packages.end(), - [](const apex::ApexFile& package){ - return package.GetManifest().name() == "com.android.art"; - })) { - LOG(FATAL_WITHOUT_ABORT) << "No activated com.android.art APEX package."; - DeactivateApexPackages(active_packages); - exit(217); + static constexpr const std::string_view kRequiredApexs[] = { + "com.android.art", + "com.android.runtime", + }; + for (std::string_view apex : kRequiredApexs) { + if (std::none_of(active_packages.begin(), active_packages.end(), + [&](const apex::ApexFile& package) { + return package.GetManifest().name() == apex; + })) { + LOG(FATAL_WITHOUT_ABORT) << "No activated " << apex << " APEX package."; + DeactivateApexPackages(active_packages); + exit(217); + } + } + + // Setup /linkerconfig. Doing it after the chroot means it doesn't need its own category + if (selinux_android_restorecon("/linkerconfig", 0) < 0) { + PLOG(ERROR) << "Failed to restorecon /linkerconfig"; + exit(219); + } + std::vector<std::string> linkerconfig_cmd{"/apex/com.android.runtime/bin/linkerconfig", + "--target", "/linkerconfig"}; + std::string linkerconfig_error_msg; + bool linkerconfig_exec_result = Exec(linkerconfig_cmd, &linkerconfig_error_msg); + if (!linkerconfig_exec_result) { + LOG(ERROR) << "Running linkerconfig failed: " << linkerconfig_error_msg; + exit(218); } // Now go on and run otapreopt. diff --git a/libs/binder/BufferedTextOutput.cpp b/libs/binder/BufferedTextOutput.cpp index 88c85bfadc..349658ecfa 100644 --- a/libs/binder/BufferedTextOutput.cpp +++ b/libs/binder/BufferedTextOutput.cpp @@ -15,7 +15,6 @@ */ #include "BufferedTextOutput.h" -#include <binder/Debug.h> #include <cutils/atomic.h> #include <utils/Log.h> @@ -26,6 +25,7 @@ #include <stdio.h> #include <stdlib.h> +#include "Debug.h" #include "Static.h" // --------------------------------------------------------------------------- diff --git a/libs/binder/Debug.cpp b/libs/binder/Debug.cpp index 3a620590ab..e4ac4b49a4 100644 --- a/libs/binder/Debug.cpp +++ b/libs/binder/Debug.cpp @@ -14,7 +14,8 @@ * limitations under the License. */ -#include <binder/Debug.h> +#include "Debug.h" + #include <binder/ProcessState.h> #include <utils/misc.h> diff --git a/libs/binder/include/binder/Debug.h b/libs/binder/Debug.h index ac71e003c4..ac71e003c4 100644 --- a/libs/binder/include/binder/Debug.h +++ b/libs/binder/Debug.h diff --git a/libs/binder/TextOutput.cpp b/libs/binder/TextOutput.cpp index 684a7dcc51..a0ade50efb 100644 --- a/libs/binder/TextOutput.cpp +++ b/libs/binder/TextOutput.cpp @@ -16,7 +16,7 @@ #include <binder/TextOutput.h> -#include <binder/Debug.h> +#include "Debug.h" #include <utils/String8.h> #include <utils/String16.h> diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs index 4492cf72f4..2598ebc804 100644 --- a/libs/binder/rust/src/error.rs +++ b/libs/binder/rust/src/error.rs @@ -77,9 +77,7 @@ fn parse_exception_code(code: i32) -> ExceptionCode { e if e == ExceptionCode::ILLEGAL_ARGUMENT as i32 => ExceptionCode::ILLEGAL_ARGUMENT, e if e == ExceptionCode::NULL_POINTER as i32 => ExceptionCode::NULL_POINTER, e if e == ExceptionCode::ILLEGAL_STATE as i32 => ExceptionCode::ILLEGAL_STATE, - e if e == ExceptionCode::NETWORK_MAIN_THREAD as i32 => { - ExceptionCode::NETWORK_MAIN_THREAD - } + e if e == ExceptionCode::NETWORK_MAIN_THREAD as i32 => ExceptionCode::NETWORK_MAIN_THREAD, e if e == ExceptionCode::UNSUPPORTED_OPERATION as i32 => { ExceptionCode::UNSUPPORTED_OPERATION } @@ -96,6 +94,16 @@ fn parse_exception_code(code: i32) -> ExceptionCode { /// Used in AIDL transactions to represent failed transactions. pub struct Status(*mut sys::AStatus); +// Safety: The `AStatus` that the `Status` points to must have an entirely thread-safe API for the +// duration of the `Status` object's lifetime. We ensure this by not allowing mutation of a `Status` +// in Rust, and the NDK API says we're the owner of our `AStatus` objects so outside code should not +// be mutating them underneath us. +unsafe impl Sync for Status {} + +// Safety: `Status` always contains an owning pointer to a global, immutable, interned `AStatus`. +// A thread-local `AStatus` would not be valid. +unsafe impl Send for Status {} + impl Status { /// Create a status object representing a successful transaction. pub fn ok() -> Self { diff --git a/libs/binder/tests/binderTextOutputTest.cpp b/libs/binder/tests/binderTextOutputTest.cpp index ce99f59d7c..b37030e7d7 100644 --- a/libs/binder/tests/binderTextOutputTest.cpp +++ b/libs/binder/tests/binderTextOutputTest.cpp @@ -26,7 +26,6 @@ #include <binder/Parcel.h> #include <binder/TextOutput.h> -#include <binder/Debug.h> static void CheckMessage(CapturedStderr& cap, const char* expected, diff --git a/libs/binder/tests/fuzzers/TextOutputFuzz.cpp b/libs/binder/tests/fuzzers/TextOutputFuzz.cpp index c9500205df..5e3502aace 100644 --- a/libs/binder/tests/fuzzers/TextOutputFuzz.cpp +++ b/libs/binder/tests/fuzzers/TextOutputFuzz.cpp @@ -16,7 +16,6 @@ #include <fuzzer/FuzzedDataProvider.h> -#include <binder/Debug.h> #include <binder/Parcel.h> #include <binder/TextOutput.h> #include "android-base/file.h" diff --git a/libs/graphicsenv/OWNERS b/libs/graphicsenv/OWNERS index c0bb75f982..8c284647f2 100644 --- a/libs/graphicsenv/OWNERS +++ b/libs/graphicsenv/OWNERS @@ -1,6 +1,4 @@ chrisforbes@google.com cnorthrop@google.com -courtneygo@google.com lpy@google.com timvp@google.com -zzyiwei@google.com diff --git a/libs/input/android/os/IInputConstants.aidl b/libs/input/android/os/IInputConstants.aidl index bce0ec8367..4b90844490 100644 --- a/libs/input/android/os/IInputConstants.aidl +++ b/libs/input/android/os/IInputConstants.aidl @@ -20,7 +20,10 @@ package android.os; /** @hide */ interface IInputConstants { - const int DEFAULT_DISPATCHING_TIMEOUT_MILLIS = 5000; // 5 seconds + // This should be multiplied by the value of the system property ro.hw_timeout_multiplier before + // use. A pre-multiplied constant is available in Java in + // android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS. + const int UNMULTIPLIED_DEFAULT_DISPATCHING_TIMEOUT_MILLIS = 5000; // 5 seconds // Compatibility changes. /** diff --git a/libs/renderengine/skia/filters/BlurFilter.cpp b/libs/renderengine/skia/filters/BlurFilter.cpp index 5960e48f7b..ec710d97c3 100644 --- a/libs/renderengine/skia/filters/BlurFilter.cpp +++ b/libs/renderengine/skia/filters/BlurFilter.cpp @@ -36,13 +36,18 @@ BlurFilter::BlurFilter() { SkString blurString(R"( in shader input; uniform float2 in_blurOffset; + uniform float2 in_maxSizeXY; half4 main(float2 xy) { half4 c = sample(input, xy); - c += sample(input, xy + float2( in_blurOffset.x, in_blurOffset.y)); - c += sample(input, xy + float2( in_blurOffset.x, -in_blurOffset.y)); - c += sample(input, xy + float2(-in_blurOffset.x, in_blurOffset.y)); - c += sample(input, xy + float2(-in_blurOffset.x, -in_blurOffset.y)); + c += sample(input, float2( clamp( in_blurOffset.x + xy.x, 0, in_maxSizeXY.x), + clamp(in_blurOffset.y + xy.y, 0, in_maxSizeXY.y))); + c += sample(input, float2( clamp( in_blurOffset.x + xy.x, 0, in_maxSizeXY.x), + clamp(-in_blurOffset.y + xy.y, 0, in_maxSizeXY.y))); + c += sample(input, float2( clamp( -in_blurOffset.x + xy.x, 0, in_maxSizeXY.x), + clamp(in_blurOffset.y + xy.y, 0, in_maxSizeXY.y))); + c += sample(input, float2( clamp( -in_blurOffset.x + xy.x, 0, in_maxSizeXY.x), + clamp(-in_blurOffset.y + xy.y, 0, in_maxSizeXY.y))); return half4(c.rgb * 0.2, 1.0); } @@ -99,6 +104,8 @@ sk_sp<SkImage> BlurFilter::generate(GrRecordingContext* context, const uint32_t blurBuilder.child("input") = input->makeShader(SkTileMode::kClamp, SkTileMode::kClamp, linear, blurMatrix); blurBuilder.uniform("in_blurOffset") = SkV2{stepX * kInputScale, stepY * kInputScale}; + blurBuilder.uniform("in_maxSizeXY") = + SkV2{blurRect.width() * kInputScale - 1, blurRect.height() * kInputScale - 1}; sk_sp<SkImage> tmpBlur(blurBuilder.makeImage(context, nullptr, scaledInfo, false)); @@ -108,6 +115,8 @@ sk_sp<SkImage> BlurFilter::generate(GrRecordingContext* context, const uint32_t blurBuilder.child("input") = tmpBlur->makeShader(SkTileMode::kClamp, SkTileMode::kClamp, linear); blurBuilder.uniform("in_blurOffset") = SkV2{stepX * stepScale, stepY * stepScale}; + blurBuilder.uniform("in_maxSizeXY") = + SkV2{blurRect.width() * kInputScale - 1, blurRect.height() * kInputScale - 1}; tmpBlur = blurBuilder.makeImage(context, nullptr, scaledInfo, false); } diff --git a/opengl/OWNERS b/opengl/OWNERS index b505712eb5..a9bd4bb2e2 100644 --- a/opengl/OWNERS +++ b/opengl/OWNERS @@ -1,7 +1,6 @@ chrisforbes@google.com cnorthrop@google.com -courtneygo@google.com ianelliott@google.com jessehall@google.com lpy@google.com -zzyiwei@google.com +timvp@google.com diff --git a/services/gpuservice/OWNERS b/services/gpuservice/OWNERS index 5d028393f6..ac300d009e 100644 --- a/services/gpuservice/OWNERS +++ b/services/gpuservice/OWNERS @@ -1,3 +1,2 @@ chrisforbes@google.com lpy@google.com -zzyiwei@google.com diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 465e8be24b..19f8694a5e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -48,6 +48,7 @@ static constexpr bool DEBUG_TOUCH_OCCLUSION = true; #define DEBUG_HOVER 0 #include <android-base/chrono_utils.h> +#include <android-base/properties.h> #include <android-base/stringprintf.h> #include <android/os/IInputConstants.h> #include <binder/Binder.h> @@ -78,6 +79,7 @@ static constexpr bool DEBUG_TOUCH_OCCLUSION = true; #define INDENT3 " " #define INDENT4 " " +using android::base::HwTimeoutMultiplier; using android::base::StringPrintf; using android::os::BlockUntrustedTouchesMode; using android::os::IInputConstants; @@ -89,8 +91,9 @@ namespace android::inputdispatcher { // Default input dispatching timeout if there is no focused application or paused window // from which to determine an appropriate dispatching timeout. -constexpr std::chrono::duration DEFAULT_INPUT_DISPATCHING_TIMEOUT = - std::chrono::milliseconds(android::os::IInputConstants::DEFAULT_DISPATCHING_TIMEOUT_MILLIS); +const std::chrono::duration DEFAULT_INPUT_DISPATCHING_TIMEOUT = std::chrono::milliseconds( + android::os::IInputConstants::UNMULTIPLIED_DEFAULT_DISPATCHING_TIMEOUT_MILLIS * + HwTimeoutMultiplier()); // Amount of time to allow for all pending events to be processed when an app switch // key is on the way. This is used to preempt input dispatch and drop input events diff --git a/services/inputflinger/docs/anr.md b/services/inputflinger/docs/anr.md index ce64fe9224..5b931d678f 100644 --- a/services/inputflinger/docs/anr.md +++ b/services/inputflinger/docs/anr.md @@ -22,7 +22,7 @@ Every dispatch cycle, InputDispatcher will check all connections to see if any a When a dispatch entry is sent to the app, its `deliveryTime` and `timeoutTime` fields are populated. The `deliveryTime` is the time that the event is delivered to the app. This is simply the current time inside `publishMotionEvent`. The `timeoutTime` is the time when this entry would be considered overdue. At that time, the ANR process would start for this connection. -Most connections are associated with a window, and each window may have a custom timeout time. To calculate the timeout time of a specific event, simply add the `window.dispatchingTimeout` to the current time. In case where there is no associated window, such as gesture monitors, use the default dispatching timeout which is defined in `IInputConstants::DEFAULT_DISPATCHING_TIMEOUT_MILLIS`. +Most connections are associated with a window, and each window may have a custom timeout time. To calculate the timeout time of a specific event, simply add the `window.dispatchingTimeout` to the current time. In case where there is no associated window, such as gesture monitors, use the default dispatching timeout which is defined in `android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS`. The `timeoutTime` field of the `DispatchEntry` is needed because the window associated with a specific connection may change its timeout time. Therefore, entries sent prior to the timeout change would need to follow the previous timeout value. If a window timeout changes, it only affects new events being dispatched, and does not alter the timeout times of events already sent to the app. diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index a96dffdcb7..eac3d95d8a 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -424,12 +424,12 @@ bool BufferLayer::shouldPresentNow(nsecs_t expectedPresentTime) const { return isBufferDue(expectedPresentTime); } -bool BufferLayer::frameIsEarly(nsecs_t expectedPresentTime) const { +bool BufferLayer::frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const { // TODO(b/169901895): kEarlyLatchVsyncThreshold should be based on the // vsync period. We can do this change as soon as ag/13100772 is merged. constexpr static std::chrono::nanoseconds kEarlyLatchVsyncThreshold = 5ms; - const auto presentTime = nextPredictedPresentTime(); + const auto presentTime = nextPredictedPresentTime(vsyncId); if (!presentTime.has_value()) { return false; } diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h index 2118f4ad18..d215298eda 100644 --- a/services/surfaceflinger/BufferLayer.h +++ b/services/surfaceflinger/BufferLayer.h @@ -191,7 +191,7 @@ protected: // Returns true if the next frame is considered too early to present // at the given expectedPresentTime - bool frameIsEarly(nsecs_t expectedPresentTime) const; + bool frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId) const; std::atomic<bool> mAutoRefresh{false}; std::atomic<bool> mSidebandStreamChanged{false}; @@ -238,7 +238,7 @@ private: FloatRect computeSourceBounds(const FloatRect& parentBounds) const override; // Returns the predicted present time of the next frame if available - virtual std::optional<nsecs_t> nextPredictedPresentTime() const = 0; + virtual std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const = 0; // The amount of time SF can delay a frame if it is considered early based // on the VsyncModulator::VsyncConfig::appWorkDuration diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 3615a0209f..63dd25f72f 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -215,7 +215,7 @@ bool BufferQueueLayer::hasFrameUpdate() const { return mQueuedFrames > 0; } -std::optional<nsecs_t> BufferQueueLayer::nextPredictedPresentTime() const { +std::optional<nsecs_t> BufferQueueLayer::nextPredictedPresentTime(int64_t /*vsyncId*/) const { Mutex::Autolock lock(mQueueItemLock); if (mQueueItems.empty()) { return std::nullopt; diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h index 0ea02e19a7..3a34b952f2 100644 --- a/services/surfaceflinger/BufferQueueLayer.h +++ b/services/surfaceflinger/BufferQueueLayer.h @@ -117,7 +117,7 @@ private: // Temporary - Used only for LEGACY camera mode. uint32_t getProducerStickyTransform() const; - std::optional<nsecs_t> nextPredictedPresentTime() const override; + std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const override; sp<BufferLayerConsumer> mConsumer; sp<IGraphicBufferProducer> mProducer; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 296085a0b5..e017cdc8c0 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -171,6 +171,14 @@ void BufferStateLayer::onLayerDisplayed(const sp<Fence>& releaseFence) { void BufferStateLayer::onSurfaceFrameCreated( const std::shared_ptr<frametimeline::SurfaceFrame>& surfaceFrame) { + while (mPendingJankClassifications.size() >= kPendingClassificationMaxSurfaceFrames) { + // Too many SurfaceFrames pending classification. The front of the deque is probably not + // tracked by FrameTimeline and will never be presented. This will only result in a memory + // leak. + ALOGW("Removing the front of pending jank deque from layer - %s to prevent memory leak", + mName.c_str()); + mPendingJankClassifications.pop_front(); + } mPendingJankClassifications.emplace_back(surfaceFrame); } @@ -605,13 +613,14 @@ bool BufferStateLayer::hasFrameUpdate() const { return mCurrentStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr); } -std::optional<nsecs_t> BufferStateLayer::nextPredictedPresentTime() const { - const State& drawingState(getDrawingState()); - if (!drawingState.isAutoTimestamp || !drawingState.bufferSurfaceFrameTX) { +std::optional<nsecs_t> BufferStateLayer::nextPredictedPresentTime(int64_t vsyncId) const { + const auto prediction = + mFlinger->mFrameTimeline->getTokenManager()->getPredictionsForToken(vsyncId); + if (!prediction.has_value()) { return std::nullopt; } - return drawingState.bufferSurfaceFrameTX->getPredictions().presentTime; + return prediction->presentTime; } status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime, diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 6b422ea204..003edd5280 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -156,7 +156,7 @@ private: bool bufferNeedsFiltering() const override; - std::optional<nsecs_t> nextPredictedPresentTime() const override; + std::optional<nsecs_t> nextPredictedPresentTime(int64_t vsyncId) const override; static const std::array<float, 16> IDENTITY_MATRIX; @@ -174,6 +174,8 @@ private: nsecs_t mCallbackHandleAcquireTime = -1; std::deque<std::shared_ptr<android::frametimeline::SurfaceFrame>> mPendingJankClassifications; + // An upper bound on the number of SurfaceFrames in the pending classifications deque. + static constexpr int kPendingClassificationMaxSurfaceFrames = 25; const std::string mBlastTransactionName{"BufferTX - " + mName}; // This integer is incremented everytime a buffer arrives at the server for this layer, diff --git a/services/surfaceflinger/EffectLayer.cpp b/services/surfaceflinger/EffectLayer.cpp index 44d4d756e0..caef33848e 100644 --- a/services/surfaceflinger/EffectLayer.cpp +++ b/services/surfaceflinger/EffectLayer.cpp @@ -77,7 +77,7 @@ std::vector<compositionengine::LayerFE::LayerSettings> EffectLayer::prepareClien } bool EffectLayer::isVisible() const { - return !isHiddenByPolicy() && getAlpha() > 0.0_hf && hasSomethingToDraw(); + return !isHiddenByPolicy() && (getAlpha() > 0.0_hf || hasBlur()) && hasSomethingToDraw(); } bool EffectLayer::setColor(const half3& color) { diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 03e38f30de..344270603e 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -444,37 +444,26 @@ void SurfaceFrame::dump(std::string& result, const std::string& indent, nsecs_t dumpTable(result, mPredictions, mActuals, indent, mPredictionState, baseTime); } -void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate, - nsecs_t displayDeadlineDelta, nsecs_t displayPresentDelta) { - std::scoped_lock lock(mMutex); - - if (mPresentState != PresentState::Presented) { - // No need to update dropped buffers +void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, + nsecs_t& deadlineDelta) { + if (mPredictionState == PredictionState::Expired || + mActuals.presentTime == Fence::SIGNAL_TIME_INVALID) { + // Cannot do any classification for invalid present time. + // For prediction expired case, we do not know what happened here to classify this + // correctly. This could potentially be AppDeadlineMissed but that's assuming no app will + // request frames 120ms apart. + mJankType = JankType::Unknown; + deadlineDelta = -1; return; } - mActuals.presentTime = presentTime; - // Jank Analysis for SurfaceFrame if (mPredictionState == PredictionState::None) { // Cannot do jank classification on frames that don't have a token. return; } - if (mPredictionState == PredictionState::Expired) { - // We do not know what happened here to classify this correctly. This could - // potentially be AppDeadlineMissed but that's assuming no app will request frames - // 120ms apart. - mJankType = JankType::Unknown; - mFramePresentMetadata = FramePresentMetadata::UnknownPresent; - mFrameReadyMetadata = FrameReadyMetadata::UnknownFinish; - const constexpr nsecs_t kAppDeadlineDelta = -1; - mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName, - mJankType, displayDeadlineDelta, displayPresentDelta, - kAppDeadlineDelta}); - return; - } + deadlineDelta = mActuals.endTime - mPredictions.endTime; const nsecs_t presentDelta = mActuals.presentTime - mPredictions.presentTime; - const nsecs_t deadlineDelta = mActuals.endTime - mPredictions.endTime; const nsecs_t deltaToVsync = refreshRate.getPeriodNsecs() > 0 ? std::abs(presentDelta) % refreshRate.getPeriodNsecs() : 0; @@ -558,8 +547,28 @@ void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, } } } - mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName, mJankType, - displayDeadlineDelta, displayPresentDelta, deadlineDelta}); +} + +void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate, + nsecs_t displayDeadlineDelta, nsecs_t displayPresentDelta) { + std::scoped_lock lock(mMutex); + + if (mPresentState != PresentState::Presented) { + // No need to update dropped buffers + return; + } + + mActuals.presentTime = presentTime; + nsecs_t deadlineDelta = 0; + + classifyJankLocked(displayFrameJankType, refreshRate, deadlineDelta); + + if (mPredictionState != PredictionState::None) { + // Only update janky frames if the app used vsync predictions + mTimeStats->incrementJankyFrames({refreshRate, mRenderRate, mOwnerUid, mLayerName, + mJankType, displayDeadlineDelta, displayPresentDelta, + deadlineDelta}); + } } void SurfaceFrame::tracePredictions(int64_t displayFrameToken) const { @@ -826,25 +835,28 @@ void FrameTimeline::DisplayFrame::setActualEndTime(nsecs_t actualEndTime) { mSurfaceFlingerActuals.endTime = actualEndTime; } -void FrameTimeline::DisplayFrame::onPresent(nsecs_t signalTime) { - mSurfaceFlingerActuals.presentTime = signalTime; - if (mPredictionState == PredictionState::Expired) { - // Cannot do jank classification with expired predictions +void FrameTimeline::DisplayFrame::classifyJank(nsecs_t& deadlineDelta, nsecs_t& deltaToVsync) { + if (mPredictionState == PredictionState::Expired || + mSurfaceFlingerActuals.presentTime == Fence::SIGNAL_TIME_INVALID) { + // Cannot do jank classification with expired predictions or invalid signal times. mJankType = JankType::Unknown; + deadlineDelta = -1; + deltaToVsync = -1; return; } // Delta between the expected present and the actual present const nsecs_t presentDelta = mSurfaceFlingerActuals.presentTime - mSurfaceFlingerPredictions.presentTime; - const nsecs_t deadlineDelta = + deadlineDelta = mSurfaceFlingerActuals.endTime - (mSurfaceFlingerPredictions.endTime - mHwcDuration); // How far off was the presentDelta when compared to the vsyncPeriod. Used in checking if there // was a prediction error or not. - nsecs_t deltaToVsync = mRefreshRate.getPeriodNsecs() > 0 + deltaToVsync = mRefreshRate.getPeriodNsecs() > 0 ? std::abs(presentDelta) % mRefreshRate.getPeriodNsecs() : 0; + if (std::abs(presentDelta) > mJankClassificationThresholds.presentThreshold) { mFramePresentMetadata = presentDelta > 0 ? FramePresentMetadata::LatePresent : FramePresentMetadata::EarlyPresent; @@ -922,6 +934,14 @@ void FrameTimeline::DisplayFrame::onPresent(nsecs_t signalTime) { mJankType = JankType::Unknown; } } +} + +void FrameTimeline::DisplayFrame::onPresent(nsecs_t signalTime) { + mSurfaceFlingerActuals.presentTime = signalTime; + nsecs_t deadlineDelta = 0; + nsecs_t deltaToVsync = 0; + classifyJank(deadlineDelta, deltaToVsync); + for (auto& surfaceFrame : mSurfaceFrames) { surfaceFrame->onPresent(signalTime, mJankType, mRefreshRate, deadlineDelta, deltaToVsync); } @@ -1084,11 +1104,9 @@ void FrameTimeline::flushPendingPresentFences() { continue; } } - if (signalTime != Fence::SIGNAL_TIME_INVALID) { - auto& displayFrame = pendingPresentFence.second; - displayFrame->onPresent(signalTime); - displayFrame->trace(mSurfaceFlingerPid); - } + auto& displayFrame = pendingPresentFence.second; + displayFrame->onPresent(signalTime); + displayFrame->trace(mSurfaceFlingerPid); mPendingPresentFences.erase(mPendingPresentFences.begin() + static_cast<int>(i)); --i; diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 7c6a0cc866..b66e02afe1 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -216,6 +216,8 @@ public: private: void tracePredictions(int64_t displayFrameToken) const; void traceActuals(int64_t displayFrameToken) const; + void classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, + nsecs_t& deadlineDelta) REQUIRES(mMutex); const int64_t mToken; const int32_t mInputEventId; @@ -355,7 +357,7 @@ public: // Sets the token, vsyncPeriod, predictions and SF start time. void onSfWakeUp(int64_t token, Fps refreshRate, std::optional<TimelineItem> predictions, nsecs_t wakeUpTime); - // Sets the appropriate metadata, classifies the jank and returns the classified jankType. + // Sets the appropriate metadata and classifies the jank. void onPresent(nsecs_t signalTime); // Adds the provided SurfaceFrame to the current display frame. void addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame); @@ -383,6 +385,7 @@ public: void dump(std::string& result, nsecs_t baseTime) const; void tracePredictions(pid_t surfaceFlingerPid) const; void traceActuals(pid_t surfaceFlingerPid) const; + void classifyJank(nsecs_t& deadlineDelta, nsecs_t& deltaToVsync); int64_t mToken = FrameTimelineInfo::INVALID_VSYNC_ID; diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index ce97155eaa..34a9f39b6d 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -922,7 +922,9 @@ public: pid_t getOwnerPid() { return mOwnerPid; } - virtual bool frameIsEarly(nsecs_t /*expectedPresentTime*/) const { return false; } + virtual bool frameIsEarly(nsecs_t /*expectedPresentTime*/, int64_t /*vsyncId*/) const { + return false; + } // This layer is not a clone, but it's the parent to the cloned hierarchy. The // variable mClonedChild represents the top layer that will be cloned so this diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 727386c859..593855ec17 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3313,10 +3313,10 @@ void SurfaceFlinger::flushTransactionQueues() { while (!transactionQueue.empty()) { const auto& transaction = transactionQueue.front(); - if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp, + if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo, + transaction.isAutoTimestamp, transaction.desiredPresentTime, - transaction.states, - pendingBuffers)) { + transaction.states, pendingBuffers)) { setTransactionFlags(eTransactionFlushNeeded); break; } @@ -3340,10 +3340,10 @@ void SurfaceFlinger::flushTransactionQueues() { const auto& transaction = mTransactionQueue.front(); bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) != mPendingTransactionQueues.end(); - if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp, + if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo, + transaction.isAutoTimestamp, transaction.desiredPresentTime, - transaction.states, - pendingBuffers) || + transaction.states, pendingBuffers) || pendingTransactions) { mPendingTransactionQueues[transaction.applyToken].push(transaction); } else { @@ -3373,7 +3373,8 @@ bool SurfaceFlinger::transactionFlushNeeded() { } bool SurfaceFlinger::transactionIsReadyToBeApplied( - bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states, + const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime, + const Vector<ComposerState>& states, std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) { const nsecs_t expectedPresentTime = mExpectedPresentTime.load(); bool ready = true; @@ -3386,24 +3387,26 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( for (const ComposerState& state : states) { const layer_state_t& s = state.state; - if (!(s.what & layer_state_t::eAcquireFenceChanged)) { - continue; - } - if (s.acquireFence && s.acquireFence->getStatus() == Fence::Status::Unsignaled) { + const bool acquireFenceChanged = (s.what & layer_state_t::eAcquireFenceChanged); + if (acquireFenceChanged && s.acquireFence && + s.acquireFence->getStatus() == Fence::Status::Unsignaled) { ready = false; } sp<Layer> layer = nullptr; if (s.surface) { layer = fromHandleLocked(s.surface).promote(); - } else { + } else if (acquireFenceChanged) { ALOGW("Transaction with buffer, but no Layer?"); continue; } if (!layer) { continue; } - if (layer->frameIsEarly(expectedPresentTime)) { + + const bool frameTimelineInfoChanged = (s.what & layer_state_t::eFrameTimelineInfoChanged); + const auto vsyncId = frameTimelineInfoChanged ? s.frameTimelineInfo.vsyncId : info.vsyncId; + if (isAutoTimestamp && layer->frameIsEarly(expectedPresentTime, vsyncId)) { ATRACE_NAME("frameIsEarly()"); return false; } @@ -3413,11 +3416,14 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( ready = false; } - // If backpressure is enabled and we already have a buffer to commit, keep the transaction - // in the queue. - const bool hasPendingBuffer = pendingBuffers.find(s.surface) != pendingBuffers.end(); - if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) { - ready = false; + if (acquireFenceChanged) { + // If backpressure is enabled and we already have a buffer to commit, keep the + // transaction in the queue. + const bool hasPendingBuffer = pendingBuffers.find(s.surface) != pendingBuffers.end(); + if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) { + ready = false; + } + pendingBuffers.insert(s.surface); } pendingBuffers.insert(s.surface); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 8e69eadb54..40d63b22b6 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -800,7 +800,8 @@ private: void commitTransaction() REQUIRES(mStateLock); void commitOffscreenLayers(); bool transactionIsReadyToBeApplied( - bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states, + const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime, + const Vector<ComposerState>& states, std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) REQUIRES(mStateLock); uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock); diff --git a/services/surfaceflinger/TimeStats/OWNERS b/services/surfaceflinger/TimeStats/OWNERS index 1441f91489..ded3ebbe05 100644 --- a/services/surfaceflinger/TimeStats/OWNERS +++ b/services/surfaceflinger/TimeStats/OWNERS @@ -1,2 +1 @@ alecmouri@google.com -zzyiwei@google.com diff --git a/services/surfaceflinger/tests/EffectLayer_test.cpp b/services/surfaceflinger/tests/EffectLayer_test.cpp index fafb49efba..7a3c45dbba 100644 --- a/services/surfaceflinger/tests/EffectLayer_test.cpp +++ b/services/surfaceflinger/tests/EffectLayer_test.cpp @@ -111,6 +111,72 @@ TEST_F(EffectLayerTest, EffectLayerCanSetColor) { } } +TEST_F(EffectLayerTest, BlurEffectLayerIsVisible) { + if (!deviceSupportsBlurs()) GTEST_SKIP(); + if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP(); + + const auto canvasSize = 256; + + sp<SurfaceControl> leftLayer = createColorLayer("Left", Color::BLUE); + sp<SurfaceControl> rightLayer = createColorLayer("Right", Color::GREEN); + sp<SurfaceControl> blurLayer; + const auto leftRect = Rect(0, 0, canvasSize / 2, canvasSize); + const auto rightRect = Rect(canvasSize / 2, 0, canvasSize, canvasSize); + const auto blurRect = Rect(0, 0, canvasSize, canvasSize); + + asTransaction([&](Transaction& t) { + t.setLayer(leftLayer, mLayerZBase + 1); + t.reparent(leftLayer, mParentLayer); + t.setCrop_legacy(leftLayer, leftRect); + t.setLayer(rightLayer, mLayerZBase + 2); + t.reparent(rightLayer, mParentLayer); + t.setCrop_legacy(rightLayer, rightRect); + t.show(leftLayer); + t.show(rightLayer); + }); + + { + auto shot = screenshot(); + shot->expectColor(leftRect, Color::BLUE); + shot->expectColor(rightRect, Color::GREEN); + } + + ASSERT_NO_FATAL_FAILURE(blurLayer = createColorLayer("BackgroundBlur", Color::TRANSPARENT)); + + const auto blurRadius = canvasSize / 2; + asTransaction([&](Transaction& t) { + t.setLayer(blurLayer, mLayerZBase + 3); + t.reparent(blurLayer, mParentLayer); + t.setBackgroundBlurRadius(blurLayer, blurRadius); + t.setCrop_legacy(blurLayer, blurRect); + t.setFrame(blurLayer, blurRect); + t.setAlpha(blurLayer, 0.0f); + t.show(blurLayer); + }); + + { + auto shot = screenshot(); + + const auto stepSize = 1; + const auto blurAreaOffset = blurRadius * 0.7f; + const auto blurAreaStartX = canvasSize / 2 - blurRadius + blurAreaOffset; + const auto blurAreaEndX = canvasSize / 2 + blurRadius - blurAreaOffset; + Color previousColor; + Color currentColor; + for (int y = 0; y < canvasSize; y++) { + shot->checkPixel(0, y, /* r = */ 0, /* g = */ 0, /* b = */ 255); + previousColor = shot->getPixelColor(0, y); + for (int x = blurAreaStartX; x < blurAreaEndX; x += stepSize) { + currentColor = shot->getPixelColor(x, y); + ASSERT_GT(currentColor.g, previousColor.g); + ASSERT_LT(currentColor.b, previousColor.b); + ASSERT_EQ(0, currentColor.r); + } + shot->checkPixel(canvasSize - 1, y, 0, 255, 0); + } + } +} + } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp index 6d28e621ac..aa1cce2586 100644 --- a/services/surfaceflinger/tests/LayerCallback_test.cpp +++ b/services/surfaceflinger/tests/LayerCallback_test.cpp @@ -18,6 +18,10 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" +#include <sys/epoll.h> + +#include <gui/DisplayEventReceiver.h> + #include "LayerTransactionTest.h" #include "utils/CallbackUtils.h" @@ -30,6 +34,24 @@ using android::hardware::graphics::common::V1_1::BufferUsage; class LayerCallbackTest : public LayerTransactionTest { public: + void SetUp() override { + LayerTransactionTest::SetUp(); + + EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.initCheck()); + + mEpollFd = epoll_create1(EPOLL_CLOEXEC); + EXPECT_GT(mEpollFd, 1); + + epoll_event event; + event.events = EPOLLIN; + EXPECT_EQ(0, epoll_ctl(mEpollFd, EPOLL_CTL_ADD, mDisplayEventReceiver.getFd(), &event)); + } + + void TearDown() override { + close(mEpollFd); + LayerTransactionTest::TearDown(); + } + virtual sp<SurfaceControl> createBufferStateLayer() { return createLayer(mClient, "test", 0, 0, ISurfaceComposerClient::eFXSurfaceBufferState); } @@ -82,6 +104,35 @@ public: ASSERT_NO_FATAL_FAILURE(helper.verifyFinalState()); } } + + DisplayEventReceiver mDisplayEventReceiver; + int mEpollFd; + + struct Vsync { + int64_t vsyncId = FrameTimelineInfo::INVALID_VSYNC_ID; + nsecs_t expectedPresentTime = std::numeric_limits<nsecs_t>::max(); + }; + + Vsync waitForNextVsync() { + mDisplayEventReceiver.requestNextVsync(); + epoll_event epollEvent; + Vsync vsync; + EXPECT_EQ(1, epoll_wait(mEpollFd, &epollEvent, 1, 1000)) + << "Timeout waiting for vsync event"; + DisplayEventReceiver::Event event; + while (mDisplayEventReceiver.getEvents(&event, 1) > 0) { + if (event.header.type != DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { + continue; + } + + vsync = {event.vsync.vsyncId, event.vsync.expectedVSyncTimestamp}; + } + + EXPECT_GE(vsync.vsyncId, 1); + EXPECT_GT(event.vsync.expectedVSyncTimestamp, systemTime()); + + return vsync; + } }; TEST_F(LayerCallbackTest, BufferColor) { @@ -873,6 +924,29 @@ TEST_F(LayerCallbackTest, DesiredPresentTime_Past) { expected.addExpectedPresentTime(systemTime()); EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true)); } + +TEST_F(LayerCallbackTest, ExpectedPresentTime) { + sp<SurfaceControl> layer; + ASSERT_NO_FATAL_FAILURE(layer = createBufferStateLayer()); + + Transaction transaction; + CallbackHelper callback; + int err = fillTransaction(transaction, &callback, layer); + if (err) { + GTEST_SUCCEED() << "test not supported"; + return; + } + + const Vsync vsync = waitForNextVsync(); + transaction.setFrameTimelineInfo({vsync.vsyncId, 0}); + transaction.apply(); + + ExpectedResult expected; + expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer); + expected.addExpectedPresentTimeForVsyncId(vsync.expectedPresentTime); + EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true)); +} + } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues diff --git a/services/surfaceflinger/tests/LayerTransactionTest.h b/services/surfaceflinger/tests/LayerTransactionTest.h index eba2c250a5..05729be34b 100644 --- a/services/surfaceflinger/tests/LayerTransactionTest.h +++ b/services/surfaceflinger/tests/LayerTransactionTest.h @@ -21,6 +21,7 @@ #pragma clang diagnostic ignored "-Wconversion" #pragma clang diagnostic ignored "-Wextra" +#include <cutils/properties.h> #include <gtest/gtest.h> #include <gui/ISurfaceComposer.h> #include <gui/SurfaceComposerClient.h> @@ -245,6 +246,18 @@ protected: sp<SurfaceComposerClient> mClient; + bool deviceSupportsBlurs() { + char value[PROPERTY_VALUE_MAX]; + property_get("ro.surface_flinger.supports_background_blur", value, "0"); + return atoi(value); + } + + bool deviceUsesSkiaRenderEngine() { + char value[PROPERTY_VALUE_MAX]; + property_get("debug.renderengine.backend", value, "default"); + return strstr(value, "skia") != nullptr; + } + sp<IBinder> mDisplay; uint32_t mDisplayWidth; uint32_t mDisplayHeight; @@ -307,4 +320,4 @@ private: } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic pop // ignored "-Wconversion -Wextra"
\ No newline at end of file +#pragma clang diagnostic pop // ignored "-Wconversion -Wextra" diff --git a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp index 782a364ea7..44e718921a 100644 --- a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp +++ b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp @@ -18,7 +18,6 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" -#include <cutils/properties.h> #include <gui/BufferItemConsumer.h> #include "TransactionTestHarnesses.h" @@ -301,47 +300,86 @@ TEST_P(LayerTypeAndRenderTypeTransactionTest, SetCornerRadiusChildCrop) { } } -TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadius) { - char value[PROPERTY_VALUE_MAX]; - property_get("ro.surface_flinger.supports_background_blur", value, "0"); - if (!atoi(value)) { - // This device doesn't support blurs, no-op. - return; - } +TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadiusSimple) { + if (!deviceSupportsBlurs()) GTEST_SKIP(); + if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP(); - auto size = 256; - auto center = size / 2; - auto blurRadius = 50; - - sp<SurfaceControl> backgroundLayer; - ASSERT_NO_FATAL_FAILURE(backgroundLayer = createLayer("background", size, size)); - ASSERT_NO_FATAL_FAILURE(fillLayerColor(backgroundLayer, Color::GREEN, size, size)); + const auto canvasSize = 256; sp<SurfaceControl> leftLayer; - ASSERT_NO_FATAL_FAILURE(leftLayer = createLayer("left", size / 2, size)); - ASSERT_NO_FATAL_FAILURE(fillLayerColor(leftLayer, Color::RED, size / 2, size)); - + sp<SurfaceControl> rightLayer; + sp<SurfaceControl> greenLayer; sp<SurfaceControl> blurLayer; - ASSERT_NO_FATAL_FAILURE(blurLayer = createLayer("blur", size, size)); - ASSERT_NO_FATAL_FAILURE(fillLayerColor(blurLayer, Color::TRANSPARENT, size, size)); + const auto leftRect = Rect(0, 0, canvasSize / 2, canvasSize); + const auto rightRect = Rect(canvasSize / 2, 0, canvasSize, canvasSize); + const auto blurRect = Rect(0, 0, canvasSize, canvasSize); - Transaction().setBackgroundBlurRadius(blurLayer, blurRadius).apply(); + ASSERT_NO_FATAL_FAILURE(leftLayer = + createLayer("Left", leftRect.getWidth(), leftRect.getHeight())); + ASSERT_NO_FATAL_FAILURE( + fillLayerColor(leftLayer, Color::BLUE, leftRect.getWidth(), leftRect.getHeight())); + ASSERT_NO_FATAL_FAILURE(greenLayer = createLayer("Green", canvasSize * 2, canvasSize * 2)); + ASSERT_NO_FATAL_FAILURE( + fillLayerColor(greenLayer, Color::GREEN, canvasSize * 2, canvasSize * 2)); + ASSERT_NO_FATAL_FAILURE( + rightLayer = createLayer("Right", rightRect.getWidth(), rightRect.getHeight())); + ASSERT_NO_FATAL_FAILURE( + fillLayerColor(rightLayer, Color::RED, rightRect.getWidth(), rightRect.getHeight())); - auto shot = getScreenCapture(); - // Edges are mixed - shot->expectColor(Rect(center - 1, center - 5, center, center + 5), Color{150, 150, 0, 255}, - 50 /* tolerance */); - shot->expectColor(Rect(center, center - 5, center + 1, center + 5), Color{150, 150, 0, 255}, - 50 /* tolerance */); + Transaction() + .setLayer(greenLayer, mLayerZBase) + .setFrame(leftLayer, {0, 0, canvasSize * 2, canvasSize * 2}) + .setLayer(leftLayer, mLayerZBase + 1) + .setFrame(leftLayer, leftRect) + .setLayer(rightLayer, mLayerZBase + 2) + .setPosition(rightLayer, rightRect.left, rightRect.top) + .setFrame(rightLayer, rightRect) + .apply(); + + { + auto shot = getScreenCapture(); + shot->expectColor(leftRect, Color::BLUE); + shot->expectColor(rightRect, Color::RED); + } + + ASSERT_NO_FATAL_FAILURE(blurLayer = createColorLayer("BackgroundBlur", Color::TRANSPARENT)); + + const auto blurRadius = canvasSize / 2; + Transaction() + .setLayer(blurLayer, mLayerZBase + 3) + .setBackgroundBlurRadius(blurLayer, blurRadius) + .setCrop_legacy(blurLayer, blurRect) + .setFrame(blurLayer, blurRect) + .setSize(blurLayer, blurRect.getWidth(), blurRect.getHeight()) + .setAlpha(blurLayer, 0.0f) + .apply(); + + { + auto shot = getScreenCapture(); + + const auto stepSize = 1; + const auto blurAreaOffset = blurRadius * 0.7f; + const auto blurAreaStartX = canvasSize / 2 - blurRadius + blurAreaOffset; + const auto blurAreaEndX = canvasSize / 2 + blurRadius - blurAreaOffset; + Color previousColor; + Color currentColor; + for (int y = 0; y < canvasSize; y++) { + shot->checkPixel(0, y, /* r = */ 0, /* g = */ 0, /* b = */ 255); + previousColor = shot->getPixelColor(0, y); + for (int x = blurAreaStartX; x < blurAreaEndX; x += stepSize) { + currentColor = shot->getPixelColor(x, y); + ASSERT_GT(currentColor.r, previousColor.r); + ASSERT_LT(currentColor.b, previousColor.b); + ASSERT_EQ(0, currentColor.g); + } + shot->checkPixel(canvasSize - 1, y, 255, 0, 0); + } + } } TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadiusOnMultipleLayers) { - char value[PROPERTY_VALUE_MAX]; - property_get("ro.surface_flinger.supports_background_blur", value, "0"); - if (!atoi(value)) { - // This device doesn't support blurs, no-op. - return; - } + if (!deviceSupportsBlurs()) GTEST_SKIP(); + if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP(); auto size = 256; auto center = size / 2; @@ -378,25 +416,15 @@ TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadiusOnMultipleL } TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurAffectedByParentAlpha) { - char value[PROPERTY_VALUE_MAX]; - property_get("ro.surface_flinger.supports_background_blur", value, "0"); - if (!atoi(value)) { - // This device doesn't support blurs, no-op. - return; - } - - property_get("debug.renderengine.backend", value, ""); - if (strcmp(value, "skiagl") != 0) { - // This device isn't using Skia render engine, no-op. - return; - } + if (!deviceSupportsBlurs()) GTEST_SKIP(); + if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP(); sp<SurfaceControl> left; sp<SurfaceControl> right; sp<SurfaceControl> blur; sp<SurfaceControl> blurParent; - const auto size = 32; + const auto size = 256; ASSERT_NO_FATAL_FAILURE(left = createLayer("Left", size, size)); ASSERT_NO_FATAL_FAILURE(fillLayerColor(left, Color::BLUE, size, size)); ASSERT_NO_FATAL_FAILURE(right = createLayer("Right", size, size)); diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp index f2cb9513f0..81efe0b270 100644 --- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp +++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp @@ -420,7 +420,54 @@ TEST_F(FrameTimelineTest, setMaxDisplayFramesSetsSizeProperly) { EXPECT_EQ(getNumberOfDisplayFrames(), *maxDisplayFrames); } +TEST_F(FrameTimelineTest, presentFenceSignaled_invalidSignalTime) { + Fps refreshRate = Fps::fromPeriodNsecs(11); + + auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 60}); + int64_t sfToken1 = mTokenManager->generateTokenForPredictions({52, 60, 60}); + + auto surfaceFrame1 = + mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne, + sUidOne, sLayerIdOne, sLayerNameOne, + sLayerNameOne); + mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate); + surfaceFrame1->setAcquireFenceTime(20); + surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented); + mFrameTimeline->addSurfaceFrame(surfaceFrame1); + + mFrameTimeline->setSfPresent(59, presentFence1); + presentFence1->signalForTest(-1); + addEmptyDisplayFrame(); + + auto displayFrame0 = getDisplayFrame(0); + EXPECT_EQ(displayFrame0->getActuals().presentTime, -1); + EXPECT_EQ(displayFrame0->getJankType(), JankType::Unknown); + EXPECT_EQ(surfaceFrame1->getActuals().presentTime, -1); + EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown); +} + // Tests related to TimeStats +TEST_F(FrameTimelineTest, presentFenceSignaled_doesNotReportForInvalidTokens) { + Fps refreshRate = Fps::fromPeriodNsecs(11); + EXPECT_CALL(*mTimeStats, incrementJankyFrames(_)).Times(0); + auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + int64_t surfaceFrameToken1 = -1; + int64_t sfToken1 = -1; + + auto surfaceFrame1 = + mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne, + sUidOne, sLayerIdOne, sLayerNameOne, + sLayerNameOne); + mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate); + surfaceFrame1->setAcquireFenceTime(20); + surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented); + mFrameTimeline->addSurfaceFrame(surfaceFrame1); + presentFence1->signalForTest(70); + + mFrameTimeline->setSfPresent(59, presentFence1); +} + TEST_F(FrameTimelineTest, presentFenceSignaled_reportsLongSfCpu) { Fps refreshRate = Fps::fromPeriodNsecs(11); // Deadline delta is 2ms because, sf's adjusted deadline is 60 - composerTime(3) = 57ms. @@ -603,6 +650,43 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_reportsAppMissWithRenderRate) { EXPECT_EQ(surfaceFrame1->getJankType(), JankType::AppDeadlineMissed); } +TEST_F(FrameTimelineTest, presentFenceSignaled_displayFramePredictionExpiredPresentsSurfaceFrame) { + Fps refreshRate = Fps::fromPeriodNsecs(11); + Fps renderRate = Fps::fromPeriodNsecs(30); + + EXPECT_CALL(*mTimeStats, + incrementJankyFrames(TimeStats::JankyFramesInfo{refreshRate, renderRate, sUidOne, + sLayerNameOne, JankType::Unknown, + -1, -1, 25})); + auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 60}); + int64_t sfToken1 = mTokenManager->generateTokenForPredictions({82, 90, 90}); + + auto surfaceFrame1 = + mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken1, sInputEventId}, sPidOne, + sUidOne, sLayerIdOne, sLayerNameOne, + sLayerNameOne); + surfaceFrame1->setAcquireFenceTime(45); + // Trigger a prediction expiry + flushTokens(systemTime() + maxTokenRetentionTime); + mFrameTimeline->setSfWakeUp(sfToken1, 52, refreshRate); + + surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented); + surfaceFrame1->setRenderRate(renderRate); + mFrameTimeline->addSurfaceFrame(surfaceFrame1); + presentFence1->signalForTest(90); + mFrameTimeline->setSfPresent(86, presentFence1); + + auto displayFrame = getDisplayFrame(0); + EXPECT_EQ(displayFrame->getJankType(), JankType::Unknown); + EXPECT_EQ(displayFrame->getFrameStartMetadata(), FrameStartMetadata::UnknownStart); + EXPECT_EQ(displayFrame->getFrameReadyMetadata(), FrameReadyMetadata::UnknownFinish); + EXPECT_EQ(displayFrame->getFramePresentMetadata(), FramePresentMetadata::UnknownPresent); + + EXPECT_EQ(surfaceFrame1->getActuals().presentTime, 90); + EXPECT_EQ(surfaceFrame1->getJankType(), JankType::Unknown); +} + /* * Tracing Tests * diff --git a/services/surfaceflinger/tests/utils/CallbackUtils.h b/services/surfaceflinger/tests/utils/CallbackUtils.h index 1318debbba..459b35c544 100644 --- a/services/surfaceflinger/tests/utils/CallbackUtils.h +++ b/services/surfaceflinger/tests/utils/CallbackUtils.h @@ -81,6 +81,10 @@ public: mExpectedPresentTime = expectedPresentTime; } + void addExpectedPresentTimeForVsyncId(nsecs_t expectedPresentTime) { + mExpectedPresentTimeForVsyncId = expectedPresentTime; + } + void verifyCallbackData(const CallbackData& callbackData) const { const auto& [latchTime, presentFence, surfaceControlStats] = callbackData; if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) { @@ -93,6 +97,11 @@ public: // misses vsync and we have to wait another 33.3ms ASSERT_LE(presentFence->getSignalTime(), mExpectedPresentTime + nsecs_t(66.666666 * 1e6)); + } else if (mExpectedPresentTimeForVsyncId >= 0) { + ASSERT_EQ(presentFence->wait(3000), NO_ERROR); + // We give 4ms for prediction error + ASSERT_GE(presentFence->getSignalTime(), + mExpectedPresentTimeForVsyncId - 4'000'000); } } else { ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented"; @@ -151,6 +160,7 @@ private: }; ExpectedResult::Transaction mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED; nsecs_t mExpectedPresentTime = -1; + nsecs_t mExpectedPresentTimeForVsyncId = -1; std::unordered_map<sp<SurfaceControl>, ExpectedSurfaceResult, SCHash> mExpectedSurfaceResults; }; diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h index 2fefa45e55..7efd730983 100644 --- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h +++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h @@ -159,6 +159,15 @@ public: } } + Color getPixelColor(uint32_t x, uint32_t y) { + if (!mOutBuffer || mOutBuffer->getPixelFormat() != HAL_PIXEL_FORMAT_RGBA_8888) { + return {0, 0, 0, 0}; + } + + const uint8_t* pixel = mPixels + (4 * (y * mOutBuffer->getStride() + x)); + return {pixel[0], pixel[1], pixel[2], pixel[3]}; + } + void expectFGColor(uint32_t x, uint32_t y) { checkPixel(x, y, 195, 63, 63); } void expectBGColor(uint32_t x, uint32_t y) { checkPixel(x, y, 63, 63, 195); } diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index cb845a0788..020b520a7c 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -1093,13 +1093,6 @@ VkResult CreateSwapchainKHR(VkDevice device, return VK_ERROR_SURFACE_LOST_KHR; } - err = native_window_set_buffer_count(window, 0); - if (err != android::OK) { - ALOGE("native_window_set_buffer_count(0) failed: %s (%d)", - strerror(-err), err); - return VK_ERROR_SURFACE_LOST_KHR; - } - int swap_interval = create_info->presentMode == VK_PRESENT_MODE_MAILBOX_KHR ? 0 : 1; err = window->setSwapInterval(window, swap_interval); @@ -1707,7 +1700,7 @@ VkResult QueuePresentKHR(VkQueue queue, const VkPresentInfoKHR* present_info) { if (err != android::OK) { ALOGE("queueBuffer failed: %s (%d)", strerror(-err), err); swapchain_result = WorstPresentResult( - swapchain_result, VK_ERROR_OUT_OF_DATE_KHR); + swapchain_result, VK_ERROR_SURFACE_LOST_KHR); } else { if (img.dequeue_fence >= 0) { close(img.dequeue_fence); |