diff options
32 files changed, 577 insertions, 289 deletions
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 70bbc33b42..1af6eddb2f 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -1273,11 +1273,6 @@ class Dex2oatFileWrapper { Dex2oatFileWrapper maybe_open_app_image(const char* out_oat_path, bool generate_app_image, bool is_public, int uid, bool is_secondary_dex) { - // We don't create an image for secondary dex files. - if (is_secondary_dex) { - return Dex2oatFileWrapper(); - } - const std::string image_path = create_image_filename(out_oat_path); if (image_path.empty()) { // Happens when the out_oat_path has an unknown extension. diff --git a/libs/dumputils/dump_utils.cpp b/libs/dumputils/dump_utils.cpp index 5b65c95e95..b2b74dce6e 100644 --- a/libs/dumputils/dump_utils.cpp +++ b/libs/dumputils/dump_utils.cpp @@ -75,6 +75,7 @@ static const char* hal_interfaces_to_dump[] { "android.hardware.automotive.audiocontrol@1.0::IAudioControl", "android.hardware.automotive.vehicle@2.0::IVehicle", "android.hardware.automotive.evs@1.0::IEvsCamera", + "android.hardware.neuralnetworks@1.0::IDevice", NULL, }; diff --git a/libs/renderengine/Android.bp b/libs/renderengine/Android.bp index 1075f161e4..eb6080fc21 100644 --- a/libs/renderengine/Android.bp +++ b/libs/renderengine/Android.bp @@ -55,6 +55,7 @@ filegroup { "gl/GLShadowTexture.cpp", "gl/GLShadowVertexGenerator.cpp", "gl/GLSkiaShadowPort.cpp", + "gl/GLVertexBuffer.cpp", "gl/ImageManager.cpp", "gl/Program.cpp", "gl/ProgramCache.cpp", diff --git a/libs/renderengine/gl/GLFramebuffer.cpp b/libs/renderengine/gl/GLFramebuffer.cpp index 153935b678..cb0d5cf95c 100644 --- a/libs/renderengine/gl/GLFramebuffer.cpp +++ b/libs/renderengine/gl/GLFramebuffer.cpp @@ -32,23 +32,14 @@ namespace renderengine { namespace gl { GLFramebuffer::GLFramebuffer(GLESRenderEngine& engine) - : GLFramebuffer(engine, false /* multiTarget */) {} - -GLFramebuffer::GLFramebuffer(GLESRenderEngine& engine, bool multiTarget) : mEngine(engine), mEGLDisplay(engine.getEGLDisplay()), mEGLImage(EGL_NO_IMAGE_KHR) { glGenTextures(1, &mTextureName); - if (multiTarget) { - glGenTextures(1, &mSecondaryTextureName); - } glGenFramebuffers(1, &mFramebufferName); } GLFramebuffer::~GLFramebuffer() { glDeleteFramebuffers(1, &mFramebufferName); glDeleteTextures(1, &mTextureName); - if (mSecondaryTextureName != -1) { - glDeleteTextures(1, &mSecondaryTextureName); - } } bool GLFramebuffer::setNativeWindowBuffer(ANativeWindowBuffer* nativeBuffer, bool isProtected, @@ -87,28 +78,12 @@ void GLFramebuffer::allocateBuffers(uint32_t width, uint32_t height) { glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_MIRRORED_REPEAT); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_MIRRORED_REPEAT); - const bool multiTarget = mSecondaryTextureName != -1; - if (multiTarget) { - glBindTexture(GL_TEXTURE_2D, mSecondaryTextureName); - glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, width, height, 0, GL_RGB, GL_UNSIGNED_BYTE, nullptr); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_MIRRORED_REPEAT); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_MIRRORED_REPEAT); - } - mBufferHeight = height; mBufferWidth = width; mEngine.checkErrors("Allocating Fbo texture"); bind(); glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextureName, 0); - if (multiTarget) { - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT1_EXT, GL_TEXTURE_2D, - mSecondaryTextureName, 0); - GLenum buffers[] = {GL_COLOR_ATTACHMENT0_EXT, GL_COLOR_ATTACHMENT1_EXT}; - glDrawBuffers(2, buffers); - } mStatus = glCheckFramebufferStatus(GL_FRAMEBUFFER); unbind(); glBindTexture(GL_TEXTURE_2D, 0); diff --git a/libs/renderengine/gl/GLFramebuffer.h b/libs/renderengine/gl/GLFramebuffer.h index 69102d6e78..b88da3b133 100644 --- a/libs/renderengine/gl/GLFramebuffer.h +++ b/libs/renderengine/gl/GLFramebuffer.h @@ -42,7 +42,6 @@ public: void allocateBuffers(uint32_t width, uint32_t height); EGLImageKHR getEGLImage() const { return mEGLImage; } uint32_t getTextureName() const { return mTextureName; } - uint32_t getSecondaryTextureName() const { return mSecondaryTextureName; } uint32_t getFramebufferName() const { return mFramebufferName; } int32_t getBufferHeight() const { return mBufferHeight; } int32_t getBufferWidth() const { return mBufferWidth; } @@ -59,7 +58,6 @@ private: bool usingFramebufferCache = false; GLenum mStatus = GL_FRAMEBUFFER_UNSUPPORTED; uint32_t mTextureName, mFramebufferName; - uint32_t mSecondaryTextureName = -1; int32_t mBufferHeight = 0; int32_t mBufferWidth = 0; diff --git a/libs/renderengine/gl/GLVertexBuffer.cpp b/libs/renderengine/gl/GLVertexBuffer.cpp new file mode 100644 index 0000000000..e50c471b6d --- /dev/null +++ b/libs/renderengine/gl/GLVertexBuffer.cpp @@ -0,0 +1,55 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define ATRACE_TAG ATRACE_TAG_GRAPHICS + +#include "GLVertexBuffer.h" + +#include <GLES/gl.h> +#include <GLES2/gl2.h> +#include <nativebase/nativebase.h> +#include <utils/Trace.h> + +namespace android { +namespace renderengine { +namespace gl { + +GLVertexBuffer::GLVertexBuffer() { + glGenBuffers(1, &mBufferName); +} + +GLVertexBuffer::~GLVertexBuffer() { + glDeleteBuffers(1, &mBufferName); +} + +void GLVertexBuffer::allocateBuffers(const GLfloat data[], const GLuint size) { + ATRACE_CALL(); + bind(); + glBufferData(GL_ARRAY_BUFFER, size * sizeof(GLfloat), data, GL_STATIC_DRAW); + unbind(); +} + +void GLVertexBuffer::bind() const { + glBindBuffer(GL_ARRAY_BUFFER, mBufferName); +} + +void GLVertexBuffer::unbind() const { + glBindBuffer(GL_ARRAY_BUFFER, 0); +} + +} // namespace gl +} // namespace renderengine +} // namespace android diff --git a/libs/renderengine/gl/GLVertexBuffer.h b/libs/renderengine/gl/GLVertexBuffer.h new file mode 100644 index 0000000000..c0fd0c1b04 --- /dev/null +++ b/libs/renderengine/gl/GLVertexBuffer.h @@ -0,0 +1,49 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <cstdint> + +#include <EGL/egl.h> +#include <EGL/eglext.h> +#include <GLES2/gl2.h> + +struct ANativeWindowBuffer; + +namespace android { +namespace renderengine { +namespace gl { + +class GLESRenderEngine; + +class GLVertexBuffer { +public: + explicit GLVertexBuffer(); + ~GLVertexBuffer(); + + void allocateBuffers(const GLfloat data[], const GLuint size); + uint32_t getBufferName() const { return mBufferName; } + void bind() const; + void unbind() const; + +private: + uint32_t mBufferName; +}; + +} // namespace gl +} // namespace renderengine +} // namespace android diff --git a/libs/renderengine/gl/filters/BlurFilter.cpp b/libs/renderengine/gl/filters/BlurFilter.cpp index e704907dac..6ba78dc025 100644 --- a/libs/renderengine/gl/filters/BlurFilter.cpp +++ b/libs/renderengine/gl/filters/BlurFilter.cpp @@ -49,6 +49,20 @@ BlurFilter::BlurFilter(GLESRenderEngine& engine) mBUvLoc = mBlurProgram.getAttributeLocation("aUV"); mBTextureLoc = mBlurProgram.getUniformLocation("uTexture"); mBOffsetLoc = mBlurProgram.getUniformLocation("uOffset"); + + static constexpr auto size = 2.0f; + static constexpr auto translation = 1.0f; + const GLfloat vboData[] = { + // Vertex data + translation-size, -translation-size, + translation-size, -translation+size, + translation+size, -translation+size, + // UV data + 0.0f, 0.0f-translation, + 0.0f, size-translation, + size, size-translation + }; + mMeshBuffer.allocateBuffers(vboData, 12 /* size */); } status_t BlurFilter::setAsDrawTarget(const DisplaySettings& display, uint32_t radius) { @@ -65,15 +79,23 @@ status_t BlurFilter::setAsDrawTarget(const DisplaySettings& display, uint32_t ra mPingFbo.allocateBuffers(fboWidth, fboHeight); mPongFbo.allocateBuffers(fboWidth, fboHeight); mTexturesAllocated = true; - } - if (mPingFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { - ALOGE("Invalid blur buffer"); - return mPingFbo.getStatus(); - } - if (mCompositionFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { - ALOGE("Invalid composition buffer"); - return mCompositionFbo.getStatus(); + if (mPingFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { + ALOGE("Invalid ping buffer"); + return mPingFbo.getStatus(); + } + if (mPongFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { + ALOGE("Invalid pong buffer"); + return mPongFbo.getStatus(); + } + if (mCompositionFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { + ALOGE("Invalid composition buffer"); + return mCompositionFbo.getStatus(); + } + if (!mBlurProgram.isValid()) { + ALOGE("Invalid shader"); + return GL_INVALID_OPERATION; + } } mCompositionFbo.bind(); @@ -82,43 +104,22 @@ status_t BlurFilter::setAsDrawTarget(const DisplaySettings& display, uint32_t ra } void BlurFilter::drawMesh(GLuint uv, GLuint position) { - static constexpr auto size = 2.0f; - static constexpr auto translation = 1.0f; - GLfloat positions[] = { - translation-size, -translation-size, - translation-size, -translation+size, - translation+size, -translation+size - }; - GLfloat texCoords[] = { - 0.0f, 0.0f-translation, - 0.0f, size-translation, - size, size-translation - }; - // set attributes glEnableVertexAttribArray(uv); - glVertexAttribPointer(uv, 2 /* size */, GL_FLOAT, GL_FALSE, 0, texCoords); glEnableVertexAttribArray(position); - glVertexAttribPointer(position, 2 /* size */, GL_FLOAT, GL_FALSE, 2 * sizeof(GLfloat), - positions); + mMeshBuffer.bind(); + glVertexAttribPointer(position, 2 /* size */, GL_FLOAT, GL_FALSE, + 2 * sizeof(GLfloat) /* stride */, 0 /* offset */); + glVertexAttribPointer(uv, 2 /* size */, GL_FLOAT, GL_FALSE, 0 /* stride */, + (GLvoid*)(6 * sizeof(GLfloat)) /* offset */); + mMeshBuffer.unbind(); // draw mesh glDrawArrays(GL_TRIANGLES, 0 /* first */, 3 /* count */); - mEngine.checkErrors("Drawing blur mesh"); } status_t BlurFilter::prepare() { ATRACE_NAME("BlurFilter::prepare"); - - if (mPongFbo.getStatus() != GL_FRAMEBUFFER_COMPLETE) { - ALOGE("Invalid FBO"); - return mPongFbo.getStatus(); - } - if (!mBlurProgram.isValid()) { - ALOGE("Invalid shader"); - return GL_INVALID_OPERATION; - } - blit(mCompositionFbo, mPingFbo); // Kawase is an approximation of Gaussian, but it behaves differently from it. @@ -136,16 +137,15 @@ status_t BlurFilter::prepare() { GLFramebuffer* draw = &mPongFbo; float stepX = radius / (float)mCompositionFbo.getBufferWidth() / (float)passes; float stepY = radius / (float)mCompositionFbo.getBufferHeight() / (float)passes; + glViewport(0, 0, draw->getBufferWidth(), draw->getBufferHeight()); glActiveTexture(GL_TEXTURE0); glUniform1i(mBTextureLoc, 0); for (auto i = 0; i < passes; i++) { ATRACE_NAME("BlurFilter::renderPass"); draw->bind(); - glViewport(0, 0, draw->getBufferWidth(), draw->getBufferHeight()); glBindTexture(GL_TEXTURE_2D, read->getTextureName()); glUniform2f(mBOffsetLoc, stepX * i, stepY * i); - mEngine.checkErrors("Setting uniforms"); drawMesh(mBUvLoc, mBPosLoc); @@ -189,17 +189,18 @@ status_t BlurFilter::render(bool multiPass) { glActiveTexture(GL_TEXTURE1); glBindTexture(GL_TEXTURE_2D, mCompositionFbo.getTextureName()); glUniform1i(mMCompositionTextureLoc, 1); - mEngine.checkErrors("Setting final pass uniforms"); drawMesh(mMUvLoc, mMPosLoc); glUseProgram(0); glActiveTexture(GL_TEXTURE0); + mEngine.checkErrors("Drawing blur mesh"); return NO_ERROR; } string BlurFilter::getVertexShader() const { return R"SHADER(#version 310 es + precision mediump float; in vec2 aPosition; in highp vec2 aUV; @@ -219,7 +220,7 @@ string BlurFilter::getFragmentShader() const { uniform sampler2D uTexture; uniform vec2 uOffset; - highp in vec2 vUV; + in highp vec2 vUV; out vec4 fragColor; void main() { diff --git a/libs/renderengine/gl/filters/BlurFilter.h b/libs/renderengine/gl/filters/BlurFilter.h index eb6120ba92..3eb5c96664 100644 --- a/libs/renderengine/gl/filters/BlurFilter.h +++ b/libs/renderengine/gl/filters/BlurFilter.h @@ -19,6 +19,7 @@ #include <ui/GraphicTypes.h> #include "../GLESRenderEngine.h" #include "../GLFramebuffer.h" +#include "../GLVertexBuffer.h" #include "GenericProgram.h" using namespace std; @@ -72,6 +73,9 @@ private: GLFramebuffer* mLastDrawTarget; bool mTexturesAllocated = false; + // VBO containing vertex and uv data of a fullscreen triangle. + GLVertexBuffer mMeshBuffer; + GenericProgram mMixProgram; GLuint mMPosLoc; GLuint mMUvLoc; diff --git a/libs/ui/include/ui/Size.h b/libs/ui/include/ui/Size.h index d9b713df4c..c2cda17a6f 100644 --- a/libs/ui/include/ui/Size.h +++ b/libs/ui/include/ui/Size.h @@ -19,6 +19,7 @@ #include <algorithm> #include <cstdint> #include <limits> +#include <ostream> #include <type_traits> #include <utility> @@ -113,13 +114,12 @@ struct Size { std::numeric_limits<Size::remove_cv_reference_t<ToType>>::is_bounded && std::numeric_limits<Size::remove_cv_reference_t<FromType>>::is_bounded, FromType&&>::type v) { - static constexpr auto toHighest = std::numeric_limits<remove_cv_reference_t<ToType>>::max(); - static constexpr auto toLowest = - std::numeric_limits<remove_cv_reference_t<ToType>>::lowest(); - static constexpr auto fromHighest = - std::numeric_limits<remove_cv_reference_t<FromType>>::max(); - static constexpr auto fromLowest = - std::numeric_limits<remove_cv_reference_t<FromType>>::lowest(); + using BareToType = remove_cv_reference_t<ToType>; + using BareFromType = remove_cv_reference_t<FromType>; + static constexpr auto toHighest = std::numeric_limits<BareToType>::max(); + static constexpr auto toLowest = std::numeric_limits<BareToType>::lowest(); + static constexpr auto fromHighest = std::numeric_limits<BareFromType>::max(); + static constexpr auto fromLowest = std::numeric_limits<BareFromType>::lowest(); // A clamp is needed if the range of FromType is not a subset of the range of ToType static constexpr bool isClampNeeded = (toLowest > fromLowest) || (toHighest < fromHighest); @@ -129,10 +129,13 @@ struct Size { return static_cast<ToType>(v); } - // Otherwise we leverage implicit conversion to safely compare values of - // different types, to ensure we return a value clamped to the range of - // ToType. - return v < toLowest ? toLowest : (static_cast<ToType>(v) > toHighest ? toHighest : static_cast<ToType>(v)); + // Otherwise we need to carefully compare the limits of ToType (casted + // for the comparisons to be warning free to FromType) while still + // ensuring we return a value clamped to the range of ToType. + return v < static_cast<const BareFromType>(toLowest) + ? toLowest + : (v > static_cast<const BareFromType>(toHighest) ? toHighest + : static_cast<ToType>(v)); } }; @@ -154,5 +157,10 @@ inline bool operator<(const Size& lhs, const Size& rhs) { return lhs.height < rhs.height; } +// Defining PrintTo helps with Google Tests. +static inline void PrintTo(const Size& size, ::std::ostream* os) { + *os << "Size(" << size.width << ", " << size.height << ")"; +} + } // namespace ui } // namespace android diff --git a/libs/ui/tests/Android.bp b/libs/ui/tests/Android.bp index ff55bc2303..605c5a9ba0 100644 --- a/libs/ui/tests/Android.bp +++ b/libs/ui/tests/Android.bp @@ -111,6 +111,7 @@ cc_test { cc_test { name: "Size_test", + test_suites: ["device-tests"], shared_libs: ["libui"], srcs: ["Size_test.cpp"], cflags: ["-Wall", "-Werror"], diff --git a/libs/ui/tests/Size_test.cpp b/libs/ui/tests/Size_test.cpp index 69e1ac8b27..40dc702a8b 100644 --- a/libs/ui/tests/Size_test.cpp +++ b/libs/ui/tests/Size_test.cpp @@ -19,8 +19,18 @@ #include <cmath> #include <cstdlib> +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic error "-Wimplicit-int-float-conversion" +#pragma clang diagnostic error "-Wconversion" +#endif // __clang__ + #include <ui/Size.h> +#ifdef __clang__ +#pragma clang diagnostic pop +#endif // __clang__ + #include <gtest/gtest.h> namespace android { diff --git a/libs/ui/tests/TEST_MAPPING b/libs/ui/tests/TEST_MAPPING new file mode 100644 index 0000000000..7fcd7de319 --- /dev/null +++ b/libs/ui/tests/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "Size_test" + } + ] +} diff --git a/services/gpuservice/gpustats/GpuStats.cpp b/services/gpuservice/gpustats/GpuStats.cpp index d0945326b3..231d0684c9 100644 --- a/services/gpuservice/gpustats/GpuStats.cpp +++ b/services/gpuservice/gpustats/GpuStats.cpp @@ -30,16 +30,11 @@ namespace android { -GpuStats::GpuStats() { - AStatsManager_registerPullAtomCallback(android::util::GPU_STATS_GLOBAL_INFO, - GpuStats::pullAtomCallback, nullptr, this); - AStatsManager_registerPullAtomCallback(android::util::GPU_STATS_APP_INFO, - GpuStats::pullAtomCallback, nullptr, this); -} - GpuStats::~GpuStats() { - AStatsManager_unregisterPullAtomCallback(android::util::GPU_STATS_GLOBAL_INFO); - AStatsManager_unregisterPullAtomCallback(android::util::GPU_STATS_APP_INFO); + if (mStatsdRegistered) { + AStatsManager_clearPullAtomCallback(android::util::GPU_STATS_GLOBAL_INFO); + AStatsManager_clearPullAtomCallback(android::util::GPU_STATS_APP_INFO); + } } static void addLoadingCount(GpuStatsInfo::Driver driver, bool isDriverLoaded, @@ -97,6 +92,7 @@ void GpuStats::insertDriverStats(const std::string& driverPackageName, ATRACE_CALL(); std::lock_guard<std::mutex> lock(mLock); + registerStatsdCallbacksIfNeeded(); ALOGV("Received:\n" "\tdriverPackageName[%s]\n" "\tdriverVersionName[%s]\n" @@ -150,6 +146,7 @@ void GpuStats::insertTargetStats(const std::string& appPackageName, const std::string appStatsKey = appPackageName + std::to_string(driverVersionCode); std::lock_guard<std::mutex> lock(mLock); + registerStatsdCallbacksIfNeeded(); if (!mAppStats.count(appStatsKey)) { return; } @@ -179,6 +176,16 @@ void GpuStats::interceptSystemDriverStatsLocked() { mGlobalStats[0].glesVersion = property_get_int32("ro.opengles.version", 0); } +void GpuStats::registerStatsdCallbacksIfNeeded() { + if (!mStatsdRegistered) { + AStatsManager_setPullAtomCallback(android::util::GPU_STATS_GLOBAL_INFO, nullptr, + GpuStats::pullAtomCallback, this); + AStatsManager_setPullAtomCallback(android::util::GPU_STATS_APP_INFO, nullptr, + GpuStats::pullAtomCallback, this); + mStatsdRegistered = true; + } +} + void GpuStats::dump(const Vector<String16>& args, std::string* result) { ATRACE_CALL(); diff --git a/services/gpuservice/gpustats/include/gpustats/GpuStats.h b/services/gpuservice/gpustats/include/gpustats/GpuStats.h index 8ca4e4ed71..55f0da1bc5 100644 --- a/services/gpuservice/gpustats/include/gpustats/GpuStats.h +++ b/services/gpuservice/gpustats/include/gpustats/GpuStats.h @@ -30,7 +30,6 @@ namespace android { class GpuStats { public: - GpuStats(); ~GpuStats(); // Insert new gpu driver stats into global stats and app stats. @@ -66,12 +65,16 @@ private: void dumpAppLocked(std::string* result); // Append cpuVulkanVersion and glesVersion to system driver stats void interceptSystemDriverStatsLocked(); + // Registers statsd callbacks if they have not already been registered + void registerStatsdCallbacksIfNeeded(); // Below limits the memory usage of GpuStats to be less than 10KB. This is // the preferred number for statsd while maintaining nice data quality. static const size_t MAX_NUM_APP_RECORDS = 100; // GpuStats access should be guarded by mLock. std::mutex mLock; + // True if statsd callbacks have been registered. + bool mStatsdRegistered = false; // Key is driver version code. std::unordered_map<uint64_t, GpuStatsGlobalInfo> mGlobalStats; // Key is <app package name>+<driver version code>. diff --git a/services/inputflinger/Android.bp b/services/inputflinger/Android.bp index 4ec4e25010..f67c9d006b 100644 --- a/services/inputflinger/Android.bp +++ b/services/inputflinger/Android.bp @@ -52,7 +52,6 @@ cc_defaults { "libstatslog", "libutils", "libui", - "server_configurable_flags", ], } diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index e5e83d752c..77a0716269 100644 --- a/services/inputflinger/InputClassifier.cpp +++ b/services/inputflinger/InputClassifier.cpp @@ -27,7 +27,6 @@ #if defined(__linux__) #include <pthread.h> #endif -#include <server_configurable_flags/get_flags.h> #include <unordered_set> #include <android/hardware/input/classifier/1.0/IInputClassifier.h> @@ -46,13 +45,6 @@ using namespace android::hardware::input; namespace android { -static constexpr bool DEBUG = false; - -// Category (=namespace) name for the input settings that are applied at boot time -static const char* INPUT_NATIVE_BOOT = "input_native_boot"; -// Feature flag name for the deep press feature -static const char* DEEP_PRESS_ENABLED = "deep_press_enabled"; - //Max number of elements to store in mEvents. static constexpr size_t MAX_EVENTS = 5; @@ -79,20 +71,6 @@ static bool isTouchEvent(const NotifyMotionArgs& args) { return args.source == AINPUT_SOURCE_TOUCHPAD || args.source == AINPUT_SOURCE_TOUCHSCREEN; } -// Check if the "deep touch" feature is on. -static bool deepPressEnabled() { - std::string flag_value = server_configurable_flags::GetServerConfigurableFlag( - INPUT_NATIVE_BOOT, DEEP_PRESS_ENABLED, "true"); - std::transform(flag_value.begin(), flag_value.end(), flag_value.begin(), ::tolower); - if (flag_value == "1" || flag_value == "true") { - ALOGI("Deep press feature enabled."); - return true; - } - ALOGI("Deep press feature is not enabled."); - return false; -} - - // --- ClassifierEvent --- ClassifierEvent::ClassifierEvent(std::unique_ptr<NotifyMotionArgs> args) : @@ -141,53 +119,40 @@ std::optional<int32_t> ClassifierEvent::getDeviceId() const { // --- MotionClassifier --- -MotionClassifier::MotionClassifier(sp<android::hardware::hidl_death_recipient> deathRecipient) : - mDeathRecipient(deathRecipient), mEvents(MAX_EVENTS) { - mHalThread = std::thread(&MotionClassifier::callInputClassifierHal, this); +MotionClassifier::MotionClassifier( + sp<android::hardware::input::classifier::V1_0::IInputClassifier> service) + : mEvents(MAX_EVENTS), mService(service) { + // Under normal operation, we do not need to reset the HAL here. But in the case where system + // crashed, but HAL didn't, we may be connecting to an existing HAL process that might already + // have received events in the past. That means, that HAL could be in an inconsistent state + // once it receives events from the newly created MotionClassifier. + mEvents.push(ClassifierEvent::createHalResetEvent()); + + mHalThread = std::thread(&MotionClassifier::processEvents, this); #if defined(__linux__) // Set the thread name for debugging pthread_setname_np(mHalThread.native_handle(), "InputClassifier"); #endif } -/** - * This function may block for some time to initialize the HAL, so it should only be called - * from the "InputClassifier HAL" thread. - */ -bool MotionClassifier::init() { - ensureHalThread(__func__); +std::unique_ptr<MotionClassifierInterface> MotionClassifier::create( + sp<android::hardware::hidl_death_recipient> deathRecipient) { sp<android::hardware::input::classifier::V1_0::IInputClassifier> service = classifier::V1_0::IInputClassifier::getService(); if (!service) { // Not really an error, maybe the device does not have this HAL, // but somehow the feature flag is flipped ALOGI("Could not obtain InputClassifier HAL"); - return false; + return nullptr; } - sp<android::hardware::hidl_death_recipient> recipient = mDeathRecipient.promote(); - if (recipient != nullptr) { - const bool linked = service->linkToDeath(recipient, 0 /* cookie */).withDefault(false); - if (!linked) { - ALOGE("Could not link MotionClassifier to the HAL death"); - return false; - } + const bool linked = service->linkToDeath(deathRecipient, 0 /* cookie */).withDefault(false); + if (!linked) { + ALOGE("Could not link death recipient to the HAL death"); + return nullptr; } - - // Under normal operation, we do not need to reset the HAL here. But in the case where system - // crashed, but HAL didn't, we may be connecting to an existing HAL process that might already - // have received events in the past. That means, that HAL could be in an inconsistent state - // once it receives events from the newly created MotionClassifier. - mEvents.push(ClassifierEvent::createHalResetEvent()); - - { - std::scoped_lock lock(mLock); - if (mService) { - ALOGE("MotionClassifier::%s should only be called once", __func__); - } - mService = service; - } - return true; + // Using 'new' to access a non-public constructor + return std::unique_ptr<MotionClassifier>(new MotionClassifier(service)); } MotionClassifier::~MotionClassifier() { @@ -195,14 +160,6 @@ MotionClassifier::~MotionClassifier() { mHalThread.join(); } -void MotionClassifier::ensureHalThread(const char* function) { - if (DEBUG) { - if (std::this_thread::get_id() != mHalThread.get_id()) { - LOG_FATAL("Function %s should only be called from InputClassifier thread", function); - } - } -} - /** * Obtain the classification from the HAL for a given MotionEvent. * Should only be called from the InputClassifier thread (mHalThread). @@ -213,23 +170,7 @@ void MotionClassifier::ensureHalThread(const char* function) { * To remove any possibility of negatively affecting the touch latency, the HAL * is called from a dedicated thread. */ -void MotionClassifier::callInputClassifierHal() { - ensureHalThread(__func__); - const bool initialized = init(); - if (!initialized) { - // MotionClassifier no longer useful. - // Deliver death notification from a separate thread - // because ~MotionClassifier may be invoked, which calls mHalThread.join() - std::thread([deathRecipient = mDeathRecipient](){ - sp<android::hardware::hidl_death_recipient> recipient = deathRecipient.promote(); - if (recipient != nullptr) { - recipient->serviceDied(0 /*cookie*/, nullptr); - } - }).detach(); - return; - } - // From this point on, mService is guaranteed to be non-null. - +void MotionClassifier::processEvents() { while (true) { ClassifierEvent event = mEvents.pop(); bool halResponseOk = true; @@ -389,24 +330,41 @@ void MotionClassifier::dump(std::string& dump) { } } +// --- HalDeathRecipient -// --- InputClassifier --- +InputClassifier::HalDeathRecipient::HalDeathRecipient(InputClassifier& parent) : mParent(parent) {} -InputClassifier::InputClassifier(const sp<InputListenerInterface>& listener) : - mListener(listener) { - // The rest of the initialization is done in onFirstRef, because we need to obtain - // an sp to 'this' in order to register for HAL death notifications +void InputClassifier::HalDeathRecipient::serviceDied( + uint64_t cookie, const wp<android::hidl::base::V1_0::IBase>& who) { + sp<android::hidl::base::V1_0::IBase> service = who.promote(); + if (service) { + service->unlinkToDeath(this); + } + mParent.setMotionClassifier(nullptr); } -void InputClassifier::onFirstRef() { - if (!deepPressEnabled()) { - // If feature is not enabled, MotionClassifier should stay null to avoid unnecessary work. - // When MotionClassifier is null, InputClassifier will forward all events - // to the next InputListener, unmodified. - return; +// --- InputClassifier --- + +InputClassifier::InputClassifier(const sp<InputListenerInterface>& listener) + : mListener(listener), mHalDeathRecipient(new HalDeathRecipient(*this)) {} + +void InputClassifier::setMotionClassifierEnabled(bool enabled) { + if (enabled) { + ALOGI("Enabling motion classifier"); + if (mInitializeMotionClassifierThread.joinable()) { + mInitializeMotionClassifierThread.join(); + } + mInitializeMotionClassifierThread = std::thread( + [this] { setMotionClassifier(MotionClassifier::create(mHalDeathRecipient)); }); +#if defined(__linux__) + // Set the thread name for debugging + pthread_setname_np(mInitializeMotionClassifierThread.native_handle(), + "Create MotionClassifier"); +#endif + } else { + ALOGI("Disabling motion classifier"); + setMotionClassifier(nullptr); } - std::scoped_lock lock(mLock); - mMotionClassifier = std::make_unique<MotionClassifier>(this); } void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) { @@ -447,22 +405,15 @@ void InputClassifier::notifyDeviceReset(const NotifyDeviceResetArgs* args) { mListener->notifyDeviceReset(args); } -void InputClassifier::serviceDied(uint64_t /*cookie*/, - const wp<android::hidl::base::V1_0::IBase>& who) { +void InputClassifier::setMotionClassifier( + std::unique_ptr<MotionClassifierInterface> motionClassifier) { std::scoped_lock lock(mLock); - ALOGE("InputClassifier HAL has died. Setting mMotionClassifier to null"); - mMotionClassifier = nullptr; - sp<android::hidl::base::V1_0::IBase> service = who.promote(); - if (service) { - service->unlinkToDeath(this); - } + mMotionClassifier = std::move(motionClassifier); } void InputClassifier::dump(std::string& dump) { std::scoped_lock lock(mLock); dump += "Input Classifier State:\n"; - dump += StringPrintf(INDENT1 "Deep press: %s\n", deepPressEnabled() ? "enabled" : "disabled"); - dump += INDENT1 "Motion Classifier:\n"; if (mMotionClassifier) { mMotionClassifier->dump(dump); @@ -472,4 +423,10 @@ void InputClassifier::dump(std::string& dump) { dump += "\n"; } +InputClassifier::~InputClassifier() { + if (mInitializeMotionClassifierThread.joinable()) { + mInitializeMotionClassifierThread.join(); + } +} + } // namespace android diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h index 96923526da..03510a623c 100644 --- a/services/inputflinger/InputClassifier.h +++ b/services/inputflinger/InputClassifier.h @@ -19,8 +19,8 @@ #include <android-base/thread_annotations.h> #include <utils/RefBase.h> -#include <unordered_map> #include <thread> +#include <unordered_map> #include "BlockingQueue.h" #include "InputListener.h" @@ -90,6 +90,7 @@ public: */ class InputClassifierInterface : public virtual RefBase, public InputListenerInterface { public: + virtual void setMotionClassifierEnabled(bool enabled) = 0; /** * Dump the state of the input classifier. * This method may be called on any thread (usually by the input manager). @@ -113,23 +114,23 @@ protected: */ class MotionClassifier final : public MotionClassifierInterface { public: - /** - * The deathRecipient will be subscribed to the HAL death. If the death recipient - * owns MotionClassifier and receives HAL death, it should delete its copy of it. - * The callback serviceDied will also be sent if the MotionClassifier itself fails - * to initialize. If the MotionClassifier fails to initialize, it is not useful, and - * should be deleted. - * If no death recipient is supplied, then the registration step will be skipped, so there will - * be no listeners registered for the HAL death. This is useful for testing - * MotionClassifier in isolation. + /* + * Create an instance of MotionClassifier. + * The death recipient, if provided, will be subscribed to the HAL death. + * The death recipient could be used to destroy MotionClassifier. + * + * This function should be called asynchronously, because getService takes a long time. */ - explicit MotionClassifier(sp<android::hardware::hidl_death_recipient> deathRecipient = nullptr); + static std::unique_ptr<MotionClassifierInterface> create( + sp<android::hardware::hidl_death_recipient> deathRecipient); + ~MotionClassifier(); /** * Classifies events asynchronously; that is, it doesn't block events on a classification, - * but instead sends them over to the classifier HAL and after a classification is - * determined, it then marks the next event it sees in the stream with it. + * but instead sends them over to the classifier HAL. After a classification of a specific + * event is determined, MotionClassifier then marks the next event in the stream with this + * classification. * * Therefore, it is acceptable to have the classifications be delayed by 1-2 events * in a particular gesture. @@ -141,15 +142,9 @@ public: virtual void dump(std::string& dump) override; private: - /** - * Initialize MotionClassifier. - * Return true if initializaion is successful. - */ - bool init(); - /** - * Entity that will be notified of the HAL death (most likely InputClassifier). - */ - wp<android::hardware::hidl_death_recipient> mDeathRecipient; + friend class MotionClassifierTest; // to create MotionClassifier with a test HAL implementation + explicit MotionClassifier( + sp<android::hardware::input::classifier::V1_0::IInputClassifier> service); // The events that need to be sent to the HAL. BlockingQueue<ClassifierEvent> mEvents; @@ -164,14 +159,9 @@ private: */ std::thread mHalThread; /** - * Print an error message if the caller is not on the InputClassifier thread. - * Caller must supply the name of the calling function as __func__ + * Process events and call the InputClassifier HAL */ - void ensureHalThread(const char* function); - /** - * Call the InputClassifier HAL - */ - void callInputClassifierHal(); + void processEvents(); /** * Access to the InputClassifier HAL. May be null if init() hasn't completed yet. * When init() successfully completes, mService is guaranteed to remain non-null and to not @@ -225,19 +215,15 @@ private: const char* getServiceStatus() REQUIRES(mLock); }; - /** * Implementation of the InputClassifierInterface. * Represents a separate stage of input processing. All of the input events go through this stage. * Acts as a passthrough for all input events except for motion events. * The events of motion type are sent to MotionClassifier. */ -class InputClassifier : public InputClassifierInterface, - public android::hardware::hidl_death_recipient { +class InputClassifier : public InputClassifierInterface { public: explicit InputClassifier(const sp<InputListenerInterface>& listener); - // Some of the constructor logic is finished in onFirstRef - virtual void onFirstRef() override; virtual void notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) override; virtual void notifyKey(const NotifyKeyArgs* args) override; @@ -245,17 +231,47 @@ public: virtual void notifySwitch(const NotifySwitchArgs* args) override; virtual void notifyDeviceReset(const NotifyDeviceResetArgs* args) override; - virtual void serviceDied(uint64_t cookie, - const wp<android::hidl::base::V1_0::IBase>& who) override; - virtual void dump(std::string& dump) override; + ~InputClassifier(); + + // Called from InputManager + virtual void setMotionClassifierEnabled(bool enabled) override; + private: // Protect access to mMotionClassifier, since it may become null via a hidl callback std::mutex mLock; - std::unique_ptr<MotionClassifierInterface> mMotionClassifier GUARDED_BY(mLock); // The next stage to pass input events to sp<InputListenerInterface> mListener; + + std::unique_ptr<MotionClassifierInterface> mMotionClassifier GUARDED_BY(mLock); + std::thread mInitializeMotionClassifierThread; + /** + * Set the value of mMotionClassifier. + * This is called from 2 different threads: + * 1) mInitializeMotionClassifierThread, when we have constructed a MotionClassifier + * 2) A binder thread of the HalDeathRecipient, which is created when HAL dies. This would cause + * mMotionClassifier to become nullptr. + */ + void setMotionClassifier(std::unique_ptr<MotionClassifierInterface> motionClassifier); + + /** + * The deathRecipient will call setMotionClassifier(null) when the HAL dies. + */ + class HalDeathRecipient : public android::hardware::hidl_death_recipient { + public: + explicit HalDeathRecipient(InputClassifier& parent); + virtual void serviceDied(uint64_t cookie, + const wp<android::hidl::base::V1_0::IBase>& who) override; + + private: + InputClassifier& mParent; + }; + // We retain a reference to death recipient, because the death recipient will be calling + // ~MotionClassifier if the HAL dies. + // If we don't retain a reference, and MotionClassifier is the only owner of the death + // recipient, the serviceDied call will cause death recipient to call its own destructor. + sp<HalDeathRecipient> mHalDeathRecipient; }; } // namespace android diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp index c7c61cf1ef..fc771a2c58 100644 --- a/services/inputflinger/InputManager.cpp +++ b/services/inputflinger/InputManager.cpp @@ -132,4 +132,8 @@ void InputManager::unregisterInputChannel(const sp<InputChannel>& channel) { mDispatcher->unregisterInputChannel(channel); } +void InputManager::setMotionClassifierEnabled(bool enabled) { + mClassifier->setMotionClassifierEnabled(enabled); +} + } // namespace android diff --git a/services/inputflinger/InputManager.h b/services/inputflinger/InputManager.h index 586097f6a3..0158441fd1 100644 --- a/services/inputflinger/InputManager.h +++ b/services/inputflinger/InputManager.h @@ -100,6 +100,8 @@ public: virtual void registerInputChannel(const sp<InputChannel>& channel); virtual void unregisterInputChannel(const sp<InputChannel>& channel); + void setMotionClassifierEnabled(bool enabled); + private: sp<InputReaderInterface> mReader; diff --git a/services/inputflinger/tests/InputClassifier_test.cpp b/services/inputflinger/tests/InputClassifier_test.cpp index 40086ef708..ab74a0498d 100644 --- a/services/inputflinger/tests/InputClassifier_test.cpp +++ b/services/inputflinger/tests/InputClassifier_test.cpp @@ -22,6 +22,9 @@ #include <android/hardware/input/classifier/1.0/IInputClassifier.h> using namespace android::hardware::input; +using android::hardware::Return; +using android::hardware::Void; +using android::hardware::input::common::V1_0::Classification; namespace android { @@ -132,6 +135,49 @@ TEST_F(InputClassifierTest, SendToNextStage_NotifyDeviceResetArgs) { ASSERT_EQ(args, outArgs); } +TEST_F(InputClassifierTest, SetMotionClassifier_Enabled) { + mClassifier->setMotionClassifierEnabled(true); +} + +TEST_F(InputClassifierTest, SetMotionClassifier_Disabled) { + mClassifier->setMotionClassifierEnabled(false); +} + +/** + * Try to break it by calling setMotionClassifierEnabled multiple times. + */ +TEST_F(InputClassifierTest, SetMotionClassifier_Multiple) { + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(false); + mClassifier->setMotionClassifierEnabled(false); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); +} + +/** + * A minimal implementation of IInputClassifier. + */ +struct TestHal : public android::hardware::input::classifier::V1_0::IInputClassifier { + Return<Classification> classify( + const android::hardware::input::common::V1_0::MotionEvent& event) override { + return Classification::NONE; + }; + Return<void> reset() override { return Void(); }; + Return<void> resetDevice(int32_t deviceId) override { return Void(); }; +}; + +/** + * An entity that will be subscribed to the HAL death. + */ +class TestDeathRecipient : public android::hardware::hidl_death_recipient { +public: + virtual void serviceDied(uint64_t cookie, + const wp<android::hidl::base::V1_0::IBase>& who) override{}; +}; + // --- MotionClassifierTest --- class MotionClassifierTest : public testing::Test { @@ -139,7 +185,14 @@ protected: std::unique_ptr<MotionClassifierInterface> mMotionClassifier; virtual void SetUp() override { - mMotionClassifier = std::make_unique<MotionClassifier>(); + mMotionClassifier = MotionClassifier::create(new TestDeathRecipient()); + if (mMotionClassifier == nullptr) { + // If the device running this test does not have IInputClassifier service, + // use the test HAL instead. + // Using 'new' to access non-public constructor + mMotionClassifier = + std::unique_ptr<MotionClassifier>(new MotionClassifier(new TestHal())); + } } }; diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index 5e04d9597a..e50a9095fe 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -443,10 +443,7 @@ void BufferLayerConsumer::freeBufferLocked(int slotIndex) { } void BufferLayerConsumer::onDisconnect() { - sp<Layer> l = mLayer.promote(); - if (l.get()) { - l->onDisconnect(); - } + mLayer->onDisconnect(); } void BufferLayerConsumer::onSidebandStreamChanged() { @@ -480,10 +477,7 @@ void BufferLayerConsumer::onBufferAvailable(const BufferItem& item) { void BufferLayerConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) { - sp<Layer> l = mLayer.promote(); - if (l.get()) { - l->addAndGetFrameTimestamps(newTimestamps, outDelta); - } + mLayer->addAndGetFrameTimestamps(newTimestamps, outDelta); } void BufferLayerConsumer::abandonLocked() { diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h index 39ed3707dd..c71a1d9f7a 100644 --- a/services/surfaceflinger/BufferLayerConsumer.h +++ b/services/surfaceflinger/BufferLayerConsumer.h @@ -332,7 +332,7 @@ private: const uint32_t mTexName; // The layer for this BufferLayerConsumer - const wp<Layer> mLayer; + Layer* mLayer; wp<ContentsChangedListener> mContentsChangedListener; diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp index 399da19c75..257b8b16bf 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -115,10 +115,10 @@ bool VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) { auto it = mRateMap.find(mIdealPeriod); auto const currentPeriod = std::get<0>(it->second); // TODO (b/144707443): its important that there's some precision in the mean of the ordinals - // for the intercept calculation, so scale the ordinals by 10 to continue + // for the intercept calculation, so scale the ordinals by 1000 to continue // fixed point calculation. Explore expanding // scheduler::utils::calculate_mean to have a fixed point fractional part. - static constexpr int kScalingFactor = 10; + static constexpr int64_t kScalingFactor = 1000; for (auto i = 0u; i < mTimestamps.size(); i++) { traceInt64If("VSP-ts", mTimestamps[i]); @@ -147,7 +147,7 @@ bool VSyncPredictor::addVsyncTimestamp(nsecs_t timestamp) { return false; } - nsecs_t const anticipatedPeriod = top / bottom * kScalingFactor; + nsecs_t const anticipatedPeriod = top * kScalingFactor / bottom; nsecs_t const intercept = meanTS - (anticipatedPeriod * meanOrdinal / kScalingFactor); auto const percent = std::abs(anticipatedPeriod - mIdealPeriod) * kMaxPercent / mIdealPeriod; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index cfaabfc78c..9c1c7f7ae3 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1028,7 +1028,7 @@ bool SurfaceFlinger::performSetActiveConfig() { ATRACE_CALL(); ALOGV("performSetActiveConfig"); if (mCheckPendingFence) { - if (previousFrameMissed()) { + if (previousFramePending()) { // fence has not signaled yet. wait for the next invalidate mEventQueue->invalidate(); return true; @@ -1775,13 +1775,17 @@ void SurfaceFlinger::updateVrFlinger() { setTransactionFlags(eDisplayTransactionNeeded); } -bool SurfaceFlinger::previousFrameMissed(int graceTimeMs) NO_THREAD_SAFETY_ANALYSIS { - ATRACE_CALL(); +sp<Fence> SurfaceFlinger::previousFrameFence() NO_THREAD_SAFETY_ANALYSIS { // We are storing the last 2 present fences. If sf's phase offset is to be // woken up before the actual vsync but targeting the next vsync, we need to check // fence N-2 - const sp<Fence>& fence = mVSyncModulator->getOffsets().sf > 0 ? mPreviousPresentFences[0] - : mPreviousPresentFences[1]; + return mVSyncModulator->getOffsets().sf > 0 ? mPreviousPresentFences[0] + : mPreviousPresentFences[1]; +} + +bool SurfaceFlinger::previousFramePending(int graceTimeMs) NO_THREAD_SAFETY_ANALYSIS { + ATRACE_CALL(); + const sp<Fence>& fence = previousFrameFence(); if (fence == Fence::NO_FENCE) { return false; @@ -1794,6 +1798,16 @@ bool SurfaceFlinger::previousFrameMissed(int graceTimeMs) NO_THREAD_SAFETY_ANALY return (fence->getStatus() == Fence::Status::Unsignaled); } +nsecs_t SurfaceFlinger::previousFramePresentTime() NO_THREAD_SAFETY_ANALYSIS { + const sp<Fence>& fence = previousFrameFence(); + + if (fence == Fence::NO_FENCE) { + return Fence::SIGNAL_TIME_INVALID; + } + + return fence->getSignalTime(); +} + void SurfaceFlinger::populateExpectedPresentTime() { DisplayStatInfo stats; mScheduler->getDisplayStatInfo(&stats); @@ -1811,6 +1825,7 @@ void SurfaceFlinger::onMessageReceived(int32_t what) NO_THREAD_SAFETY_ANALYSIS { // calculate the expected present time once and use the cached // value throughout this frame to make sure all layers are // seeing this same value. + const nsecs_t lastExpectedPresentTime = mExpectedPresentTime.load(); populateExpectedPresentTime(); // When Backpressure propagation is enabled we want to give a small grace period @@ -1821,12 +1836,32 @@ void SurfaceFlinger::onMessageReceived(int32_t what) NO_THREAD_SAFETY_ANALYSIS { (mPropagateBackpressureClientComposition || !mHadClientComposition)) ? 1 : 0; - const TracedOrdinal<bool> frameMissed = {"FrameMissed", - previousFrameMissed( - graceTimeForPresentFenceMs)}; - const TracedOrdinal<bool> hwcFrameMissed = {"HwcFrameMissed", + + // Pending frames may trigger backpressure propagation. + const TracedOrdinal<bool> framePending = {"PrevFramePending", + previousFramePending( + graceTimeForPresentFenceMs)}; + + // Frame missed counts for metrics tracking. + // A frame is missed if the prior frame is still pending. If no longer pending, + // then we still count the frame as missed if the predicted present time + // was further in the past than when the fence actually fired. + + // Add some slop to correct for drift. This should generally be + // smaller than a typical frame duration, but should not be so small + // that it reports reasonable drift as a missed frame. + DisplayStatInfo stats; + mScheduler->getDisplayStatInfo(&stats); + const nsecs_t frameMissedSlop = stats.vsyncPeriod / 2; + const nsecs_t previousPresentTime = previousFramePresentTime(); + const TracedOrdinal<bool> frameMissed = + {"PrevFrameMissed", + framePending || + (previousPresentTime >= 0 && + (lastExpectedPresentTime < previousPresentTime - frameMissedSlop))}; + const TracedOrdinal<bool> hwcFrameMissed = {"PrevHwcFrameMissed", mHadDeviceComposition && frameMissed}; - const TracedOrdinal<bool> gpuFrameMissed = {"GpuFrameMissed", + const TracedOrdinal<bool> gpuFrameMissed = {"PrevGpuFrameMissed", mHadClientComposition && frameMissed}; if (frameMissed) { @@ -1846,7 +1881,7 @@ void SurfaceFlinger::onMessageReceived(int32_t what) NO_THREAD_SAFETY_ANALYSIS { mGpuFrameMissedCount++; } - if (frameMissed && mPropagateBackpressure) { + if (framePending && mPropagateBackpressure) { if ((hwcFrameMissed && !gpuFrameMissed) || mPropagateBackpressureClientComposition) { signalLayerUpdate(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 44e18a7463..6ab1fcf9d5 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -844,7 +844,21 @@ private: bool isDisplayConfigAllowed(HwcConfigIndexType configId) const REQUIRES(mStateLock); - bool previousFrameMissed(int graceTimeMs = 0); + // Gets the fence for the previous frame. + // Must be called on the main thread. + sp<Fence> previousFrameFence(); + + // Whether the previous frame has not yet been presented to the display. + // If graceTimeMs is positive, this method waits for at most the provided + // grace period before reporting if the frame missed. + // Must be called on the main thread. + bool previousFramePending(int graceTimeMs = 0); + + // Returns the previous time that the frame was presented. If the frame has + // not been presented yet, then returns Fence::SIGNAL_TIME_PENDING. If there + // is no pending frame, then returns Fence::SIGNAL_TIME_INVALID. + // Must be called on the main thread. + nsecs_t previousFramePresentTime(); // Populates the expected present time for this frame. For negative offsets, performs a // correction using the predicted vsync for the next frame instead. diff --git a/services/surfaceflinger/SurfaceTracing.h b/services/surfaceflinger/SurfaceTracing.h index 4b9f7778ed..83872ed3ae 100644 --- a/services/surfaceflinger/SurfaceTracing.h +++ b/services/surfaceflinger/SurfaceTracing.h @@ -101,7 +101,7 @@ private: std::condition_variable mCanStartTrace; std::mutex& mSfLock; - uint32_t mTraceFlags GUARDED_BY(mSfLock) = TRACE_ALL; + uint32_t mTraceFlags GUARDED_BY(mSfLock) = TRACE_CRITICAL | TRACE_INPUT; const char* mWhere GUARDED_BY(mSfLock) = ""; mutable std::mutex mTraceLock; diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp index 8038eba2e1..4f59bf247c 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.cpp +++ b/services/surfaceflinger/TimeStats/TimeStats.cpp @@ -188,17 +188,16 @@ TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, TimeStats::~TimeStats() { std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->unregisterStatsPullAtomCallback( - android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); - mStatsDelegate->unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO); + mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); + mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO); } void TimeStats::onBootFinished() { std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - TimeStats::pullAtomCallback, nullptr, this); - mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - TimeStats::pullAtomCallback, nullptr, this); + mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, + nullptr, TimeStats::pullAtomCallback, this); + mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, + nullptr, TimeStats::pullAtomCallback, this); } void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) { diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h index ddebeb184b..f9bd90babd 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.h +++ b/services/surfaceflinger/TimeStats/TimeStats.h @@ -165,15 +165,15 @@ public: virtual AStatsEvent* addStatsEventToPullData(AStatsEventList* data) { return AStatsEventList_addStatsEvent(data); } - virtual void registerStatsPullAtomCallback(int32_t atom_tag, - AStatsManager_PullAtomCallback callback, - AStatsManager_PullAtomMetadata* metadata, - void* cookie) { - return AStatsManager_registerPullAtomCallback(atom_tag, callback, metadata, cookie); + virtual void setStatsPullAtomCallback(int32_t atom_tag, + AStatsManager_PullAtomMetadata* metadata, + AStatsManager_PullAtomCallback callback, + void* cookie) { + return AStatsManager_setPullAtomCallback(atom_tag, metadata, callback, cookie); } - virtual void unregisterStatsPullAtomCallback(int32_t atom_tag) { - return AStatsManager_unregisterPullAtomCallback(atom_tag); + virtual void clearStatsPullAtomCallback(int32_t atom_tag) { + return AStatsManager_clearPullAtomCallback(atom_tag); } virtual void statsEventSetAtomId(AStatsEvent* event, uint32_t atom_id) { diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp index a7a4d4837e..4f65aee43f 100644 --- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp @@ -152,9 +152,9 @@ public: struct AStatsEvent* addStatsEventToPullData(AStatsEventList*) override { return mEvent; } - void registerStatsPullAtomCallback(int32_t atom_tag, - AStatsManager_PullAtomCallback callback, - AStatsManager_PullAtomMetadata*, void* cookie) override { + void setStatsPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata*, + AStatsManager_PullAtomCallback callback, + void* cookie) override { mAtomTags.push_back(atom_tag); mCallback = callback; mCookie = cookie; @@ -164,7 +164,7 @@ public: return (*mCallback)(atom_tag, nullptr, cookie); } - MOCK_METHOD1(unregisterStatsPullAtomCallback, void(int32_t)); + MOCK_METHOD1(clearStatsPullAtomCallback, void(int32_t)); MOCK_METHOD2(statsEventSetAtomId, void(AStatsEvent*, uint32_t)); MOCK_METHOD2(statsEventWriteInt32, void(AStatsEvent*, int32_t)); MOCK_METHOD2(statsEventWriteInt64, void(AStatsEvent*, int64_t)); @@ -261,18 +261,18 @@ TEST_F(TimeStatsTest, disabledByDefault) { ASSERT_FALSE(mTimeStats->isEnabled()); } -TEST_F(TimeStatsTest, registersCallbacksAfterBoot) { +TEST_F(TimeStatsTest, setsCallbacksAfterBoot) { mTimeStats->onBootFinished(); EXPECT_THAT(mDelegate->mAtomTags, UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, android::util::SURFACEFLINGER_STATS_LAYER_INFO)); } -TEST_F(TimeStatsTest, unregistersCallbacksOnDestruction) { +TEST_F(TimeStatsTest, clearsCallbacksOnDestruction) { EXPECT_CALL(*mDelegate, - unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); + clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); EXPECT_CALL(*mDelegate, - unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO)); + clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO)); mTimeStats.reset(); } @@ -1039,8 +1039,7 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleBuckets) { insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 4, 5000000); - // Now make sure that TimeStats flushes global stats to register the - // callback. + // Now make sure that TimeStats flushes global stats to set the callback. mTimeStats->setPowerMode(HWC_POWER_MODE_NORMAL); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(3000000)); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(5000000)); diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp index f834af895c..bf2a889c89 100644 --- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp @@ -204,7 +204,7 @@ TEST_F(VSyncPredictorTest, againstOutliersDiscontinuous_500hzLowVariance) { }; auto idealPeriod = 2000000; auto expectedPeriod = 1999892; - auto expectedIntercept = 175409; + auto expectedIntercept = 86342; tracker.setPeriod(idealPeriod); for (auto const& timestamp : simulatedVsyncs) { @@ -335,8 +335,8 @@ TEST_F(VSyncPredictorTest, doesNotPredictBeforeTimePointWithHigherIntercept) { 158929706370359, }; auto const idealPeriod = 11111111; - auto const expectedPeriod = 11113500; - auto const expectedIntercept = -395335; + auto const expectedPeriod = 11113919; + auto const expectedIntercept = -1195945; tracker.setPeriod(idealPeriod); for (auto const& timestamp : simulatedVsyncs) { @@ -355,6 +355,32 @@ TEST_F(VSyncPredictorTest, doesNotPredictBeforeTimePointWithHigherIntercept) { EXPECT_THAT(prediction, Ge(timePoint)); } +// See b/151146131 +TEST_F(VSyncPredictorTest, hasEnoughPrecision) { + VSyncPredictor tracker{mPeriod, 20, kMinimumSamplesForPrediction, kOutlierTolerancePercent}; + std::vector<nsecs_t> const simulatedVsyncs{840873348817, 840890049444, 840906762675, + 840923581635, 840940161584, 840956868096, + 840973702473, 840990256277, 841007116851, + 841023722530, 841040452167, 841057073002, + 841073800920, 841090474360, 841107278632, + 841123898634, 841140750875, 841157287127, + 841591357014, 840856664232 + + }; + auto const idealPeriod = 16666666; + auto const expectedPeriod = 16698426; + auto const expectedIntercept = 58055; + + tracker.setPeriod(idealPeriod); + for (auto const& timestamp : simulatedVsyncs) { + tracker.addVsyncTimestamp(timestamp); + } + + auto [slope, intercept] = tracker.getVSyncPredictionModel(); + EXPECT_THAT(slope, IsCloseTo(expectedPeriod, mMaxRoundingError)); + EXPECT_THAT(intercept, IsCloseTo(expectedIntercept, mMaxRoundingError)); +} + TEST_F(VSyncPredictorTest, resetsWhenInstructed) { auto const idealPeriod = 10000; auto const realPeriod = 10500; @@ -390,6 +416,22 @@ TEST_F(VSyncPredictorTest, slopeAlwaysValid) { } } +constexpr nsecs_t operator""_years(unsigned long long years) noexcept { + using namespace std::chrono_literals; + return years * 365 * 24 * 3600 * + std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count(); +} +TEST_F(VSyncPredictorTest, aPhoneThatHasBeenAroundAWhileCanStillComputePeriod) { + constexpr nsecs_t timeBase = 100_years; + + for (auto i = 0; i < kHistorySize; i++) { + tracker.addVsyncTimestamp(timeBase + i * mPeriod); + } + auto [slope, intercept] = tracker.getVSyncPredictionModel(); + EXPECT_THAT(slope, IsCloseTo(mPeriod, mMaxRoundingError)); + EXPECT_THAT(intercept, Eq(0)); +} + } // namespace android::scheduler // TODO(b/129481165): remove the #pragma below and fix conversion issues diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index aae72db0fe..e607b058eb 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -29,6 +29,8 @@ #include <algorithm> #include <mutex> #include <new> +#include <string> +#include <unordered_set> #include <utility> #include <android-base/strings.h> @@ -1280,9 +1282,66 @@ VkResult EnumerateInstanceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO(b/143293104): expose extensions from implicitly enabled layers - return vulkan::driver::EnumerateInstanceExtensionProperties( - nullptr, pPropertyCount, pProperties); + // If the pLayerName is nullptr, we must advertise all instance extensions + // from all implicitly enabled layers and the driver implementation. If + // there are duplicates among layers and the driver implementation, always + // only preserve the top layer closest to the application regardless of the + // spec version. + std::vector<VkExtensionProperties> properties; + std::unordered_set<std::string> extensionNames; + + // Expose extensions from implicitly enabled layers. + const std::string layersSetting = + android::GraphicsEnv::getInstance().getDebugLayers(); + if (!layersSetting.empty()) { + std::vector<std::string> layers = + android::base::Split(layersSetting, ":"); + for (uint32_t i = 0; i < layers.size(); i++) { + const Layer* layer = FindLayer(layers[i].c_str()); + if (!layer) { + continue; + } + uint32_t count = 0; + const VkExtensionProperties* props = + GetLayerInstanceExtensions(*layer, count); + if (count > 0) { + for (uint32_t i = 0; i < count; ++i) { + if (extensionNames.emplace(props[i].extensionName).second) { + properties.push_back(props[i]); + } + } + } + } + } + + // TODO(b/143293104): Parse debug.vulkan.layers properties + + // Expose extensions from driver implementation. + { + uint32_t count = 0; + VkResult result = vulkan::driver::EnumerateInstanceExtensionProperties( + nullptr, &count, nullptr); + if (result == VK_SUCCESS && count > 0) { + std::vector<VkExtensionProperties> props(count); + result = vulkan::driver::EnumerateInstanceExtensionProperties( + nullptr, &count, props.data()); + for (auto prop : props) { + if (extensionNames.emplace(prop.extensionName).second) { + properties.push_back(prop); + } + } + } + } + + uint32_t totalCount = properties.size(); + if (!pProperties || *pPropertyCount > totalCount) { + *pPropertyCount = totalCount; + } + if (pProperties) { + std::copy(properties.data(), properties.data() + *pPropertyCount, + pProperties); + } + return *pPropertyCount < totalCount ? VK_INCOMPLETE : VK_SUCCESS; } VkResult EnumerateDeviceLayerProperties(VkPhysicalDevice physicalDevice, |