diff options
| -rw-r--r-- | include/android/sensor.h | 4 | ||||
| -rw-r--r-- | include/input/VelocityTracker.h | 34 | ||||
| -rw-r--r-- | libs/input/VelocityTracker.cpp | 202 | ||||
| -rw-r--r-- | libs/sensor/OWNERS | 2 | ||||
| -rw-r--r-- | opengl/tests/lib/include/EGLUtils.h | 178 | ||||
| -rw-r--r-- | services/sensorservice/OWNERS | 2 | ||||
| -rw-r--r-- | services/sensorservice/SensorDevice.cpp | 21 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp | 5 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/fakehwc/FakeComposerClient.h | 10 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp | 40 |
10 files changed, 454 insertions, 44 deletions
diff --git a/include/android/sensor.h b/include/android/sensor.h index a88733cac7..2db0ee7fd9 100644 --- a/include/android/sensor.h +++ b/include/android/sensor.h @@ -197,7 +197,7 @@ enum { * A sensor event. */ -/* NOTE: Must match hardware/sensors.h */ +/* NOTE: changes to these structs have to be backward compatible */ typedef struct ASensorVector { union { float v[3]; @@ -259,7 +259,7 @@ typedef struct { }; } AAdditionalInfoEvent; -/* NOTE: Must match hardware/sensors.h */ +/* NOTE: changes to this struct has to be backward compatible */ typedef struct ASensorEvent { int32_t version; /* sizeof(struct ASensorEvent) */ int32_t sensor; diff --git a/include/input/VelocityTracker.h b/include/input/VelocityTracker.h index 795f575a2e..ffa1614b55 100644 --- a/include/input/VelocityTracker.h +++ b/include/input/VelocityTracker.h @@ -264,6 +264,40 @@ private: Movement mMovements[HISTORY_SIZE]; }; +class ImpulseVelocityTrackerStrategy : public VelocityTrackerStrategy { +public: + ImpulseVelocityTrackerStrategy(); + virtual ~ImpulseVelocityTrackerStrategy(); + + virtual void clear(); + virtual void clearPointers(BitSet32 idBits); + virtual void addMovement(nsecs_t eventTime, BitSet32 idBits, + const VelocityTracker::Position* positions); + virtual bool getEstimator(uint32_t id, VelocityTracker::Estimator* outEstimator) const; + +private: + // Sample horizon. + // We don't use too much history by default since we want to react to quick + // changes in direction. + static constexpr nsecs_t HORIZON = 100 * 1000000; // 100 ms + + // Number of samples to keep. + static constexpr size_t HISTORY_SIZE = 20; + + struct Movement { + nsecs_t eventTime; + BitSet32 idBits; + VelocityTracker::Position positions[MAX_POINTERS]; + + inline const VelocityTracker::Position& getPosition(uint32_t id) const { + return positions[idBits.getIndexOfBit(id)]; + } + }; + + size_t mIndex; + Movement mMovements[HISTORY_SIZE]; +}; + } // namespace android #endif // _LIBINPUT_VELOCITY_TRACKER_H diff --git a/libs/input/VelocityTracker.cpp b/libs/input/VelocityTracker.cpp index 62acea360e..cc74b9b5cf 100644 --- a/libs/input/VelocityTracker.cpp +++ b/libs/input/VelocityTracker.cpp @@ -105,7 +105,7 @@ static std::string matrixToString(const float* a, uint32_t m, uint32_t n, bool r // this is the strategy that applications will actually use. Be very careful // when adjusting the default strategy because it can dramatically affect // (often in a bad way) the user experience. -const char* VelocityTracker::DEFAULT_STRATEGY = "lsq2"; +const char* VelocityTracker::DEFAULT_STRATEGY = "impulse"; VelocityTracker::VelocityTracker(const char* strategy) : mLastEventTime(0), mCurrentPointerIdBits(0), mActivePointerId(-1) { @@ -141,6 +141,11 @@ bool VelocityTracker::configureStrategy(const char* strategy) { } VelocityTrackerStrategy* VelocityTracker::createStrategy(const char* strategy) { + if (!strcmp("impulse", strategy)) { + // Physical model of pushing an object. Quality: VERY GOOD. + // Works with duplicate coordinates, unclean finger liftoff. + return new ImpulseVelocityTrackerStrategy(); + } if (!strcmp("lsq1", strategy)) { // 1st order least squares. Quality: POOR. // Frequently underfits the touch data especially when the finger accelerates @@ -352,9 +357,6 @@ bool VelocityTracker::getEstimator(uint32_t id, Estimator* outEstimator) const { // --- LeastSquaresVelocityTrackerStrategy --- -const nsecs_t LeastSquaresVelocityTrackerStrategy::HORIZON; -const uint32_t LeastSquaresVelocityTrackerStrategy::HISTORY_SIZE; - LeastSquaresVelocityTrackerStrategy::LeastSquaresVelocityTrackerStrategy( uint32_t degree, Weighting weighting) : mDegree(degree), mWeighting(weighting) { @@ -863,10 +865,6 @@ void IntegratingVelocityTrackerStrategy::populateEstimator(const State& state, // --- LegacyVelocityTrackerStrategy --- -const nsecs_t LegacyVelocityTrackerStrategy::HORIZON; -const uint32_t LegacyVelocityTrackerStrategy::HISTORY_SIZE; -const nsecs_t LegacyVelocityTrackerStrategy::MIN_DURATION; - LegacyVelocityTrackerStrategy::LegacyVelocityTrackerStrategy() { clear(); } @@ -979,4 +977,192 @@ bool LegacyVelocityTrackerStrategy::getEstimator(uint32_t id, return true; } +// --- ImpulseVelocityTrackerStrategy --- + +ImpulseVelocityTrackerStrategy::ImpulseVelocityTrackerStrategy() { + clear(); +} + +ImpulseVelocityTrackerStrategy::~ImpulseVelocityTrackerStrategy() { +} + +void ImpulseVelocityTrackerStrategy::clear() { + mIndex = 0; + mMovements[0].idBits.clear(); +} + +void ImpulseVelocityTrackerStrategy::clearPointers(BitSet32 idBits) { + BitSet32 remainingIdBits(mMovements[mIndex].idBits.value & ~idBits.value); + mMovements[mIndex].idBits = remainingIdBits; +} + +void ImpulseVelocityTrackerStrategy::addMovement(nsecs_t eventTime, BitSet32 idBits, + const VelocityTracker::Position* positions) { + if (++mIndex == HISTORY_SIZE) { + mIndex = 0; + } + + Movement& movement = mMovements[mIndex]; + movement.eventTime = eventTime; + movement.idBits = idBits; + uint32_t count = idBits.count(); + for (uint32_t i = 0; i < count; i++) { + movement.positions[i] = positions[i]; + } +} + +/** + * Calculate the total impulse provided to the screen and the resulting velocity. + * + * The touchscreen is modeled as a physical object. + * Initial condition is discussed below, but for now suppose that v(t=0) = 0 + * + * The kinetic energy of the object at the release is E=0.5*m*v^2 + * Then vfinal = sqrt(2E/m). The goal is to calculate E. + * + * The kinetic energy at the release is equal to the total work done on the object by the finger. + * The total work W is the sum of all dW along the path. + * + * dW = F*dx, where dx is the piece of path traveled. + * Force is change of momentum over time, F = dp/dt = m dv/dt. + * Then substituting: + * dW = m (dv/dt) * dx = m * v * dv + * + * Summing along the path, we get: + * W = sum(dW) = sum(m * v * dv) = m * sum(v * dv) + * Since the mass stays constant, the equation for final velocity is: + * vfinal = sqrt(2*sum(v * dv)) + * + * Here, + * dv : change of velocity = (v[i+1]-v[i]) + * dx : change of distance = (x[i+1]-x[i]) + * dt : change of time = (t[i+1]-t[i]) + * v : instantaneous velocity = dx/dt + * + * The final formula is: + * vfinal = sqrt(2) * sqrt(sum((v[i]-v[i-1])*|v[i]|)) for all i + * The absolute value is needed to properly account for the sign. If the velocity over a + * particular segment descreases, then this indicates braking, which means that negative + * work was done. So for two positive, but decreasing, velocities, this contribution would be + * negative and will cause a smaller final velocity. + * + * Initial condition + * There are two ways to deal with initial condition: + * 1) Assume that v(0) = 0, which would mean that the screen is initially at rest. + * This is not entirely accurate. We are only taking the past X ms of touch data, where X is + * currently equal to 100. However, a touch event that created a fling probably lasted for longer + * than that, which would mean that the user has already been interacting with the touchscreen + * and it has probably already been moving. + * 2) Assume that the touchscreen has already been moving at a certain velocity, calculate this + * initial velocity and the equivalent energy, and start with this initial energy. + * Consider an example where we have the following data, consisting of 3 points: + * time: t0, t1, t2 + * x : x0, x1, x2 + * v : 0 , v1, v2 + * Here is what will happen in each of these scenarios: + * 1) By directly applying the formula above with the v(0) = 0 boundary condition, we will get + * vfinal = sqrt(2*(|v1|*(v1-v0) + |v2|*(v2-v1))). This can be simplified since v0=0 + * vfinal = sqrt(2*(|v1|*v1 + |v2|*(v2-v1))) = sqrt(2*(v1^2 + |v2|*(v2 - v1))) + * since velocity is a real number + * 2) If we treat the screen as already moving, then it must already have an energy (per mass) + * equal to 1/2*v1^2. Then the initial energy should be 1/2*v1*2, and only the second segment + * will contribute to the total kinetic energy (since we can effectively consider that v0=v1). + * This will give the following expression for the final velocity: + * vfinal = sqrt(2*(1/2*v1^2 + |v2|*(v2-v1))) + * This analysis can be generalized to an arbitrary number of samples. + * + * + * Comparing the two equations above, we see that the only mathematical difference + * is the factor of 1/2 in front of the first velocity term. + * This boundary condition would allow for the "proper" calculation of the case when all of the + * samples are equally spaced in time and distance, which should suggest a constant velocity. + * + * Note that approach 2) is sensitive to the proper ordering of the data in time, since + * the boundary condition must be applied to the oldest sample to be accurate. + */ +static float calculateImpulseVelocity(const nsecs_t* t, const float* x, size_t count) { + // The input should be in reversed time order (most recent sample at index i=0) + // t[i] is in nanoseconds, but due to FP arithmetic, convert to seconds inside this function + static constexpr float NANOS_PER_SECOND = 1E-9; + static constexpr float sqrt2 = 1.41421356237; + + if (count < 2) { + return 0; // if 0 or 1 points, velocity is zero + } + if (t[1] > t[0]) { // Algorithm will still work, but not perfectly + ALOGE("Samples provided to calculateImpulseVelocity in the wrong order"); + } + if (count == 2) { // if 2 points, basic linear calculation + if (t[1] == t[0]) { + ALOGE("Events have identical time stamps t=%" PRId64 ", setting velocity = 0", t[0]); + return 0; + } + return (x[1] - x[0]) / (NANOS_PER_SECOND * (t[1] - t[0])); + } + // Guaranteed to have at least 3 points here + float work = 0; + float vprev, vcurr; // v[i-1] and v[i], respectively + vprev = 0; + for (size_t i = count - 1; i > 0 ; i--) { // start with the oldest sample and go forward in time + if (t[i] == t[i-1]) { + ALOGE("Events have identical time stamps t=%" PRId64 ", skipping sample", t[i]); + continue; + } + vcurr = (x[i] - x[i-1]) / (NANOS_PER_SECOND * (t[i] - t[i-1])); + work += (vcurr - vprev) * fabsf(vcurr); + if (i == count - 1) { + work *= 0.5; // initial condition, case 2) above + } + vprev = vcurr; + } + return (work < 0 ? -1.0 : 1.0) * sqrtf(fabsf(work)) * sqrt2; +} + +bool ImpulseVelocityTrackerStrategy::getEstimator(uint32_t id, + VelocityTracker::Estimator* outEstimator) const { + outEstimator->clear(); + + // Iterate over movement samples in reverse time order and collect samples. + float x[HISTORY_SIZE]; + float y[HISTORY_SIZE]; + nsecs_t time[HISTORY_SIZE]; + size_t m = 0; // number of points that will be used for fitting + size_t index = mIndex; + const Movement& newestMovement = mMovements[mIndex]; + do { + const Movement& movement = mMovements[index]; + if (!movement.idBits.hasBit(id)) { + break; + } + + nsecs_t age = newestMovement.eventTime - movement.eventTime; + if (age > HORIZON) { + break; + } + + const VelocityTracker::Position& position = movement.getPosition(id); + x[m] = position.x; + y[m] = position.y; + time[m] = movement.eventTime; + index = (index == 0 ? HISTORY_SIZE : index) - 1; + } while (++m < HISTORY_SIZE); + + if (m == 0) { + return false; // no data + } + outEstimator->xCoeff[0] = 0; + outEstimator->yCoeff[0] = 0; + outEstimator->xCoeff[1] = calculateImpulseVelocity(time, x, m); + outEstimator->yCoeff[1] = calculateImpulseVelocity(time, y, m); + outEstimator->xCoeff[2] = 0; + outEstimator->yCoeff[2] = 0; + outEstimator->time = newestMovement.eventTime; + outEstimator->degree = 2; // similar results to 2nd degree fit + outEstimator->confidence = 1; +#if DEBUG_STRATEGY + ALOGD("velocity: (%f, %f)", outEstimator->xCoeff[1], outEstimator->yCoeff[1]); +#endif + return true; +} + } // namespace android diff --git a/libs/sensor/OWNERS b/libs/sensor/OWNERS new file mode 100644 index 0000000000..6a38a1ff14 --- /dev/null +++ b/libs/sensor/OWNERS @@ -0,0 +1,2 @@ +ashutoshj@google.com +pengxu@google.com diff --git a/opengl/tests/lib/include/EGLUtils.h b/opengl/tests/lib/include/EGLUtils.h index 014c2611ae..29f4fe442d 100644 --- a/opengl/tests/lib/include/EGLUtils.h +++ b/opengl/tests/lib/include/EGLUtils.h @@ -20,11 +20,16 @@ #include <stdint.h> #include <stdlib.h> +#include <vector> +#include <EGL/egl.h> +#include <EGL/eglext.h> +#include <GLES2/gl2.h> #include <system/window.h> #include <utils/Errors.h> -#include <EGL/egl.h> +#include <utils/String8.h> +EGLAPI const char* eglQueryStringImplementationANDROID(EGLDisplay dpy, EGLint name); // ---------------------------------------------------------------------------- namespace android { @@ -47,6 +52,17 @@ public: EGLint const* attrs, EGLNativeWindowType window, EGLConfig* outConfig); + + static inline String8 printGLString(const char* name, GLenum s); + static inline String8 printEGLString(EGLDisplay dpy, const char* name, GLenum s); + static inline String8 checkEglError(const char* op, EGLBoolean returnVal); + static inline String8 checkGlError(const char* op); + static inline String8 printEGLConfiguration(EGLDisplay dpy, EGLConfig config); + static inline bool printEGLConfigurations(EGLDisplay dpy, String8& msg); + static inline bool printEGLConfigurations(FILE* output, EGLDisplay dpy); + static inline String8 decodeColorSpace(EGLint colorSpace); + static inline bool hasEglExtension(EGLDisplay dpy, const char* name); + static inline bool hasExtension(const char* exts, const char* name); }; // ---------------------------------------------------------------------------- @@ -91,9 +107,8 @@ status_t EGLUtils::selectConfigForPixelFormat( if (eglGetConfigs(dpy, NULL, 0, &numConfigs) == EGL_FALSE) return BAD_VALUE; - EGLConfig* const configs = (EGLConfig*)malloc(sizeof(EGLConfig)*numConfigs); - if (eglChooseConfig(dpy, attrs, configs, numConfigs, &n) == EGL_FALSE) { - free(configs); + std::vector<EGLConfig> configs(numConfigs); + if (eglChooseConfig(dpy, attrs, configs.data(), numConfigs, &n) == EGL_FALSE) { return BAD_VALUE; } @@ -108,8 +123,6 @@ status_t EGLUtils::selectConfigForPixelFormat( } } - free(configs); - if (i<n) { *outConfig = config; return NO_ERROR; @@ -137,6 +150,159 @@ status_t EGLUtils::selectConfigForNativeWindow( return selectConfigForPixelFormat(dpy, attrs, format, outConfig); } +String8 EGLUtils::printGLString(const char* name, GLenum s) { + String8 msg; + const char* v = reinterpret_cast<const char*>(glGetString(s)); + msg.appendFormat("GL %s = %s\n", name, v); + return msg; +} + +String8 EGLUtils::printEGLString(EGLDisplay dpy, const char* name, GLenum s) { + String8 msg; + const char* v = static_cast<const char*>(eglQueryString(dpy, s)); + msg.appendFormat("GL %s = %s\n", name, v); + const char* va = (const char*)eglQueryStringImplementationANDROID(dpy, s); + msg.appendFormat("ImplementationANDROID: %s = %s\n", name, va); + return msg; +} + +String8 EGLUtils::checkEglError(const char* op, EGLBoolean returnVal = EGL_TRUE) { + String8 msg; + if (returnVal != EGL_TRUE) { + msg.appendFormat("%s() returned %d\n", op, returnVal); + } + + for (EGLint error = eglGetError(); error != EGL_SUCCESS; error = eglGetError()) { + msg.appendFormat("after %s() eglError %s (0x%x)\n", op, EGLUtils::strerror(error), error); + } + return msg; +} + +String8 EGLUtils::checkGlError(const char* op) { + String8 msg; + for (GLint error = glGetError(); error != GL_NO_ERROR; error = glGetError()) { + msg.appendFormat("after %s() glError (0x%x)\n", op, error); + } + return msg; +} + +String8 EGLUtils::printEGLConfiguration(EGLDisplay dpy, EGLConfig config) { +#define X(VAL) \ + { VAL, #VAL } + struct { + EGLint attribute; + const char* name; + } names[] = { + X(EGL_BUFFER_SIZE), + X(EGL_ALPHA_SIZE), + X(EGL_BLUE_SIZE), + X(EGL_GREEN_SIZE), + X(EGL_RED_SIZE), + X(EGL_DEPTH_SIZE), + X(EGL_STENCIL_SIZE), + X(EGL_CONFIG_CAVEAT), + X(EGL_CONFIG_ID), + X(EGL_LEVEL), + X(EGL_MAX_PBUFFER_HEIGHT), + X(EGL_MAX_PBUFFER_PIXELS), + X(EGL_MAX_PBUFFER_WIDTH), + X(EGL_NATIVE_RENDERABLE), + X(EGL_NATIVE_VISUAL_ID), + X(EGL_NATIVE_VISUAL_TYPE), + X(EGL_SAMPLES), + X(EGL_SAMPLE_BUFFERS), + X(EGL_SURFACE_TYPE), + X(EGL_TRANSPARENT_TYPE), + X(EGL_TRANSPARENT_RED_VALUE), + X(EGL_TRANSPARENT_GREEN_VALUE), + X(EGL_TRANSPARENT_BLUE_VALUE), + X(EGL_BIND_TO_TEXTURE_RGB), + X(EGL_BIND_TO_TEXTURE_RGBA), + X(EGL_MIN_SWAP_INTERVAL), + X(EGL_MAX_SWAP_INTERVAL), + X(EGL_LUMINANCE_SIZE), + X(EGL_ALPHA_MASK_SIZE), + X(EGL_COLOR_BUFFER_TYPE), + X(EGL_RENDERABLE_TYPE), + X(EGL_CONFORMANT), + }; +#undef X + + String8 msg; + for (size_t j = 0; j < sizeof(names) / sizeof(names[0]); j++) { + EGLint value = -1; + EGLint returnVal = eglGetConfigAttrib(dpy, config, names[j].attribute, &value); + EGLint error = eglGetError(); + if (returnVal && error == EGL_SUCCESS) { + msg.appendFormat(" %s: %d (0x%x)", names[j].name, value, value); + } + } + msg.append("\n"); + return msg; +} + +bool EGLUtils::printEGLConfigurations(EGLDisplay dpy, String8& msg) { + EGLint numConfig = 0; + EGLint returnVal = eglGetConfigs(dpy, NULL, 0, &numConfig); + msg.append(checkEglError("eglGetConfigs", returnVal)); + if (!returnVal) { + return false; + } + + msg.appendFormat("Number of EGL configuration: %d\n", numConfig); + + std::vector<EGLConfig> configs(numConfig); + + returnVal = eglGetConfigs(dpy, configs.data(), numConfig, &numConfig); + msg.append(checkEglError("eglGetConfigs", returnVal)); + if (!returnVal) { + return false; + } + + for (int i = 0; i < numConfig; i++) { + msg.appendFormat("Configuration %d\n", i); + msg.append(printEGLConfiguration(dpy, configs[i])); + } + + return true; +} + +bool EGLUtils::printEGLConfigurations(FILE* output, EGLDisplay dpy) { + String8 msg; + bool status = printEGLConfigurations(dpy, msg); + fprintf(output, "%s", msg.c_str()); + return status; +} + +String8 EGLUtils::decodeColorSpace(EGLint colorSpace) { + switch (colorSpace) { + case EGL_GL_COLORSPACE_SRGB_KHR: + return String8("EGL_GL_COLORSPACE_SRGB_KHR"); + case EGL_GL_COLORSPACE_DISPLAY_P3_EXT: + return String8("EGL_GL_COLORSPACE_DISPLAY_P3_EXT"); + case EGL_GL_COLORSPACE_LINEAR_KHR: + return String8("EGL_GL_COLORSPACE_LINEAR_KHR"); + default: + return String8::format("UNKNOWN ColorSpace %d", colorSpace); + } +} + +bool EGLUtils::hasExtension(const char* exts, const char* name) { + size_t nameLen = strlen(name); + if (exts) { + for (const char* match = strstr(exts, name); match; match = strstr(match + nameLen, name)) { + if (match[nameLen] == '\0' || match[nameLen] == ' ') { + return true; + } + } + } + return false; +} + +bool EGLUtils::hasEglExtension(EGLDisplay dpy, const char* name) { + return hasExtension(eglQueryString(dpy, EGL_EXTENSIONS), name); +} + // ---------------------------------------------------------------------------- }; // namespace android // ---------------------------------------------------------------------------- diff --git a/services/sensorservice/OWNERS b/services/sensorservice/OWNERS new file mode 100644 index 0000000000..6a38a1ff14 --- /dev/null +++ b/services/sensorservice/OWNERS @@ -0,0 +1,2 @@ +ashutoshj@google.com +pengxu@google.com diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index 7d9b0b730a..da3b2758d1 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -223,8 +223,13 @@ ssize_t SensorDevice::poll(sensors_event_t* buffer, size_t count) { } void SensorDevice::autoDisable(void *ident, int handle) { - Info& info( mActivationCount.editValueFor(handle) ); Mutex::Autolock _l(mLock); + ssize_t activationIndex = mActivationCount.indexOfKey(handle); + if (activationIndex < 0) { + ALOGW("Handle %d cannot be found in activation record", handle); + return; + } + Info& info(mActivationCount.editValueAt(activationIndex)); info.removeBatchParamsForIdent(ident); } @@ -235,7 +240,12 @@ status_t SensorDevice::activate(void* ident, int handle, int enabled) { bool actuateHardware = false; Mutex::Autolock _l(mLock); - Info& info( mActivationCount.editValueFor(handle) ); + ssize_t activationIndex = mActivationCount.indexOfKey(handle); + if (activationIndex < 0) { + ALOGW("Handle %d cannot be found in activation record", handle); + return BAD_VALUE; + } + Info& info(mActivationCount.editValueAt(activationIndex)); ALOGD_IF(DEBUG_CONNECTIONS, "SensorDevice::activate: ident=%p, handle=0x%08x, enabled=%d, count=%zu", @@ -329,7 +339,12 @@ status_t SensorDevice::batch( ident, handle, flags, samplingPeriodNs, maxBatchReportLatencyNs); Mutex::Autolock _l(mLock); - Info& info(mActivationCount.editValueFor(handle)); + ssize_t activationIndex = mActivationCount.indexOfKey(handle); + if (activationIndex < 0) { + ALOGW("Handle %d cannot be found in activation record", handle); + return BAD_VALUE; + } + Info& info(mActivationCount.editValueAt(activationIndex)); if (info.batchParams.indexOfKey(ident) < 0) { BatchParams params(samplingPeriodNs, maxBatchReportLatencyNs); diff --git a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp index 60916f3ab9..d97ffa3148 100644 --- a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp +++ b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.cpp @@ -36,7 +36,6 @@ #include <thread> constexpr Config NULL_DISPLAY_CONFIG = static_cast<Config>(0); -constexpr Display DEFAULT_DISPLAY = static_cast<Display>(1); using namespace sftest; @@ -168,7 +167,7 @@ void FakeComposerClient::enableCallback(bool enable) { ALOGV("enableCallback"); mCallbacksOn = enable; if (mCallbacksOn) { - mClient->onHotplug(DEFAULT_DISPLAY, IComposerCallback::Connection::CONNECTED); + mClient->onHotplug(PRIMARY_DISPLAY, IComposerCallback::Connection::CONNECTED); } } @@ -507,7 +506,7 @@ void FakeComposerClient::requestVSync(uint64_t vsyncTime) { if (mSurfaceComposer != nullptr) { mSurfaceComposer->injectVSync(timestamp); } else { - mClient->onVsync(DEFAULT_DISPLAY, timestamp); + mClient->onVsync(PRIMARY_DISPLAY, timestamp); } } } diff --git a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h index 294abb2c59..2a5a8ad03f 100644 --- a/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h +++ b/services/surfaceflinger/tests/fakehwc/FakeComposerClient.h @@ -19,6 +19,9 @@ #include "ComposerClient.h" #include "RenderState.h" +// Needed for display type/ID enums +#include <hardware/hwcomposer_defs.h> + #include <utils/Condition.h> #include <chrono> @@ -40,6 +43,13 @@ class SurfaceComposerClient; namespace sftest { +// NOTE: The ID's need to be exactly these. VR composer and parts of +// the SurfaceFlinger assume the display IDs to have these values +// despite the enum being documented as a display type. +// TODO: Reference to actual documentation +constexpr Display PRIMARY_DISPLAY = static_cast<Display>(HWC_DISPLAY_PRIMARY); +constexpr Display EXTERNAL_DISPLAY = static_cast<Display>(HWC_DISPLAY_EXTERNAL); + class FakeComposerClient : public ComposerBase { public: FakeComposerClient(); diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp index 8902ede301..9ac3331892 100644 --- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp +++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp @@ -168,17 +168,15 @@ void DisplayTest::SetUp() { android::hardware::ProcessState::self()->startThreadPool(); android::ProcessState::self()->startThreadPool(); - EXPECT_CALL(*mMockComposer, getDisplayType(1, _)) + EXPECT_CALL(*mMockComposer, getDisplayType(PRIMARY_DISPLAY, _)) .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayType::PHYSICAL), Return(Error::NONE))); - // Seems to be doubled right now, once for display ID 1 and once for 0. This sounds fishy - // but encoding that here exactly. - EXPECT_CALL(*mMockComposer, getDisplayAttribute(1, 1, _, _)) - .Times(5) - .WillRepeatedly(Invoke(mMockComposer, &MockComposerClient::getDisplayAttributeFake)); - // TODO: Find out what code is generating the ID 0. - EXPECT_CALL(*mMockComposer, getDisplayAttribute(0, 1, _, _)) - .Times(5) + // Primary display will be queried twice for all 5 attributes. One + // set of queries comes from the SurfaceFlinger proper an the + // other set from the VR composer. + // TODO: Is VR composer always present? Change to atLeast(5)? + EXPECT_CALL(*mMockComposer, getDisplayAttribute(PRIMARY_DISPLAY, 1, _, _)) + .Times(2 * 5) .WillRepeatedly(Invoke(mMockComposer, &MockComposerClient::getDisplayAttributeFake)); startSurfaceFlinger(); @@ -207,31 +205,32 @@ void DisplayTest::TearDown() { TEST_F(DisplayTest, Hotplug) { ALOGD("DisplayTest::Hotplug"); - EXPECT_CALL(*mMockComposer, getDisplayType(2, _)) + EXPECT_CALL(*mMockComposer, getDisplayType(EXTERNAL_DISPLAY, _)) .Times(2) .WillRepeatedly(DoAll(SetArgPointee<1>(IComposerClient::DisplayType::PHYSICAL), Return(Error::NONE))); // The attribute queries will get done twice. This is for defaults - EXPECT_CALL(*mMockComposer, getDisplayAttribute(2, 1, _, _)) + EXPECT_CALL(*mMockComposer, getDisplayAttribute(EXTERNAL_DISPLAY, 1, _, _)) .Times(2 * 3) .WillRepeatedly(Invoke(mMockComposer, &MockComposerClient::getDisplayAttributeFake)); // ... and then special handling for dimensions. Specifying this // rules later means that gmock will try them first, i.e., // ordering of width/height vs. the default implementation for // other queries is significant. - EXPECT_CALL(*mMockComposer, getDisplayAttribute(2, 1, IComposerClient::Attribute::WIDTH, _)) + EXPECT_CALL(*mMockComposer, + getDisplayAttribute(EXTERNAL_DISPLAY, 1, IComposerClient::Attribute::WIDTH, _)) .Times(2) .WillRepeatedly(DoAll(SetArgPointee<3>(400), Return(Error::NONE))); - EXPECT_CALL(*mMockComposer, getDisplayAttribute(2, 1, IComposerClient::Attribute::HEIGHT, _)) + EXPECT_CALL(*mMockComposer, + getDisplayAttribute(EXTERNAL_DISPLAY, 1, IComposerClient::Attribute::HEIGHT, _)) .Times(2) .WillRepeatedly(DoAll(SetArgPointee<3>(200), Return(Error::NONE))); // TODO: Width and height queries are not actually called. Display // info returns dimensions 0x0 in display info. Why? - mMockComposer->hotplugDisplay(static_cast<Display>(2), - IComposerCallback::Connection::CONNECTED); + mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::CONNECTED); { sp<android::IBinder> display( @@ -257,13 +256,11 @@ TEST_F(DisplayTest, Hotplug) { } } - mMockComposer->hotplugDisplay(static_cast<Display>(2), - IComposerCallback::Connection::DISCONNECTED); + mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::DISCONNECTED); mMockComposer->clearFrames(); - mMockComposer->hotplugDisplay(static_cast<Display>(2), - IComposerCallback::Connection::CONNECTED); + mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::CONNECTED); { sp<android::IBinder> display( @@ -288,8 +285,7 @@ TEST_F(DisplayTest, Hotplug) { ASSERT_EQ(NO_ERROR, surfaceControl->show()); } } - mMockComposer->hotplugDisplay(static_cast<Display>(2), - IComposerCallback::Connection::DISCONNECTED); + mMockComposer->hotplugDisplay(EXTERNAL_DISPLAY, IComposerCallback::Connection::DISCONNECTED); } //////////////////////////////////////////////// @@ -664,7 +660,7 @@ TEST_F(TransactionTest, LayerSetMatrix) { {{0.f, 1.f, 1.f, 0.f}, HWC_TRANSFORM_FLIP_H_ROT_90, {64, 64, 128, 128}}, {{0.f, 1.f, 1.f, 0.f}, HWC_TRANSFORM_FLIP_V_ROT_90, {64, 64, 128, 128}}}; // clang-format on - constexpr int TEST_COUNT = sizeof(MATRIX_TESTS)/sizeof(matrixTestData); + constexpr int TEST_COUNT = sizeof(MATRIX_TESTS) / sizeof(matrixTestData); for (int i = 0; i < TEST_COUNT; i++) { // TODO: How to leverage the HWC2 stringifiers? |