From 07f190f7ba5f3a41299411ef39736ffc21c9a8c4 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Thu, 23 Aug 2018 13:44:43 -0700 Subject: surfaceflinger: reorder width and height in RenderArea ctor Height is before width only in a crazy world. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: Ia10b26dbba9a6a91abb5dae9fbe20bf17cd3e78f Merged-In: Ia10b26dbba9a6a91abb5dae9fbe20bf17cd3e78f --- services/surfaceflinger/SurfaceFlinger.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 4ec1e7f166..f523504748 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4845,7 +4845,7 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp& layerHandleBinder, public: LayerRenderArea(SurfaceFlinger* flinger, const sp& layer, const Rect crop, int32_t reqWidth, int32_t reqHeight, bool childrenOnly) - : RenderArea(reqHeight, reqWidth, CaptureFill::CLEAR), + : RenderArea(reqWidth, reqHeight, CaptureFill::CLEAR), mLayer(layer), mCrop(crop), mFlinger(flinger), -- cgit v1.2.3-59-g8ed1b From 82144eb9bfe26c63d8ec3bc0546f0a2b12555306 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Fri, 24 Aug 2018 15:34:02 -0700 Subject: surfaceflinger: remove ISurfaceComposer.h from RenderArea RenderArea can be made more generic by removing ISurfaceComposer.h dependency. The caller also prefers to work with ui::Transform::orientation_flags than ISurfaceComposer::Rotation (we want to move updateDimensions to the caller). Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: I16e1392d5c92c2f423f98307e867918415404d26 Merged-In: I16e1392d5c92c2f423f98307e867918415404d26 --- services/surfaceflinger/DisplayDevice.h | 5 ++--- services/surfaceflinger/RenderArea.h | 15 +++++++-------- services/surfaceflinger/SurfaceFlinger.cpp | 26 +++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 12 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index f73654c482..6e0ab578cf 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -25,7 +25,6 @@ #include #include -#include #include #include #include @@ -337,11 +336,11 @@ struct DisplayDeviceState { class DisplayRenderArea : public RenderArea { public: DisplayRenderArea(const sp device, - ISurfaceComposer::Rotation rotation = ISurfaceComposer::eRotateNone) + Transform::orientation_flags rotation = Transform::ROT_0) : DisplayRenderArea(device, device->getBounds(), device->getWidth(), device->getHeight(), rotation) {} DisplayRenderArea(const sp device, Rect sourceCrop, uint32_t reqWidth, - uint32_t reqHeight, ISurfaceComposer::Rotation rotation) + uint32_t reqHeight, Transform::orientation_flags rotation) : RenderArea(reqWidth, reqHeight, CaptureFill::OPAQUE, rotation), mDevice(device), mSourceCrop(sourceCrop) {} diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index 38f4cbc639..c343846128 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -1,7 +1,5 @@ #pragma once -#include - #include "Transform.h" #include @@ -21,10 +19,11 @@ public: static float getCaptureFillValue(CaptureFill captureFill); RenderArea(uint32_t reqWidth, uint32_t reqHeight, CaptureFill captureFill, - ISurfaceComposer::Rotation rotation = ISurfaceComposer::eRotateNone) - : mReqWidth(reqWidth), mReqHeight(reqHeight), mCaptureFill(captureFill) { - mRotationFlags = Transform::fromRotation(rotation); - } + Transform::orientation_flags rotation = Transform::ROT_0) + : mReqWidth(reqWidth), + mReqHeight(reqHeight), + mCaptureFill(captureFill), + mRotationFlags(rotation) {} virtual ~RenderArea() = default; @@ -76,8 +75,8 @@ public: private: uint32_t mReqWidth; uint32_t mReqHeight; - Transform::orientation_flags mRotationFlags; - CaptureFill mCaptureFill; + const CaptureFill mCaptureFill; + const Transform::orientation_flags mRotationFlags; }; } // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f523504748..a0cacd4ec6 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -75,6 +75,7 @@ #include "LayerVector.h" #include "MonitoredProducer.h" #include "SurfaceFlinger.h" +#include "Transform.h" #include "clz.h" #include "DisplayHardware/ComposerHal.h" @@ -112,6 +113,27 @@ using ui::Hdr; using ui::RenderIntent; namespace { + +#pragma clang diagnostic push +#pragma clang diagnostic error "-Wswitch-enum" + +Transform::orientation_flags fromSurfaceComposerRotation(ISurfaceComposer::Rotation rotation) { + switch (rotation) { + case ISurfaceComposer::eRotateNone: + return Transform::ROT_0; + case ISurfaceComposer::eRotate90: + return Transform::ROT_90; + case ISurfaceComposer::eRotate180: + return Transform::ROT_180; + case ISurfaceComposer::eRotate270: + return Transform::ROT_270; + } + ALOGE("Invalid rotation passed to captureScreen(): %d\n", rotation); + return Transform::ROT_0; +} + +#pragma clang diagnostic pop + class ConditionalLock { public: ConditionalLock(Mutex& mutex, bool lock) : mMutex(mutex), mLocked(lock) { @@ -4832,6 +4854,8 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp device(getDisplayDeviceLocked(display)); if (CC_UNLIKELY(device == 0)) return BAD_VALUE; @@ -4845,7 +4869,7 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp Date: Thu, 23 Aug 2018 12:55:44 -0700 Subject: surfaceflinger: fix race conditions in captureScreen The display was looked up without holding the state lock. It was also used (including indirectly from updateDimensions) without holding the lock either. Inline updateDimensions in captureScreen with the state lock held. Stop calling updateDimensions in captureLayers. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: I8b361847c44373ce08930d906ec4a995efd1c21b Merged-In: I8b361847c44373ce08930d906ec4a995efd1c21b --- services/surfaceflinger/RenderArea.cpp | 34 ------------------ services/surfaceflinger/RenderArea.h | 6 ++-- services/surfaceflinger/SurfaceFlinger.cpp | 56 ++++++++++++++++++++++++------ 3 files changed, 47 insertions(+), 49 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/RenderArea.cpp b/services/surfaceflinger/RenderArea.cpp index 1a8edf3e79..93759e8ad7 100644 --- a/services/surfaceflinger/RenderArea.cpp +++ b/services/surfaceflinger/RenderArea.cpp @@ -1,7 +1,5 @@ #include "RenderArea.h" -#include - namespace android { float RenderArea::getCaptureFillValue(CaptureFill captureFill) { @@ -13,37 +11,5 @@ float RenderArea::getCaptureFillValue(CaptureFill captureFill) { return 1.0f; } } -/* - * Checks that the requested width and height are valid and updates them to the render area - * dimensions if they are set to 0 - */ -status_t RenderArea::updateDimensions(int displayRotation) { - // get screen geometry - - uint32_t width = getWidth(); - uint32_t height = getHeight(); - - if (mRotationFlags & Transform::ROT_90) { - std::swap(width, height); - } - - if (displayRotation & DisplayState::eOrientationSwapMask) { - std::swap(width, height); - } - - if ((mReqWidth > width) || (mReqHeight > height)) { - ALOGE("size mismatch (%d, %d) > (%d, %d)", mReqWidth, mReqHeight, width, height); - return BAD_VALUE; - } - - if (mReqWidth == 0) { - mReqWidth = width; - } - if (mReqHeight == 0) { - mReqHeight = height; - } - - return NO_ERROR; -} } // namespace android diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index c343846128..d980bd53c8 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -70,11 +70,9 @@ public: // covered by any rendered layer should be filled with this color. CaptureFill getCaptureFill() const { return mCaptureFill; }; - status_t updateDimensions(int displayRotation); - private: - uint32_t mReqWidth; - uint32_t mReqHeight; + const uint32_t mReqWidth; + const uint32_t mReqHeight; const CaptureFill mCaptureFill; const Transform::orientation_flags mRotationFlags; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a0cacd4ec6..5e2b974e2c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4856,16 +4856,44 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp device(getDisplayDeviceLocked(display)); - if (CC_UNLIKELY(device == 0)) return BAD_VALUE; + sp device; + { + Mutex::Autolock _l(mStateLock); + + device = getDisplayDeviceLocked(display); + if (!device) return BAD_VALUE; + + const Rect& dispScissor = device->getScissor(); + if (!dispScissor.isEmpty()) { + sourceCrop.set(dispScissor); + // adb shell screencap will default reqWidth and reqHeight to zeros. + if (reqWidth == 0 || reqHeight == 0) { + reqWidth = uint32_t(device->getViewport().width()); + reqHeight = uint32_t(device->getViewport().height()); + } + } + + // get screen geometry + uint32_t width = device->getWidth(); + uint32_t height = device->getHeight(); + + if (renderAreaRotation & Transform::ROT_90) { + std::swap(width, height); + } - const Rect& dispScissor = device->getScissor(); - if (!dispScissor.isEmpty()) { - sourceCrop.set(dispScissor); - // adb shell screencap will default reqWidth and reqHeight to zeros. - if (reqWidth == 0 || reqHeight == 0) { - reqWidth = uint32_t(device->getViewport().width()); - reqHeight = uint32_t(device->getViewport().height()); + if (mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) { + std::swap(width, height); + } + + if ((reqWidth > width) || (reqHeight > height)) { + ALOGE("size mismatch (%d, %d) > (%d, %d)", reqWidth, reqHeight, width, height); + } else { + if (reqWidth == 0) { + reqWidth = width; + } + if (reqHeight == 0) { + reqHeight = height; + } } } @@ -4975,6 +5003,14 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, int32_t reqWidth = crop.width() * frameScale; int32_t reqHeight = crop.height() * frameScale; + // really small crop or frameScale + if (reqWidth <= 0) { + reqWidth = 1; + } + if (reqHeight <= 0) { + reqHeight = 1; + } + LayerRenderArea renderArea(this, parent, crop, reqWidth, reqHeight, childrenOnly); auto traverseLayers = [parent, childrenOnly](const LayerVector::Visitor& visitor) { @@ -4996,8 +5032,6 @@ status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, bool useIdentityTransform) { ATRACE_CALL(); - renderArea.updateDimensions(mPrimaryDisplayOrientation); - const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE; *outBuffer = new GraphicBuffer(renderArea.getReqWidth(), renderArea.getReqHeight(), -- cgit v1.2.3-59-g8ed1b From 89e1ec278c751fe7e2544b1ad8fae7cb9da3b9fd Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 27 Aug 2018 15:40:25 -0700 Subject: surfaceflinger: silence some RenderArea errors Allow physical size and source crop size to be larger than the logical size. Bug: 113041375 Test: builds Change-Id: I5a0e2158d114a6e06c8241b48d90bbb6172216cc Merged-In: I5a0e2158d114a6e06c8241b48d90bbb6172216cc --- services/surfaceflinger/SurfaceFlinger.cpp | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 5e2b974e2c..b91c6c57fb 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4885,15 +4885,11 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp width) || (reqHeight > height)) { - ALOGE("size mismatch (%d, %d) > (%d, %d)", reqWidth, reqHeight, width, height); - } else { - if (reqWidth == 0) { - reqWidth = width; - } - if (reqHeight == 0) { - reqHeight = height; - } + if (reqWidth == 0) { + reqWidth = width; + } + if (reqHeight == 0) { + reqHeight = height; } } @@ -5145,20 +5141,6 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, sourceCrop = tr.transform(sourceCrop); } - // ensure that sourceCrop is inside screen - if (sourceCrop.left < 0) { - ALOGE("Invalid crop rect: l = %d (< 0)", sourceCrop.left); - } - if (sourceCrop.right > raWidth) { - ALOGE("Invalid crop rect: r = %d (> %d)", sourceCrop.right, raWidth); - } - if (sourceCrop.top < 0) { - ALOGE("Invalid crop rect: t = %d (< 0)", sourceCrop.top); - } - if (sourceCrop.bottom > raHeight) { - ALOGE("Invalid crop rect: b = %d (> %d)", sourceCrop.bottom, raHeight); - } - // assume ColorMode::SRGB / RenderIntent::COLORIMETRIC engine.setOutputDataSpace(Dataspace::SRGB); engine.setDisplayMaxLuminance(DisplayDevice::sDefaultMaxLumiance); -- cgit v1.2.3-59-g8ed1b From 6df5400efeb34bea66f5c7a2bd2626dd2b00e835 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 28 Aug 2018 12:57:23 -0700 Subject: surfaceflinger: clean up captureScreen dispScissor is never empty. Remove the check and update the comments so that the intention is clear. It also becomes obvious that reqWidth/reqHeight is never zero when we reach "// get screen geometry". All code following the comment is actually dead, which means we probably broke it since commit 06a58e22. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: Ida1430383ce62271365d9ef64ad9b055638e1eac Merged-In: Ida1430383ce62271365d9ef64ad9b055638e1eac --- services/surfaceflinger/SurfaceFlinger.cpp | 34 ++++++++---------------------- 1 file changed, 9 insertions(+), 25 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index b91c6c57fb..0d1cf1c86d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4863,34 +4863,18 @@ status_t SurfaceFlinger::captureScreen(const sp& display, spgetScissor(); - if (!dispScissor.isEmpty()) { - sourceCrop.set(dispScissor); - // adb shell screencap will default reqWidth and reqHeight to zeros. - if (reqWidth == 0 || reqHeight == 0) { - reqWidth = uint32_t(device->getViewport().width()); - reqHeight = uint32_t(device->getViewport().height()); - } - } + // set the source crop to the (projected) logical display viewport + // unconditionally until the framework is fixed + sourceCrop.set(device->getScissor()); - // get screen geometry - uint32_t width = device->getWidth(); - uint32_t height = device->getHeight(); - - if (renderAreaRotation & Transform::ROT_90) { - std::swap(width, height); + // set the requested width/height to the logical display viewport size + // by default + if (reqWidth == 0 || reqHeight == 0) { + reqWidth = uint32_t(device->getViewport().width()); + reqHeight = uint32_t(device->getViewport().height()); } - if (mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) { - std::swap(width, height); - } - - if (reqWidth == 0) { - reqWidth = width; - } - if (reqHeight == 0) { - reqHeight = height; - } + // XXX mPrimaryDisplayOrientation is ignored } DisplayRenderArea renderArea(device, sourceCrop, reqWidth, reqHeight, renderAreaRotation); -- cgit v1.2.3-59-g8ed1b From d63fd945a53f94e2c89e5f3fb97289e74ee8a1b1 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 27 Aug 2018 14:38:14 -0700 Subject: surfaceflinger: make mPrimaryDisplayOrientation static The convention is to store configstore values in static variables. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: I085178803bc897e3e9201fd10bd8731cc5b617c1 Merged-In: I085178803bc897e3e9201fd10bd8731cc5b617c1 --- services/surfaceflinger/DisplayDevice.cpp | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 26 +++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 5 ++--- .../tests/unittests/TestableSurfaceFlinger.h | 2 +- 4 files changed, 17 insertions(+), 18 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 309fd0a791..30322a8b67 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -610,7 +610,7 @@ void DisplayDevice::setProjection(int orientation, // need to take care of primary display rotation for mGlobalTransform // for case if the panel is not installed aligned with device orientation if (mType == DisplayType::DISPLAY_PRIMARY) { - int primaryDisplayOrientation = mFlinger->getPrimaryDisplayOrientation(); + int primaryDisplayOrientation = SurfaceFlinger::primaryDisplayOrientation; DisplayDevice::orientationToTransfrom( (orientation + primaryDisplayOrientation) % (DisplayState::eOrientation270 + 1), w, h, &R); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 0d1cf1c86d..f51e71382c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -166,7 +166,7 @@ bool SurfaceFlinger::useVrFlinger; int64_t SurfaceFlinger::maxFrameBufferAcquiredBuffers; // TODO(courtneygo): Rename hasWideColorDisplay to clarify its actual meaning. bool SurfaceFlinger::hasWideColorDisplay; - +int SurfaceFlinger::primaryDisplayOrientation = DisplayState::eOrientationDefault; std::string getHwcServiceName() { char value[PROPERTY_VALUE_MAX] = {}; @@ -300,19 +300,19 @@ SurfaceFlinger::SurfaceFlinger() : SurfaceFlinger(SkipInitialization) { switch (primaryDisplayOrientation) { case V1_1::DisplayOrientation::ORIENTATION_90: - mPrimaryDisplayOrientation = DisplayState::eOrientation90; + SurfaceFlinger::primaryDisplayOrientation = DisplayState::eOrientation90; break; case V1_1::DisplayOrientation::ORIENTATION_180: - mPrimaryDisplayOrientation = DisplayState::eOrientation180; + SurfaceFlinger::primaryDisplayOrientation = DisplayState::eOrientation180; break; case V1_1::DisplayOrientation::ORIENTATION_270: - mPrimaryDisplayOrientation = DisplayState::eOrientation270; + SurfaceFlinger::primaryDisplayOrientation = DisplayState::eOrientation270; break; default: - mPrimaryDisplayOrientation = DisplayState::eOrientationDefault; + SurfaceFlinger::primaryDisplayOrientation = DisplayState::eOrientationDefault; break; } - ALOGV("Primary Display Orientation is set to %2d.", mPrimaryDisplayOrientation); + ALOGV("Primary Display Orientation is set to %2d.", SurfaceFlinger::primaryDisplayOrientation); mPrimaryDispSync.init(SurfaceFlinger::hasSyncFramework, SurfaceFlinger::dispSyncPresentTimeOffset); @@ -974,7 +974,7 @@ status_t SurfaceFlinger::getDisplayConfigs(const sp& display, info.secure = true; if (type == DisplayDevice::DISPLAY_PRIMARY && - mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) { + primaryDisplayOrientation & DisplayState::eOrientationSwapMask) { std::swap(info.w, info.h); } @@ -4874,7 +4874,7 @@ status_t SurfaceFlinger::captureScreen(const sp& display, spgetViewport().height()); } - // XXX mPrimaryDisplayOrientation is ignored + // XXX primaryDisplayOrientation is ignored } DisplayRenderArea renderArea(device, sourceCrop, reqWidth, reqHeight, renderAreaRotation); @@ -5095,7 +5095,7 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, Rect sourceCrop = renderArea.getSourceCrop(); bool filtering = false; - if (mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) { + if (primaryDisplayOrientation & DisplayState::eOrientationSwapMask) { filtering = static_cast(reqWidth) != raHeight || static_cast(reqHeight) != raWidth; } else { @@ -5107,10 +5107,10 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, if (sourceCrop.width() == 0 || sourceCrop.height() == 0 || !sourceCrop.isValid()) { sourceCrop.setLeftTop(Point(0, 0)); sourceCrop.setRightBottom(Point(raWidth, raHeight)); - } else if (mPrimaryDisplayOrientation != DisplayState::eOrientationDefault) { + } else if (primaryDisplayOrientation != DisplayState::eOrientationDefault) { Transform tr; uint32_t flags = 0x00; - switch (mPrimaryDisplayOrientation) { + switch (primaryDisplayOrientation) { case DisplayState::eOrientation90: flags = Transform::ROT_90; break; @@ -5133,12 +5133,12 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, engine.checkErrors(); Transform::orientation_flags rotation = renderArea.getRotationFlags(); - if (mPrimaryDisplayOrientation != DisplayState::eOrientationDefault) { + if (primaryDisplayOrientation != DisplayState::eOrientationDefault) { // convert hw orientation into flag presentation // here inverse transform needed uint8_t hw_rot_90 = 0x00; uint8_t hw_flip_hv = 0x00; - switch (mPrimaryDisplayOrientation) { + switch (primaryDisplayOrientation) { case DisplayState::eOrientation90: hw_rot_90 = Transform::ROT_90; hw_flip_hv = Transform::ROT_180; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index d2b1233a5e..0914a097e4 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -286,6 +286,8 @@ public: // want to support color management to disable color management. static bool hasWideColorDisplay; + static int primaryDisplayOrientation; + static char const* getServiceName() ANDROID_API { return "SurfaceFlinger"; } @@ -345,8 +347,6 @@ public: bool authenticateSurfaceTextureLocked( const sp& bufferProducer) const; - int getPrimaryDisplayOrientation() const { return mPrimaryDisplayOrientation; } - private: friend class Client; friend class DisplayEventConnection; @@ -858,7 +858,6 @@ private: mutable std::unique_ptr mEventQueue{std::make_unique()}; FrameTracker mAnimFrameTracker; DispSync mPrimaryDispSync; - int mPrimaryDisplayOrientation = DisplayState::eOrientationDefault; // protected by mDestroyedLayerLock; mutable Mutex mDestroyedLayerLock; diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 558845f09b..61b0e42310 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -110,7 +110,7 @@ public: */ auto& mutableHasWideColorDisplay() { return SurfaceFlinger::hasWideColorDisplay; } - + auto& mutablePrimaryDisplayOrientation() { return SurfaceFlinger::primaryDisplayOrientation; } auto& mutableBuiltinDisplays() { return mFlinger->mBuiltinDisplays; } auto& mutableCurrentState() { return mFlinger->mCurrentState; } auto& mutableDisplays() { return mFlinger->mDisplays; } -- cgit v1.2.3-59-g8ed1b From 690a76f15c4f267076ba8a70aff0c120e996a115 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 27 Aug 2018 14:38:23 -0700 Subject: surfaceflinger: add install orientation to DisplayDevice Rather than querying it from SurfaceFlinger, initialize DisplayDevice with the install orientation. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: Ibffe47033276e938388af775749c56f170fe8c77 Merged-In: Ibffe47033276e938388af775749c56f170fe8c77 --- services/surfaceflinger/DisplayDevice.cpp | 5 +++-- services/surfaceflinger/DisplayDevice.h | 3 +++ services/surfaceflinger/SurfaceFlinger.cpp | 7 +++++-- services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 9 +++++---- 4 files changed, 16 insertions(+), 8 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 30322a8b67..65c3839141 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -224,6 +224,7 @@ DisplayDevice::DisplayDevice( std::unique_ptr renderSurface, int displayWidth, int displayHeight, + int displayInstallOrientation, bool hasWideColorGamut, const HdrCapabilities& hdrCapabilities, const int32_t supportedPerFrameMetadata, @@ -239,6 +240,7 @@ DisplayDevice::DisplayDevice( mSurface{std::move(renderSurface)}, mDisplayWidth(displayWidth), mDisplayHeight(displayHeight), + mDisplayInstallOrientation(displayInstallOrientation), mPageFlipCount(0), mIsSecure(isSecure), mLayerStack(NO_LAYER_STACK), @@ -610,9 +612,8 @@ void DisplayDevice::setProjection(int orientation, // need to take care of primary display rotation for mGlobalTransform // for case if the panel is not installed aligned with device orientation if (mType == DisplayType::DISPLAY_PRIMARY) { - int primaryDisplayOrientation = SurfaceFlinger::primaryDisplayOrientation; DisplayDevice::orientationToTransfrom( - (orientation + primaryDisplayOrientation) % (DisplayState::eOrientation270 + 1), + (orientation + mDisplayInstallOrientation) % (DisplayState::eOrientation270 + 1), w, h, &R); } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 6e0ab578cf..21af37f043 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -87,6 +87,7 @@ public: std::unique_ptr renderSurface, int displayWidth, int displayHeight, + int displayInstallOrientation, bool hasWideColorGamut, const HdrCapabilities& hdrCapabilities, const int32_t supportedPerFrameMetadata, @@ -110,6 +111,7 @@ public: int getWidth() const; int getHeight() const; + int getInstallOrientation() const { return mDisplayInstallOrientation; } void setVisibleLayersSortedByZ(const Vector< sp >& layers); const Vector< sp >& getVisibleLayersSortedByZ() const; @@ -233,6 +235,7 @@ private: std::unique_ptr mSurface; int mDisplayWidth; int mDisplayHeight; + const int mDisplayInstallOrientation; mutable uint32_t mPageFlipCount; String8 mDisplayName; bool mIsSecure; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f51e71382c..a503bba23b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2417,6 +2417,9 @@ sp SurfaceFlinger::setupNewDisplayDeviceInternal( nativeWindow->setSwapInterval(nativeWindow.get(), 0); } + const int displayInstallOrientation = state.type == DisplayDevice::DISPLAY_PRIMARY ? + primaryDisplayOrientation : DisplayState::eOrientationDefault; + // virtual displays are always considered enabled auto initialPowerMode = (state.type >= DisplayDevice::DISPLAY_VIRTUAL) ? HWC_POWER_MODE_NORMAL : HWC_POWER_MODE_OFF; @@ -2424,7 +2427,7 @@ sp SurfaceFlinger::setupNewDisplayDeviceInternal( sp hw = new DisplayDevice(this, state.type, hwcId, state.isSecure, display, nativeWindow, dispSurface, std::move(renderSurface), displayWidth, displayHeight, - hasWideColorGamut, hdrCapabilities, + displayInstallOrientation, hasWideColorGamut, hdrCapabilities, supportedPerFrameMetadata, hwcColorModes, initialPowerMode); if (maxFrameBufferAcquiredBuffers >= 3) { @@ -4874,7 +4877,7 @@ status_t SurfaceFlinger::captureScreen(const sp& display, spgetViewport().height()); } - // XXX primaryDisplayOrientation is ignored + // XXX display->getInstallOrientation() is ignored } DisplayRenderArea renderArea(device, sourceCrop, reqWidth, reqHeight, renderAreaRotation); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 61b0e42310..4c5fa9953e 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -320,10 +320,11 @@ public: sp inject() { std::unordered_map> hdrAndRenderIntents; sp device = - new DisplayDevice(mFlinger.mFlinger.get(), mType, mHwcId, mSecure, mDisplayToken, - mNativeWindow, mDisplaySurface, std::move(mRenderSurface), 0, - 0, false, HdrCapabilities(), 0, hdrAndRenderIntents, - HWC_POWER_MODE_NORMAL); + new DisplayDevice(mFlinger.mFlinger.get(), mType, mHwcId, mSecure, + mDisplayToken, mNativeWindow, mDisplaySurface, + std::move(mRenderSurface), 0, 0, + DisplayState::eOrientationDefault, false, HdrCapabilities(), + 0, hdrAndRenderIntents, HWC_POWER_MODE_NORMAL); mFlinger.mutableDisplays().add(mDisplayToken, device); DisplayDeviceState state(mType, mSecure); -- cgit v1.2.3-59-g8ed1b From becab0f31aa76f57fdbd87913f027daa07054cf9 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Mon, 27 Aug 2018 14:54:37 -0700 Subject: surfaceflinger: respect install orientation in DisplayRenderArea DisplayRenderArea getTransform takes install orientation into consideration. Fix getSourceCrop and getRotationFlags to respect install orientation as well. This results in some changes - source crop is no longer rotated by install orientation for LayerRenderArea - rotation flags is no longer rotated by install orientation for LayerRenderArea - source crop is no longer set to getBounds by default for LayerRenderArea (it is already done in captureLayers) - source crop is always rotated by install orientation for DisplayRenderArea AFAICT, all the changes are actually fixes. Quickly verified by forcing primaryDisplayOrientation to DisplayState::eOrientation90. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: If19df222ae52f6b276f9b0572e7b9bec872e3ae4 Merged-In: If19df222ae52f6b276f9b0572e7b9bec872e3ae4 --- services/surfaceflinger/DisplayDevice.h | 67 ++++++++++++++++++++++++++++-- services/surfaceflinger/SurfaceFlinger.cpp | 56 +------------------------ 2 files changed, 66 insertions(+), 57 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 21af37f043..0e8867f609 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -344,8 +345,10 @@ public: rotation) {} DisplayRenderArea(const sp device, Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight, Transform::orientation_flags rotation) - : RenderArea(reqWidth, reqHeight, CaptureFill::OPAQUE, rotation), mDevice(device), - mSourceCrop(sourceCrop) {} + : RenderArea(reqWidth, reqHeight, CaptureFill::OPAQUE, + getDisplayRotation(rotation, device->getInstallOrientation())), + mDevice(device), + mSourceCrop(sourceCrop) {} const Transform& getTransform() const override { return mDevice->getTransform(); } Rect getBounds() const override { return mDevice->getBounds(); } @@ -353,9 +356,67 @@ public: int getWidth() const override { return mDevice->getWidth(); } bool isSecure() const override { return mDevice->isSecure(); } bool needsFiltering() const override { return mDevice->needsFiltering(); } - Rect getSourceCrop() const override { return mSourceCrop; } + + Rect getSourceCrop() const override { + const int orientation = mDevice->getInstallOrientation(); + if (orientation == DisplayState::eOrientationDefault) { + return mSourceCrop; + } + + uint32_t flags = 0x00; + switch (orientation) { + case DisplayState::eOrientation90: + flags = Transform::ROT_90; + break; + case DisplayState::eOrientation180: + flags = Transform::ROT_180; + break; + case DisplayState::eOrientation270: + flags = Transform::ROT_270; + break; + } + Transform tr; + tr.set(flags, getWidth(), getHeight()); + return tr.transform(mSourceCrop); + } private: + static Transform::orientation_flags getDisplayRotation( + Transform::orientation_flags rotation, int orientation) { + if (orientation == DisplayState::eOrientationDefault) { + return rotation; + } + + // convert hw orientation into flag presentation + // here inverse transform needed + uint8_t hw_rot_90 = 0x00; + uint8_t hw_flip_hv = 0x00; + switch (orientation) { + case DisplayState::eOrientation90: + hw_rot_90 = Transform::ROT_90; + hw_flip_hv = Transform::ROT_180; + break; + case DisplayState::eOrientation180: + hw_flip_hv = Transform::ROT_180; + break; + case DisplayState::eOrientation270: + hw_rot_90 = Transform::ROT_90; + break; + } + + // transform flags operation + // 1) flip H V if both have ROT_90 flag + // 2) XOR these flags + uint8_t rotation_rot_90 = rotation & Transform::ROT_90; + uint8_t rotation_flip_hv = rotation & Transform::ROT_180; + if (rotation_rot_90 & hw_rot_90) { + rotation_flip_hv = (~rotation_flip_hv) & Transform::ROT_180; + } + + return static_cast( + (rotation_rot_90 ^ hw_rot_90) | (rotation_flip_hv ^ hw_flip_hv)); + } + const sp mDevice; const Rect mSourceCrop; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a503bba23b..cadbf8afbd 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5095,7 +5095,8 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, const auto reqWidth = renderArea.getReqWidth(); const auto reqHeight = renderArea.getReqHeight(); - Rect sourceCrop = renderArea.getSourceCrop(); + const auto sourceCrop = renderArea.getSourceCrop(); + const auto rotation = renderArea.getRotationFlags(); bool filtering = false; if (primaryDisplayOrientation & DisplayState::eOrientationSwapMask) { @@ -5106,28 +5107,6 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, static_cast(reqHeight) != raHeight; } - // if a default or invalid sourceCrop is passed in, set reasonable values - if (sourceCrop.width() == 0 || sourceCrop.height() == 0 || !sourceCrop.isValid()) { - sourceCrop.setLeftTop(Point(0, 0)); - sourceCrop.setRightBottom(Point(raWidth, raHeight)); - } else if (primaryDisplayOrientation != DisplayState::eOrientationDefault) { - Transform tr; - uint32_t flags = 0x00; - switch (primaryDisplayOrientation) { - case DisplayState::eOrientation90: - flags = Transform::ROT_90; - break; - case DisplayState::eOrientation180: - flags = Transform::ROT_180; - break; - case DisplayState::eOrientation270: - flags = Transform::ROT_270; - break; - } - tr.set(flags, raWidth, raHeight); - sourceCrop = tr.transform(sourceCrop); - } - // assume ColorMode::SRGB / RenderIntent::COLORIMETRIC engine.setOutputDataSpace(Dataspace::SRGB); engine.setDisplayMaxLuminance(DisplayDevice::sDefaultMaxLumiance); @@ -5135,37 +5114,6 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, // make sure to clear all GL error flags engine.checkErrors(); - Transform::orientation_flags rotation = renderArea.getRotationFlags(); - if (primaryDisplayOrientation != DisplayState::eOrientationDefault) { - // convert hw orientation into flag presentation - // here inverse transform needed - uint8_t hw_rot_90 = 0x00; - uint8_t hw_flip_hv = 0x00; - switch (primaryDisplayOrientation) { - case DisplayState::eOrientation90: - hw_rot_90 = Transform::ROT_90; - hw_flip_hv = Transform::ROT_180; - break; - case DisplayState::eOrientation180: - hw_flip_hv = Transform::ROT_180; - break; - case DisplayState::eOrientation270: - hw_rot_90 = Transform::ROT_90; - break; - } - - // transform flags operation - // 1) flip H V if both have ROT_90 flag - // 2) XOR these flags - uint8_t rotation_rot_90 = rotation & Transform::ROT_90; - uint8_t rotation_flip_hv = rotation & Transform::ROT_180; - if (rotation_rot_90 & hw_rot_90) { - rotation_flip_hv = (~rotation_flip_hv) & Transform::ROT_180; - } - rotation = static_cast - ((rotation_rot_90 ^ hw_rot_90) | (rotation_flip_hv ^ hw_flip_hv)); - } - // set-up our viewport engine.setViewportAndProjection(reqWidth, reqHeight, sourceCrop, raHeight, yswap, rotation); -- cgit v1.2.3-59-g8ed1b From 75dcd905652b490a31537f81a1209100df050d42 Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Tue, 28 Aug 2018 11:01:44 -0700 Subject: surfaceflinger: improve RenderArea needsFiltering Compare source crop (instead of the logical render area) against physical render area to decide whether filtering is required. This allows us to get rid of Layer::setFiltering. As a result, captureLayers for Recents no longer enables filtering. Screenshots under landscape mode no longer enables filtering. Bug: 113041375 Test: take screenshot, rotate screen, screencap Change-Id: Ida95fdfec3a0dde7a19adf35c91bf3d570bab6bb Merged-In: Ida95fdfec3a0dde7a19adf35c91bf3d570bab6bb --- services/surfaceflinger/BufferLayer.cpp | 2 +- services/surfaceflinger/DisplayDevice.h | 15 ++++++++++++++- services/surfaceflinger/Layer.cpp | 9 --------- services/surfaceflinger/Layer.h | 5 ----- services/surfaceflinger/SurfaceFlinger.cpp | 21 ++++++++------------- 5 files changed, 23 insertions(+), 29 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index f5b5eda9ec..707cb42336 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -204,7 +204,7 @@ void BufferLayer::onDraw(const RenderArea& renderArea, const Region& clip, if (!blackOutLayer) { // TODO: we could be more subtle with isFixedSize() - const bool useFiltering = getFiltering() || needsFiltering(renderArea) || isFixedSize(); + const bool useFiltering = needsFiltering(renderArea) || isFixedSize(); // Query the texture matrix given our current filtering mode. float textureMatrix[16]; diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 0e8867f609..548cabc7e9 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -355,7 +355,20 @@ public: int getHeight() const override { return mDevice->getHeight(); } int getWidth() const override { return mDevice->getWidth(); } bool isSecure() const override { return mDevice->isSecure(); } - bool needsFiltering() const override { return mDevice->needsFiltering(); } + + bool needsFiltering() const override { + if (mDevice->needsFiltering()) { + return true; + } + + const Rect sourceCrop = getSourceCrop(); + int width = sourceCrop.width(); + int height = sourceCrop.height(); + if (getRotationFlags() & Transform::ROT_90) { + std::swap(width, height); + } + return width != getReqWidth() || height != getReqHeight(); + } Rect getSourceCrop() const override { const int orientation = mDevice->getInstallOrientation(); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0caac9b16d..72f1fc4363 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -88,7 +88,6 @@ Layer::Layer(SurfaceFlinger* flinger, const sp& client, const String8& n mCurrentOpacity(true), mCurrentFrameNumber(0), mFrameLatencyNeeded(false), - mFiltering(false), mNeedsFiltering(false), mProtectedByApp(false), mClientRef(client), @@ -793,14 +792,6 @@ bool Layer::addSyncPoint(const std::shared_ptr& point) { return true; } -void Layer::setFiltering(bool filtering) { - mFiltering = filtering; -} - -bool Layer::getFiltering() const { - return mFiltering; -} - // ---------------------------------------------------------------------------- // local state // ---------------------------------------------------------------------------- diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 301f190e16..239f3979e8 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -530,9 +530,6 @@ public: // ----------------------------------------------------------------------- void clearWithOpenGL(const RenderArea& renderArea) const; - void setFiltering(bool filtering); - bool getFiltering() const; - inline const State& getDrawingState() const { return mDrawingState; } inline const State& getCurrentState() const { return mCurrentState; } @@ -755,8 +752,6 @@ protected: bool mCurrentOpacity; std::atomic mCurrentFrameNumber; bool mFrameLatencyNeeded; - // Whether filtering is forced on or not - bool mFiltering; // Whether filtering is needed b/c of the drawingstate bool mNeedsFiltering; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index cadbf8afbd..ab2cbe2468 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4899,6 +4899,7 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, : RenderArea(reqWidth, reqHeight, CaptureFill::CLEAR), mLayer(layer), mCrop(crop), + mNeedsFiltering(false), mFlinger(flinger), mChildrenOnly(childrenOnly) {} const Transform& getTransform() const override { return mTransform; } @@ -4909,7 +4910,7 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, int getHeight() const override { return mLayer->getDrawingState().active.h; } int getWidth() const override { return mLayer->getDrawingState().active.w; } bool isSecure() const override { return false; } - bool needsFiltering() const override { return false; } + bool needsFiltering() const override { return mNeedsFiltering; } Rect getSourceCrop() const override { if (mCrop.isEmpty()) { return getBounds(); @@ -4930,6 +4931,11 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, }; void render(std::function drawLayers) override { + const Rect sourceCrop = getSourceCrop(); + // no need to check rotation because there is none + mNeedsFiltering = sourceCrop.width() != getReqWidth() || + sourceCrop.height() != getReqHeight(); + if (!mChildrenOnly) { mTransform = mLayer->getTransform().inverse(); drawLayers(); @@ -4952,6 +4958,7 @@ status_t SurfaceFlinger::captureLayers(const sp& layerHandleBinder, // layer which has no properties set and which does not draw. sp screenshotParentLayer; Transform mTransform; + bool mNeedsFiltering; SurfaceFlinger* mFlinger; const bool mChildrenOnly; @@ -5090,7 +5097,6 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, auto& engine(getRenderEngine()); // get screen geometry - const auto raWidth = renderArea.getWidth(); const auto raHeight = renderArea.getHeight(); const auto reqWidth = renderArea.getReqWidth(); @@ -5098,15 +5104,6 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, const auto sourceCrop = renderArea.getSourceCrop(); const auto rotation = renderArea.getRotationFlags(); - bool filtering = false; - if (primaryDisplayOrientation & DisplayState::eOrientationSwapMask) { - filtering = static_cast(reqWidth) != raHeight || - static_cast(reqHeight) != raWidth; - } else { - filtering = static_cast(reqWidth) != raWidth || - static_cast(reqHeight) != raHeight; - } - // assume ColorMode::SRGB / RenderIntent::COLORIMETRIC engine.setOutputDataSpace(Dataspace::SRGB); engine.setDisplayMaxLuminance(DisplayDevice::sDefaultMaxLumiance); @@ -5124,9 +5121,7 @@ void SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, engine.clearWithColor(0, 0, 0, alpha); traverseLayers([&](Layer* layer) { - if (filtering) layer->setFiltering(true); layer->draw(renderArea, useIdentityTransform); - if (filtering) layer->setFiltering(false); }); } -- cgit v1.2.3-59-g8ed1b From c356f6659cf632a2ab8ab2e506e091b485d50a6f Mon Sep 17 00:00:00 2001 From: Chia-I Wu Date: Wed, 29 Aug 2018 09:25:59 -0700 Subject: surfaceflinger: fix captureScreen for landscape LCM Make DisplayRenderArea::getSourceCrop return the display scissor when the source crop is empty. Force the source crop to be empty in captureScreen. This makes sure the install orientation is applied on the source crop once, not twice. Bug: 113041375 Test: force primaryDisplayOrientation, rotate screen, and switch apps Change-Id: I15006f867ff2d4a92ebccb1334ce59ab32abe69a Merged-In: I15006f867ff2d4a92ebccb1334ce59ab32abe69a --- services/surfaceflinger/DisplayDevice.h | 12 ++++++++++++ services/surfaceflinger/SurfaceFlinger.cpp | 8 +++----- 2 files changed, 15 insertions(+), 5 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 548cabc7e9..3cf06bceaf 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -357,10 +357,14 @@ public: bool isSecure() const override { return mDevice->isSecure(); } bool needsFiltering() const override { + // check if the projection from the logical display to the physical + // display needs filtering if (mDevice->needsFiltering()) { return true; } + // check if the projection from the logical render area (i.e., the + // physical display) to the physical render area requires filtering const Rect sourceCrop = getSourceCrop(); int width = sourceCrop.width(); int height = sourceCrop.height(); @@ -371,11 +375,17 @@ public: } Rect getSourceCrop() const override { + // use the (projected) logical display viewport by default + if (mSourceCrop.isEmpty()) { + return mDevice->getScissor(); + } + const int orientation = mDevice->getInstallOrientation(); if (orientation == DisplayState::eOrientationDefault) { return mSourceCrop; } + // Install orientation is transparent to the callers. Apply it now. uint32_t flags = 0x00; switch (orientation) { case DisplayState::eOrientation90: @@ -394,6 +404,8 @@ public: } private: + // Install orientation is transparent to the callers. We need to cancel + // it out by modifying rotation flags. static Transform::orientation_flags getDisplayRotation( Transform::orientation_flags rotation, int orientation) { if (orientation == DisplayState::eOrientationDefault) { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ab2cbe2468..58d77653ac 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4866,9 +4866,9 @@ status_t SurfaceFlinger::captureScreen(const sp& display, spgetScissor()); + // ignore sourceCrop (i.e., use the projected logical display + // viewport) until the framework is fixed + sourceCrop.clear(); // set the requested width/height to the logical display viewport size // by default @@ -4876,8 +4876,6 @@ status_t SurfaceFlinger::captureScreen(const sp& display, spgetViewport().width()); reqHeight = uint32_t(device->getViewport().height()); } - - // XXX display->getInstallOrientation() is ignored } DisplayRenderArea renderArea(device, sourceCrop, reqWidth, reqHeight, renderAreaRotation); -- cgit v1.2.3-59-g8ed1b From 76607ada1efdff72d24554fab45d8133414f7603 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 26 Oct 2018 15:49:45 -0700 Subject: Respect source crop when capturing layers. Source crop we receive from window manager appears to be correct, no need for this workaround. Bug: 117892959 Test: adb shell screencap, rotation animation, power cycle display Change-Id: I54ea3fb561ce4f2ea6f48079ad51efdd9984d186 Merged-In: I54ea3fb561ce4f2ea6f48079ad51efdd9984d186 --- services/surfaceflinger/SurfaceFlinger.cpp | 4 ---- 1 file changed, 4 deletions(-) (limited to 'services/surfaceflinger/SurfaceFlinger.cpp') diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 58d77653ac..78d751a212 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4866,10 +4866,6 @@ status_t SurfaceFlinger::captureScreen(const sp& display, sp