From 7584c6a6ca02a5780ac68c8fcfa1d3408671e04d Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Sat, 29 Oct 2022 02:10:58 +0000 Subject: SF: refactor renderScreenImpl to use CompositionEngine Bug: 238643986 Test: presubmits Test: go/wm-smoke Change-Id: I6dbbcd5ce5070ec1d801dca55b5bb89fafe839cb --- services/surfaceflinger/ScreenCaptureOutput.cpp | 107 ++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 services/surfaceflinger/ScreenCaptureOutput.cpp (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp new file mode 100644 index 0000000000..8f93ba40ee --- /dev/null +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -0,0 +1,107 @@ +/* + * Copyright 2022 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. + */ + +#include "ScreenCaptureOutput.h" +#include "ScreenCaptureRenderSurface.h" + +#include +#include +#include +#include + +namespace android { + +std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs&& args) { + std::shared_ptr output = compositionengine::impl::createOutputTemplated< + ScreenCaptureOutput, compositionengine::CompositionEngine, + ScreenCaptureOutputArgs&&>(args.compositionEngine, std::move(args)); + output->editState().isSecure = args.renderArea.isSecure(); + output->setCompositionEnabled(true); + output->setLayerFilter({args.layerStack}); + output->setRenderSurface(std::make_unique(std::move(args.buffer))); + output->setDisplayBrightness(args.sdrWhitePointNits, args.displayBrightnessNits); + + output->setDisplayColorProfile(std::make_unique( + compositionengine::DisplayColorProfileCreationArgsBuilder() + .setHasWideColorGamut(true) + .Build())); + + ui::Rotation orientation = ui::Transform::toRotation(args.renderArea.getRotationFlags()); + Rect orientedDisplaySpaceRect{args.renderArea.getReqWidth(), args.renderArea.getReqHeight()}; + output->setProjection(orientation, args.renderArea.getLayerStackSpaceRect(), + orientedDisplaySpaceRect); + + Rect sourceCrop = args.renderArea.getSourceCrop(); + output->setDisplaySize({sourceCrop.getWidth(), sourceCrop.getHeight()}); + + return output; +} + +ScreenCaptureOutput::ScreenCaptureOutput(ScreenCaptureOutputArgs&& args) + : mRenderArea(args.renderArea), + mFilterForScreenshot(std::move(args.filterForScreenshot)), + mColorProfile(args.colorProfile), + mRegionSampling(args.regionSampling) {} + +void ScreenCaptureOutput::updateColorProfile(const compositionengine::CompositionRefreshArgs&) { + auto& outputState = editState(); + outputState.dataspace = mColorProfile.dataspace; + outputState.renderIntent = mColorProfile.renderIntent; +} + +renderengine::DisplaySettings ScreenCaptureOutput::generateClientCompositionDisplaySettings() + const { + auto clientCompositionDisplay = + compositionengine::impl::Output::generateClientCompositionDisplaySettings(); + clientCompositionDisplay.clip = mRenderArea.getSourceCrop(); + clientCompositionDisplay.targetLuminanceNits = -1; + return clientCompositionDisplay; +} + +std::vector +ScreenCaptureOutput::generateClientCompositionRequests( + bool supportsProtectedContent, ui::Dataspace outputDataspace, + std::vector& outLayerFEs) { + auto clientCompositionLayers = compositionengine::impl::Output:: + generateClientCompositionRequests(supportsProtectedContent, outputDataspace, + outLayerFEs); + + if (mRegionSampling) { + for (auto& layer : clientCompositionLayers) { + layer.backgroundBlurRadius = 0; + layer.blurRegions.clear(); + } + } + + Rect sourceCrop = mRenderArea.getSourceCrop(); + compositionengine::LayerFE::LayerSettings fillLayer; + fillLayer.source.buffer.buffer = nullptr; + fillLayer.source.solidColor = half3(0.0f, 0.0f, 0.0f); + fillLayer.geometry.boundaries = + FloatRect(static_cast(sourceCrop.left), static_cast(sourceCrop.top), + static_cast(sourceCrop.right), static_cast(sourceCrop.bottom)); + fillLayer.alpha = half(RenderArea::getCaptureFillValue(mRenderArea.getCaptureFill())); + clientCompositionLayers.insert(clientCompositionLayers.begin(), fillLayer); + + return clientCompositionLayers; +} + +bool ScreenCaptureOutput::layerNeedsFiltering(const compositionengine::OutputLayer* layer) const { + return mRenderArea.needsFiltering() || + mFilterForScreenshot.find(&layer->getLayerFE()) != mFilterForScreenshot.end(); +} + +} // namespace android -- cgit v1.2.3-59-g8ed1b From 01c31167ce5880ec5f6f97ac82c81c85c9807f19 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Mon, 14 Nov 2022 17:45:19 +0000 Subject: SF: Fix use after move bug in screen capture refactor Bug: 238643986 Test: presubmits Change-Id: I1ff3d214eafcf02a1a50e2dc80a0ff4c01807aaf --- services/surfaceflinger/ScreenCaptureOutput.cpp | 25 +++++++++++++++++-------- services/surfaceflinger/ScreenCaptureOutput.h | 7 +++++-- 2 files changed, 22 insertions(+), 10 deletions(-) (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index 8f93ba40ee..37b3218138 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -24,10 +24,16 @@ namespace android { -std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs&& args) { +std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs args) { std::shared_ptr output = compositionengine::impl::createOutputTemplated< - ScreenCaptureOutput, compositionengine::CompositionEngine, - ScreenCaptureOutputArgs&&>(args.compositionEngine, std::move(args)); + ScreenCaptureOutput, compositionengine::CompositionEngine, const RenderArea&, + std::unordered_set, + const compositionengine::Output::ColorProfile&, bool>(args.compositionEngine, + args.renderArea, + std::move( + args.filterForScreenshot), + args.colorProfile, + args.regionSampling); output->editState().isSecure = args.renderArea.isSecure(); output->setCompositionEnabled(true); output->setLayerFilter({args.layerStack}); @@ -50,11 +56,14 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp return output; } -ScreenCaptureOutput::ScreenCaptureOutput(ScreenCaptureOutputArgs&& args) - : mRenderArea(args.renderArea), - mFilterForScreenshot(std::move(args.filterForScreenshot)), - mColorProfile(args.colorProfile), - mRegionSampling(args.regionSampling) {} +ScreenCaptureOutput::ScreenCaptureOutput( + const RenderArea& renderArea, + std::unordered_set filterForScreenshot, + const compositionengine::Output::ColorProfile& colorProfile, bool regionSampling) + : mRenderArea(renderArea), + mFilterForScreenshot(std::move(filterForScreenshot)), + mColorProfile(colorProfile), + mRegionSampling(regionSampling) {} void ScreenCaptureOutput::updateColorProfile(const compositionengine::CompositionRefreshArgs&) { auto& outputState = editState(); diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h index 61b5ddb1bb..5dffc1d530 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.h +++ b/services/surfaceflinger/ScreenCaptureOutput.h @@ -43,7 +43,10 @@ struct ScreenCaptureOutputArgs { // SurfaceFlinger::captureLayers and SurfaceFlinger::captureDisplay. class ScreenCaptureOutput : public compositionengine::impl::Output { public: - ScreenCaptureOutput(ScreenCaptureOutputArgs&&); + ScreenCaptureOutput(const RenderArea& renderArea, + std::unordered_set filterForScreenshot, + const compositionengine::Output::ColorProfile& colorProfile, + bool regionSampling); void updateColorProfile(const compositionengine::CompositionRefreshArgs&) override; @@ -64,6 +67,6 @@ private: const bool mRegionSampling; }; -std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs&&); +std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs); } // namespace android -- cgit v1.2.3-59-g8ed1b From c03d4659733145dd256e3f4a69f20c0156d21969 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Thu, 5 Jan 2023 13:03:53 -0500 Subject: Name the ScreenCaptureOutput Set a name for ScreenCaptureOutput based on whether it's used for region sampling or not and include the display's id if present. In Output's mNamePlusId, drop " (NA)" for those without a display id. This would be extraneous on the ScreenCaptureOutput, which will mark the display for which it's being used. Bug: 262706813 Test: look at traces Change-Id: If447795f3580601434cbcb6493f5fb7c3fa734c9 --- services/surfaceflinger/CompositionEngine/src/Output.cpp | 5 +++-- services/surfaceflinger/ScreenCaptureOutput.cpp | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index d513731fa3..d27add994d 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -118,8 +118,9 @@ const std::string& Output::getName() const { void Output::setName(const std::string& name) { mName = name; auto displayIdOpt = getDisplayId(); - mNamePlusId = base::StringPrintf("%s (%s)", mName.c_str(), - displayIdOpt ? to_string(*displayIdOpt).c_str() : "NA"); + mNamePlusId = displayIdOpt ? base::StringPrintf("%s (%s)", mName.c_str(), + to_string(*displayIdOpt).c_str()) + : mName; } void Output::setCompositionEnabled(bool enabled) { diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index 37b3218138..6d195b9f7b 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -53,6 +53,13 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp Rect sourceCrop = args.renderArea.getSourceCrop(); output->setDisplaySize({sourceCrop.getWidth(), sourceCrop.getHeight()}); + { + std::string name = args.regionSampling ? "RegionSampling" : "ScreenCaptureOutput"; + if (auto displayDevice = args.renderArea.getDisplayDevice()) { + base::StringAppendF(&name, " for %" PRIu64, displayDevice->getId().value); + } + output->setName(name); + } return output; } -- cgit v1.2.3-59-g8ed1b From 278a88f603fbdbc46275dc67463b796a22e71353 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Fri, 27 Jan 2023 16:52:40 -0600 Subject: SF: clean up texture filtering logic * Removes redundant logic for determining whether or not SurfaceFlinger should request texture filtering. * Fixes an issue where ScreenCaptureOutput was using incorrect values for its layer stack projection space. Bug: 238348307 Test: atest TextureFilteringTest Change-Id: I4ece87321ee73d10b86a2c01d53717d337c11926 --- .../include/compositionengine/impl/Output.h | 5 +- .../CompositionEngine/src/Output.cpp | 6 +- services/surfaceflinger/DisplayDevice.cpp | 4 -- services/surfaceflinger/DisplayDevice.h | 1 - services/surfaceflinger/DisplayRenderArea.cpp | 30 +------- services/surfaceflinger/DisplayRenderArea.h | 6 +- services/surfaceflinger/FrontEnd/LayerSnapshot.h | 1 - .../FrontEnd/LayerSnapshotBuilder.cpp | 11 --- services/surfaceflinger/Layer.cpp | 82 ---------------------- services/surfaceflinger/Layer.h | 4 -- services/surfaceflinger/LayerFE.cpp | 9 +-- services/surfaceflinger/LayerRenderArea.cpp | 28 ++------ services/surfaceflinger/LayerRenderArea.h | 8 +-- services/surfaceflinger/RenderArea.h | 19 +---- services/surfaceflinger/ScreenCaptureOutput.cpp | 29 +++----- services/surfaceflinger/ScreenCaptureOutput.h | 5 -- services/surfaceflinger/SurfaceFlinger.cpp | 18 ++--- .../fuzzer/surfaceflinger_layer_fuzzer.cpp | 2 +- 18 files changed, 30 insertions(+), 238 deletions(-) (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index 8ec77c075b..229a657236 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -136,9 +136,8 @@ protected: compositionengine::Output::FrameFences presentAndGetFrameFences() override; virtual renderengine::DisplaySettings generateClientCompositionDisplaySettings() const; std::vector generateClientCompositionRequests( - bool supportsProtectedContent, ui::Dataspace outputDataspace, - std::vector &outLayerFEs) override; - virtual bool layerNeedsFiltering(const OutputLayer*) const; + bool supportsProtectedContent, ui::Dataspace outputDataspace, + std::vector& outLayerFEs) override; void appendRegionFlashRequests(const Region&, std::vector&) override; void setExpensiveRenderingExpected(bool enabled) override; void setHintSessionGpuFence(std::unique_ptr&& gpuFence) override; diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 3ec681668b..403385ea8c 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1438,7 +1438,7 @@ std::vector Output::generateClientCompositionRequests( Enabled); compositionengine::LayerFE::ClientCompositionTargetSettings targetSettings{.clip = clip, - .needsFiltering = layerNeedsFiltering(layer) || + .needsFiltering = layer->needsFiltering() || outputState.needsFiltering, .isSecure = outputState.isSecure, .supportsProtectedContent = supportsProtectedContent, @@ -1469,10 +1469,6 @@ std::vector Output::generateClientCompositionRequests( return clientCompositionLayers; } -bool Output::layerNeedsFiltering(const compositionengine::OutputLayer* layer) const { - return layer->needsFiltering(); -} - void Output::appendRegionFlashRequests( const Region& flashRegion, std::vector& clientCompositionLayers) { if (flashRegion.isEmpty()) { diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index c61f7d8e55..d0e0c4f8cf 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -347,10 +347,6 @@ const Region& DisplayDevice::getUndefinedRegion() const { return mCompositionDisplay->getState().undefinedRegion; } -bool DisplayDevice::needsFiltering() const { - return mCompositionDisplay->getState().needsFiltering; -} - ui::LayerStack DisplayDevice::getLayerStack() const { return mCompositionDisplay->getState().layerFilter.layerStack; } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 370bd66b9e..e48e8130e1 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -115,7 +115,6 @@ public: const ui::Transform& getTransform() const; const Rect& getLayerStackSpaceRect() const; const Rect& getOrientedDisplaySpaceRect() const; - bool needsFiltering() const; ui::LayerStack getLayerStack() const; bool receivesInput() const { return mFlags & eReceivesInput; } diff --git a/services/surfaceflinger/DisplayRenderArea.cpp b/services/surfaceflinger/DisplayRenderArea.cpp index 20486e0aa3..8f39e26e0f 100644 --- a/services/surfaceflinger/DisplayRenderArea.cpp +++ b/services/surfaceflinger/DisplayRenderArea.cpp @@ -48,8 +48,8 @@ std::unique_ptr DisplayRenderArea::create(wp di DisplayRenderArea::DisplayRenderArea(sp display, const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool useIdentityTransform, bool allowSecureLayers) - : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, display->getLayerStackSpaceRect(), - allowSecureLayers, applyDeviceOrientation(useIdentityTransform, *display)), + : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, allowSecureLayers, + applyDeviceOrientation(useIdentityTransform, *display)), mDisplay(std::move(display)), mSourceCrop(sourceCrop) {} @@ -57,18 +57,6 @@ const ui::Transform& DisplayRenderArea::getTransform() const { return mTransform; } -Rect DisplayRenderArea::getBounds() const { - return mDisplay->getBounds(); -} - -int DisplayRenderArea::getHeight() const { - return mDisplay->getHeight(); -} - -int DisplayRenderArea::getWidth() const { - return mDisplay->getWidth(); -} - bool DisplayRenderArea::isSecure() const { return mAllowSecureLayers && mDisplay->isSecure(); } @@ -77,18 +65,6 @@ sp DisplayRenderArea::getDisplayDevice() const { return mDisplay; } -bool DisplayRenderArea::needsFiltering() const { - // check if the projection from the logical render area - // to the physical render area requires filtering - const Rect& sourceCrop = getSourceCrop(); - int width = sourceCrop.width(); - int height = sourceCrop.height(); - if (getRotationFlags() & ui::Transform::ROT_90) { - std::swap(width, height); - } - return width != getReqWidth() || height != getReqHeight(); -} - Rect DisplayRenderArea::getSourceCrop() const { // use the projected display viewport by default. if (mSourceCrop.isEmpty()) { @@ -107,4 +83,4 @@ Rect DisplayRenderArea::getSourceCrop() const { return rotation.transform(mSourceCrop); } -} // namespace android \ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/DisplayRenderArea.h b/services/surfaceflinger/DisplayRenderArea.h index 3478fc16c4..ce5410a90d 100644 --- a/services/surfaceflinger/DisplayRenderArea.h +++ b/services/surfaceflinger/DisplayRenderArea.h @@ -33,12 +33,8 @@ public: bool allowSecureLayers = true); const ui::Transform& getTransform() const override; - Rect getBounds() const override; - int getHeight() const override; - int getWidth() const override; bool isSecure() const override; sp getDisplayDevice() const override; - bool needsFiltering() const override; Rect getSourceCrop() const override; private: @@ -50,4 +46,4 @@ private: const ui::Transform mTransform; }; -} // namespace android \ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h index 159410f3ea..d173db3ec5 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshot.h +++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.h @@ -68,7 +68,6 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState { renderengine::ShadowSettings shadowSettings; bool premultipliedAlpha; bool isHdrY410; - bool bufferNeedsFiltering; ui::Transform parentTransform; Rect bufferSize; Rect croppedBufferSize; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index d0ffe613cc..6490476396 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -237,12 +237,6 @@ void handleDropInputMode(LayerSnapshot& snapshot, const LayerSnapshot& parentSna } } -bool getBufferNeedsFiltering(const LayerSnapshot& snapshot, const ui::Size& unrotatedBufferSize) { - const int32_t layerWidth = static_cast(snapshot.geomLayerBounds.getWidth()); - const int32_t layerHeight = static_cast(snapshot.geomLayerBounds.getHeight()); - return layerWidth != unrotatedBufferSize.width || layerHeight != unrotatedBufferSize.height; -} - auto getBlendMode(const LayerSnapshot& snapshot, const RequestedLayerState& requested) { auto blendMode = Hwc2::IComposerClient::BlendMode::NONE; if (snapshot.alpha != 1.0f || !snapshot.isContentOpaque()) { @@ -871,11 +865,6 @@ void LayerSnapshotBuilder::updateLayerBounds(LayerSnapshot& snapshot, if (requested.potentialCursor) { snapshot.cursorFrame = snapshot.geomLayerTransform.transform(bounds); } - - // TODO(b/238781169) use dest vs src - snapshot.bufferNeedsFiltering = snapshot.externalTexture && - getBufferNeedsFiltering(snapshot, - requested.getUnrotatedBufferSize(displayRotationFlags)); } void LayerSnapshotBuilder::updateShadows(LayerSnapshot& snapshot, diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 7a4b3375a1..c58ee2bdb2 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -3319,39 +3319,6 @@ sp Layer::createClone() { return layer; } -bool Layer::bufferNeedsFiltering() const { - const State& s(getDrawingState()); - if (!s.buffer) { - return false; - } - - int32_t bufferWidth = static_cast(s.buffer->getWidth()); - int32_t bufferHeight = static_cast(s.buffer->getHeight()); - - // Undo any transformations on the buffer and return the result. - if (s.bufferTransform & ui::Transform::ROT_90) { - std::swap(bufferWidth, bufferHeight); - } - - if (s.transformToDisplayInverse) { - uint32_t invTransform = DisplayDevice::getPrimaryDisplayRotationFlags(); - if (invTransform & ui::Transform::ROT_90) { - std::swap(bufferWidth, bufferHeight); - } - } - - const Rect layerSize{getBounds()}; - int32_t layerWidth = layerSize.getWidth(); - int32_t layerHeight = layerSize.getHeight(); - - // Align the layer orientation with the buffer before comparism - if (mTransformHint & ui::Transform::ROT_90) { - std::swap(layerWidth, layerHeight); - } - - return layerWidth != bufferWidth || layerHeight != bufferHeight; -} - void Layer::decrementPendingBufferCount() { int32_t pendingBuffers = --mPendingBufferTransactions; tracePendingBufferCount(pendingBuffers); @@ -3821,54 +3788,6 @@ bool Layer::isProtected() const { (mBufferInfo.mBuffer->getUsage() & GRALLOC_USAGE_PROTECTED); } -bool Layer::needsFiltering(const DisplayDevice* display) const { - if (!hasBufferOrSidebandStream()) { - return false; - } - const auto outputLayer = findOutputLayerForDisplay(display); - if (outputLayer == nullptr) { - return false; - } - - // We need filtering if the sourceCrop rectangle size does not match the - // displayframe rectangle size (not a 1:1 render) - const auto& compositionState = outputLayer->getState(); - const auto displayFrame = compositionState.displayFrame; - const auto sourceCrop = compositionState.sourceCrop; - return sourceCrop.getHeight() != displayFrame.getHeight() || - sourceCrop.getWidth() != displayFrame.getWidth(); -} - -bool Layer::needsFilteringForScreenshots(const DisplayDevice* display, - const ui::Transform& inverseParentTransform) const { - if (!hasBufferOrSidebandStream()) { - return false; - } - const auto outputLayer = findOutputLayerForDisplay(display); - if (outputLayer == nullptr) { - return false; - } - - // We need filtering if the sourceCrop rectangle size does not match the - // viewport rectangle size (not a 1:1 render) - const auto& compositionState = outputLayer->getState(); - const ui::Transform& displayTransform = display->getTransform(); - const ui::Transform inverseTransform = inverseParentTransform * displayTransform.inverse(); - // Undo the transformation of the displayFrame so that we're back into - // layer-stack space. - const Rect frame = inverseTransform.transform(compositionState.displayFrame); - const FloatRect sourceCrop = compositionState.sourceCrop; - - int32_t frameHeight = frame.getHeight(); - int32_t frameWidth = frame.getWidth(); - // If the display transform had a rotational component then undo the - // rotation so that the orientation matches the source crop. - if (displayTransform.getOrientation() & ui::Transform::ROT_90) { - std::swap(frameHeight, frameWidth); - } - return sourceCrop.getHeight() != frameHeight || sourceCrop.getWidth() != frameWidth; -} - void Layer::latchAndReleaseBuffer() { if (hasReadyFrame()) { bool ignored = false; @@ -4014,7 +3933,6 @@ void Layer::updateSnapshot(bool updateGeometry) { snapshot->layerOpaqueFlagSet = (mDrawingState.flags & layer_state_t::eLayerOpaque) == layer_state_t::eLayerOpaque; snapshot->isHdrY410 = isHdrY410(); - snapshot->bufferNeedsFiltering = bufferNeedsFiltering(); sp p = mDrawingParent.promote(); if (p != nullptr) { snapshot->parentTransform = p->getTransform(); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 3384e4af2d..6ba772f2f0 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -1038,10 +1038,6 @@ private: bool willPresentCurrentTransaction() const; - // Returns true if the transformed buffer size does not match the layer size and we need - // to apply filtering. - bool bufferNeedsFiltering() const; - void callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence, diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp index c31a2e3cfc..b9c8b78f55 100644 --- a/services/surfaceflinger/LayerFE.cpp +++ b/services/surfaceflinger/LayerFE.cpp @@ -249,14 +249,11 @@ void LayerFE::prepareBufferStateClientComposition( layerSettings.frameNumber = mSnapshot->frameNumber; layerSettings.bufferId = mSnapshot->externalTexture->getId(); - const bool useFiltering = targetSettings.needsFiltering || - mSnapshot->geomLayerTransform.needsBilinearFiltering() || - mSnapshot->bufferNeedsFiltering; - // Query the texture matrix given our current filtering mode. float textureMatrix[16]; getDrawingTransformMatrix(layerSettings.source.buffer.buffer, mSnapshot->geomContentCrop, - mSnapshot->geomBufferTransform, useFiltering, textureMatrix); + mSnapshot->geomBufferTransform, targetSettings.needsFiltering, + textureMatrix); if (mSnapshot->geomBufferUsesDisplayInverseTransform) { /* @@ -306,7 +303,7 @@ void LayerFE::prepareBufferStateClientComposition( mat4::translate(vec4(translateX, translateY, 0.f, 1.f)) * mat4::scale(vec4(scaleWidth, scaleHeight, 1.0f, 1.0f)); - layerSettings.source.buffer.useTextureFiltering = useFiltering; + layerSettings.source.buffer.useTextureFiltering = targetSettings.needsFiltering; layerSettings.source.buffer.textureTransform = mat4(static_cast(textureMatrix)) * tr; diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp index 554fae401e..2b4375b0fa 100644 --- a/services/surfaceflinger/LayerRenderArea.cpp +++ b/services/surfaceflinger/LayerRenderArea.cpp @@ -39,8 +39,8 @@ void reparentForDrawing(const sp& oldParent, const sp& newParent, LayerRenderArea::LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool childrenOnly, - const Rect& layerStackRect, bool allowSecureLayers) - : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, layerStackRect, allowSecureLayers), + bool allowSecureLayers) + : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, allowSecureLayers), mLayer(std::move(layer)), mCrop(crop), mFlinger(flinger), @@ -50,33 +50,17 @@ const ui::Transform& LayerRenderArea::getTransform() const { return mTransform; } -Rect LayerRenderArea::getBounds() const { - return mLayer->getBufferSize(mLayer->getDrawingState()); -} - -int LayerRenderArea::getHeight() const { - return mLayer->getBufferSize(mLayer->getDrawingState()).getHeight(); -} - -int LayerRenderArea::getWidth() const { - return mLayer->getBufferSize(mLayer->getDrawingState()).getWidth(); -} - bool LayerRenderArea::isSecure() const { return mAllowSecureLayers; } -bool LayerRenderArea::needsFiltering() const { - return mNeedsFiltering; -} - sp LayerRenderArea::getDisplayDevice() const { return nullptr; } Rect LayerRenderArea::getSourceCrop() const { if (mCrop.isEmpty()) { - return getBounds(); + return mLayer->getBufferSize(mLayer->getDrawingState()); } else { return mCrop; } @@ -85,10 +69,6 @@ Rect LayerRenderArea::getSourceCrop() const { void LayerRenderArea::render(std::function drawLayers) { using namespace std::string_literals; - const Rect sourceCrop = getSourceCrop(); - // no need to check rotation because there is none - mNeedsFiltering = sourceCrop.width() != getReqWidth() || sourceCrop.height() != getReqHeight(); - // If layer is offscreen, update mirroring info if it exists if (mLayer->isRemovedFromCurrentState()) { mLayer->traverse(LayerVector::StateSet::Drawing, @@ -116,7 +96,7 @@ void LayerRenderArea::render(std::function drawLayers) { LayerMetadata()}); { Mutex::Autolock _l(mFlinger.mStateLock); - reparentForDrawing(mLayer, screenshotParentLayer, sourceCrop); + reparentForDrawing(mLayer, screenshotParentLayer, getSourceCrop()); } drawLayers(); { diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h index 41273e01c1..322dbd1bf4 100644 --- a/services/surfaceflinger/LayerRenderArea.h +++ b/services/surfaceflinger/LayerRenderArea.h @@ -33,15 +33,10 @@ class SurfaceFlinger; class LayerRenderArea : public RenderArea { public: LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, - ui::Dataspace reqDataSpace, bool childrenOnly, const Rect& layerStackRect, - bool allowSecureLayers); + ui::Dataspace reqDataSpace, bool childrenOnly, bool allowSecureLayers); const ui::Transform& getTransform() const override; - Rect getBounds() const override; - int getHeight() const override; - int getWidth() const override; bool isSecure() const override; - bool needsFiltering() const override; sp getDisplayDevice() const override; Rect getSourceCrop() const override; @@ -53,7 +48,6 @@ private: const Rect mCrop; ui::Transform mTransform; - bool mNeedsFiltering = false; SurfaceFlinger& mFlinger; const bool mChildrenOnly; diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index 3c20e3b1e2..910fce043c 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -25,14 +25,12 @@ public: static float getCaptureFillValue(CaptureFill captureFill); RenderArea(ui::Size reqSize, CaptureFill captureFill, ui::Dataspace reqDataSpace, - const Rect& layerStackRect, bool allowSecureLayers = false, - RotationFlags rotation = ui::Transform::ROT_0) + bool allowSecureLayers = false, RotationFlags rotation = ui::Transform::ROT_0) : mAllowSecureLayers(allowSecureLayers), mReqSize(reqSize), mReqDataSpace(reqDataSpace), mCaptureFill(captureFill), - mRotationFlags(rotation), - mLayerStackSpaceRect(layerStackRect) {} + mRotationFlags(rotation) {} static std::function>>()> fromTraverseLayersLambda( std::function traverseLayers) { @@ -58,20 +56,10 @@ public: // blacked out / skipped when rendered to an insecure render area. virtual bool isSecure() const = 0; - // Returns true if the otherwise disabled layer filtering should be - // enabled when rendering to this render area. - virtual bool needsFiltering() const = 0; - // Returns the transform to be applied on layers to transform them into // the logical render area. virtual const ui::Transform& getTransform() const = 0; - // Returns the size of the logical render area. Layers are clipped to the - // logical render area. - virtual int getWidth() const = 0; - virtual int getHeight() const = 0; - virtual Rect getBounds() const = 0; - // Returns the source crop of the render area. The source crop defines // how layers are projected from the logical render area onto the physical // render area. It can be larger than the logical render area. It can @@ -98,9 +86,6 @@ public: virtual sp getDisplayDevice() const = 0; - // Returns the source display viewport. - const Rect& getLayerStackSpaceRect() const { return mLayerStackSpaceRect; } - // If this is a LayerRenderArea, return the root layer of the // capture operation. virtual sp getParentLayer() const { return nullptr; } diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index 6d195b9f7b..a1d5cd7379 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -27,11 +27,8 @@ namespace android { std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutputArgs args) { std::shared_ptr output = compositionengine::impl::createOutputTemplated< ScreenCaptureOutput, compositionengine::CompositionEngine, const RenderArea&, - std::unordered_set, const compositionengine::Output::ColorProfile&, bool>(args.compositionEngine, args.renderArea, - std::move( - args.filterForScreenshot), args.colorProfile, args.regionSampling); output->editState().isSecure = args.renderArea.isSecure(); @@ -45,12 +42,11 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp .setHasWideColorGamut(true) .Build())); - ui::Rotation orientation = ui::Transform::toRotation(args.renderArea.getRotationFlags()); - Rect orientedDisplaySpaceRect{args.renderArea.getReqWidth(), args.renderArea.getReqHeight()}; - output->setProjection(orientation, args.renderArea.getLayerStackSpaceRect(), - orientedDisplaySpaceRect); - - Rect sourceCrop = args.renderArea.getSourceCrop(); + const Rect& sourceCrop = args.renderArea.getSourceCrop(); + const ui::Rotation orientation = ui::Transform::toRotation(args.renderArea.getRotationFlags()); + const Rect orientedDisplaySpaceRect{args.renderArea.getReqWidth(), + args.renderArea.getReqHeight()}; + output->setProjection(orientation, sourceCrop, orientedDisplaySpaceRect); output->setDisplaySize({sourceCrop.getWidth(), sourceCrop.getHeight()}); { @@ -64,13 +60,9 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp } ScreenCaptureOutput::ScreenCaptureOutput( - const RenderArea& renderArea, - std::unordered_set filterForScreenshot, - const compositionengine::Output::ColorProfile& colorProfile, bool regionSampling) - : mRenderArea(renderArea), - mFilterForScreenshot(std::move(filterForScreenshot)), - mColorProfile(colorProfile), - mRegionSampling(regionSampling) {} + const RenderArea& renderArea, const compositionengine::Output::ColorProfile& colorProfile, + bool regionSampling) + : mRenderArea(renderArea), mColorProfile(colorProfile), mRegionSampling(regionSampling) {} void ScreenCaptureOutput::updateColorProfile(const compositionengine::CompositionRefreshArgs&) { auto& outputState = editState(); @@ -115,9 +107,4 @@ ScreenCaptureOutput::generateClientCompositionRequests( return clientCompositionLayers; } -bool ScreenCaptureOutput::layerNeedsFiltering(const compositionengine::OutputLayer* layer) const { - return mRenderArea.needsFiltering() || - mFilterForScreenshot.find(&layer->getLayerFE()) != mFilterForScreenshot.end(); -} - } // namespace android diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h index 5dffc1d530..4e5a0ccfd2 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.h +++ b/services/surfaceflinger/ScreenCaptureOutput.h @@ -33,7 +33,6 @@ struct ScreenCaptureOutputArgs { std::shared_ptr buffer; float sdrWhitePointNits; float displayBrightnessNits; - std::unordered_set filterForScreenshot; bool regionSampling; }; @@ -44,7 +43,6 @@ struct ScreenCaptureOutputArgs { class ScreenCaptureOutput : public compositionengine::impl::Output { public: ScreenCaptureOutput(const RenderArea& renderArea, - std::unordered_set filterForScreenshot, const compositionengine::Output::ColorProfile& colorProfile, bool regionSampling); @@ -54,15 +52,12 @@ public: bool supportsProtectedContent, ui::Dataspace outputDataspace, std::vector& outLayerFEs) override; - bool layerNeedsFiltering(const compositionengine::OutputLayer*) const override; - protected: bool getSkipColorTransform() const override { return false; } renderengine::DisplaySettings generateClientCompositionDisplaySettings() const override; private: const RenderArea& mRenderArea; - const std::unordered_set mFilterForScreenshot; const compositionengine::Output::ColorProfile& mColorProfile; const bool mRegionSampling; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 0dc8b05993..ac61e94e01 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6559,13 +6559,10 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return BAD_VALUE; } - Rect layerStackSpaceRect(crop.left, crop.top, crop.left + reqSize.width, - crop.top + reqSize.height); bool childrenOnly = args.childrenOnly; RenderAreaFuture renderAreaFuture = ftl::defer([=]() -> std::unique_ptr { return std::make_unique(*this, parent, crop, reqSize, dataspace, - childrenOnly, layerStackSpaceRect, - args.captureSecureLayers); + childrenOnly, args.captureSecureLayers); }); auto traverseLayers = [parent, args, excludeLayerIds](const LayerVector::Visitor& visitor) { @@ -6720,9 +6717,6 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( ScreenCaptureResults& captureResults) { ATRACE_CALL(); - const auto& display = renderArea->getDisplayDevice(); - const auto& transform = renderArea->getTransform(); - std::unordered_set filterForScreenshot; auto layers = getLayerSnapshots(); for (auto& [layer, layerFE] : layers) { frontend::LayerSnapshot* snapshot = layerFE->mSnapshot.get(); @@ -6730,9 +6724,6 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( captureResults.capturedHdrLayers |= isHdrLayer(*snapshot); layerFE->mSnapshot->geomLayerTransform = renderArea->getTransform() * layerFE->mSnapshot->geomLayerTransform; - if (layer->needsFilteringForScreenshots(display.get(), transform)) { - filterForScreenshot.insert(layerFE.get()); - } } // We allow the system server to take screenshots of secure layers for @@ -6783,9 +6774,9 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( }; auto present = [this, buffer = std::move(buffer), dataspace, sdrWhitePointNits, - displayBrightnessNits, filterForScreenshot = std::move(filterForScreenshot), - grayscale, layerFEs = copyLayerFEs(), layerStack, regionSampling, - renderArea = std::move(renderArea), renderIntent]() -> FenceResult { + displayBrightnessNits, grayscale, layerFEs = copyLayerFEs(), layerStack, + regionSampling, renderArea = std::move(renderArea), + renderIntent]() -> FenceResult { std::unique_ptr compositionEngine = mFactory.createCompositionEngine(); compositionEngine->setRenderEngine(mRenderEngine.get()); @@ -6801,7 +6792,6 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( .buffer = std::move(buffer), .sdrWhitePointNits = sdrWhitePointNits, .displayBrightnessNits = displayBrightnessNits, - .filterForScreenshot = std::move(filterForScreenshot), .regionSampling = regionSampling}); const float colorSaturation = grayscale ? 0 : 1; diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp index acfc1d4dc8..11719c435e 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp @@ -166,7 +166,7 @@ void LayerFuzzer::invokeBufferStateLayer() { {mFdp.ConsumeIntegral(), mFdp.ConsumeIntegral()} /*reqSize*/, mFdp.PickValueInArray(kDataspaces), mFdp.ConsumeBool(), - getFuzzedRect(), mFdp.ConsumeBool()); + mFdp.ConsumeBool()); layerArea.render([]() {} /*drawLayers*/); if (!ownsHandle) { -- cgit v1.2.3-59-g8ed1b From 3e5965f3184a828a1b7ee2b5bd5d5c2177807a35 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 7 Apr 2023 18:00:58 +0000 Subject: Support screenshots of HDR content Previously screenshots always rendered to either an SDR or a wide gamut colorspace. For screenshotting HDR content, this is only appropriate when the resulting screenshot (a) never leaves the device and (b) the relevant code has workarounds for the display to appropriately handle its luminance range. HDR screenshots will now have two paths: * A standard path for rendering to HLG. HLG was chosen because the OOTF shape is less hand-wavey than PQ's, does not require metadata, and bands less at 8-bits of color. * A special path for "display-native" screenshots. This is for use-cases like screen rotation where there are stricter color accuracy requirements for round-tripping. Skia already encodes the resulting screenshot by supplying an HLG CICP alongside a backwards-compatible transfer function, so it's only sufficient to change how SurfaceFlinger renders. Bug: 242324609 Bug: 276812775 Test: screencap binary Test: rotation animation Test: swiping in Recents Change-Id: Ic9edb92391d3beb38d076fba8f15e3fdcc2b8f50 --- libs/gui/LayerState.cpp | 4 +- libs/gui/aidl/android/gui/ISurfaceComposer.aidl | 4 + libs/gui/include/gui/DisplayCaptureArgs.h | 11 +- libs/renderengine/skia/SkiaRenderEngine.cpp | 6 +- libs/shaders/shaders.cpp | 58 ++++++---- services/surfaceflinger/DisplayRenderArea.cpp | 11 +- services/surfaceflinger/DisplayRenderArea.h | 4 +- services/surfaceflinger/LayerRenderArea.cpp | 5 +- services/surfaceflinger/LayerRenderArea.h | 3 +- services/surfaceflinger/RegionSamplingThread.cpp | 4 +- services/surfaceflinger/RenderArea.h | 12 ++- services/surfaceflinger/ScreenCaptureOutput.cpp | 2 +- services/surfaceflinger/ScreenCaptureOutput.h | 2 + services/surfaceflinger/SurfaceFlinger.cpp | 118 ++++++++++++++------- .../fuzzer/surfaceflinger_layer_fuzzer.cpp | 3 +- .../tests/unittests/CompositionTest.cpp | 2 +- 16 files changed, 173 insertions(+), 76 deletions(-) (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index fee91a40c2..2322b70d1c 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -897,11 +897,11 @@ status_t CaptureArgs::writeToParcel(Parcel* output) const { SAFE_PARCEL(output->writeInt32, static_cast(dataspace)); SAFE_PARCEL(output->writeBool, allowProtected); SAFE_PARCEL(output->writeBool, grayscale); - SAFE_PARCEL(output->writeInt32, excludeHandles.size()); for (auto& excludeHandle : excludeHandles) { SAFE_PARCEL(output->writeStrongBinder, excludeHandle); } + SAFE_PARCEL(output->writeBool, hintForSeamlessTransition); return NO_ERROR; } @@ -918,7 +918,6 @@ status_t CaptureArgs::readFromParcel(const Parcel* input) { dataspace = static_cast(value); SAFE_PARCEL(input->readBool, &allowProtected); SAFE_PARCEL(input->readBool, &grayscale); - int32_t numExcludeHandles = 0; SAFE_PARCEL_READ_SIZE(input->readInt32, &numExcludeHandles, input->dataSize()); excludeHandles.reserve(numExcludeHandles); @@ -927,6 +926,7 @@ status_t CaptureArgs::readFromParcel(const Parcel* input) { SAFE_PARCEL(input->readStrongBinder, &binder); excludeHandles.emplace(binder); } + SAFE_PARCEL(input->readBool, &hintForSeamlessTransition); return NO_ERROR; } diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index aa58e2e580..ec3266ca83 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -230,6 +230,10 @@ interface ISurfaceComposer { */ void captureDisplay(in DisplayCaptureArgs args, IScreenCaptureListener listener); + /** + * Capture the specified screen. This requires the READ_FRAME_BUFFER + * permission. + */ void captureDisplayById(long displayId, IScreenCaptureListener listener); /** diff --git a/libs/gui/include/gui/DisplayCaptureArgs.h b/libs/gui/include/gui/DisplayCaptureArgs.h index 5c794aea37..2676e0a338 100644 --- a/libs/gui/include/gui/DisplayCaptureArgs.h +++ b/libs/gui/include/gui/DisplayCaptureArgs.h @@ -41,7 +41,7 @@ struct CaptureArgs : public Parcelable { bool captureSecureLayers{false}; int32_t uid{UNSET_UID}; // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured - // result will be in the display's colorspace. + // result will be in a colorspace appropriate for capturing the display contents // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be // different from SRGB (byte per color), and failed when checking colors in tests. // NOTE: In normal cases, we want the screen to be captured in display's colorspace. @@ -59,6 +59,15 @@ struct CaptureArgs : public Parcelable { std::unordered_set, SpHash> excludeHandles; + // Hint that the caller will use the screenshot animation as part of a transition animation. + // The canonical example would be screen rotation - in such a case any color shift in the + // screenshot is a detractor so composition in the display's colorspace is required. + // Otherwise, the system may choose a colorspace that is more appropriate for use-cases + // such as file encoding or for blending HDR content into an ap's UI, where the display's + // exact colorspace is not an appropriate intermediate result. + // Note that if the caller is requesting a specific dataspace, this hint does nothing. + bool hintForSeamlessTransition = false; + virtual status_t writeToParcel(Parcel* output) const; virtual status_t readFromParcel(const Parcel* input); }; diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 8256dd8e69..2a51493cea 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -517,16 +517,18 @@ sk_sp SkiaRenderEngine::createRuntimeEffectShader( } else { runtimeEffect = effectIter->second; } + mat4 colorTransform = parameters.layer.colorTransform; colorTransform *= mat4::scale(vec4(parameters.layerDimmingRatio, parameters.layerDimmingRatio, parameters.layerDimmingRatio, 1.f)); + const auto targetBuffer = parameters.layer.source.buffer.buffer; const auto graphicBuffer = targetBuffer ? targetBuffer->getBuffer() : nullptr; const auto hardwareBuffer = graphicBuffer ? graphicBuffer->toAHardwareBuffer() : nullptr; - return createLinearEffectShader(parameters.shader, effect, runtimeEffect, colorTransform, - parameters.display.maxLuminance, + return createLinearEffectShader(parameters.shader, effect, runtimeEffect, + std::move(colorTransform), parameters.display.maxLuminance, parameters.display.currentLuminanceNits, parameters.layer.source.buffer.maxLuminanceNits, hardwareBuffer, parameters.display.renderIntent); diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index a3c403e551..19518eaecc 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -191,31 +191,35 @@ void generateLuminanceScalesForOOTF(ui::Dataspace inputDataspace, ui::Dataspace )"); break; default: + // Input is SDR so map to its white point luminance switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { - case HAL_DATASPACE_TRANSFER_ST2084: + // Max HLG output is nominally 1000 nits, but BT. 2100-2 allows + // for gamma correcting the HLG OOTF for displays with a different + // dynamic range. Scale to 1000 nits to apply an inverse OOTF against + // a reference display correctly. + // TODO: Use knowledge of the dimming ratio here to prevent + // unintended gamma shaft. case HAL_DATASPACE_TRANSFER_HLG: - // SDR -> HDR tonemap shader.append(R"( float3 ScaleLuminance(float3 xyz) { - return xyz * in_libtonemap_inputMaxLuminance; + return xyz * 1000.0; } )"); break; default: - // Input and output are both SDR, so no tone-mapping is expected so - // no-op the luminance normalization. shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * in_libtonemap_displayMaxLuminance; - } - )"); + float3 ScaleLuminance(float3 xyz) { + return xyz * in_libtonemap_displayMaxLuminance; + } + )"); break; } } } // Normalizes from absolute light back to relative light (maps from [0, maxNits] back to [0, 1]) -static void generateLuminanceNormalizationForOOTF(ui::Dataspace outputDataspace, +static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, + ui::Dataspace outputDataspace, std::string& shader) { switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { case HAL_DATASPACE_TRANSFER_ST2084: @@ -226,11 +230,28 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace outputDataspace, )"); break; case HAL_DATASPACE_TRANSFER_HLG: - shader.append(R"( - float3 NormalizeLuminance(float3 xyz) { - return xyz / 1000.0; - } - )"); + switch (inputDataspace & HAL_DATASPACE_TRANSFER_MASK) { + case HAL_DATASPACE_TRANSFER_HLG: + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + return xyz / 1000.0; + } + )"); + break; + default: + // Transcoding to HLG requires applying the inverse OOTF + // with the expectation that the OOTF is then applied during + // tonemapping downstream. + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + // BT. 2100-2 operates on normalized luminances, + // so renormalize to the input + float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2) / 1000.0; + return xyz * ootfGain; + } + )"); + break; + } break; default: shader.append(R"( @@ -250,7 +271,7 @@ void generateOOTF(ui::Dataspace inputDataspace, ui::Dataspace outputDataspace, .c_str()); generateLuminanceScalesForOOTF(inputDataspace, outputDataspace, shader); - generateLuminanceNormalizationForOOTF(outputDataspace, shader); + generateLuminanceNormalizationForOOTF(inputDataspace, outputDataspace, shader); shader.append(R"( float3 OOTF(float3 linearRGB, float3 xyz) { @@ -501,9 +522,8 @@ std::vector buildLinearEffectUniforms( tonemap::Metadata metadata{.displayMaxLuminance = maxDisplayLuminance, // If the input luminance is unknown, use display luminance (aka, - // no-op any luminance changes) - // This will be the case for eg screenshots in addition to - // uncalibrated displays + // no-op any luminance changes). + // This is expected to only be meaningful for PQ content .contentMaxLuminance = maxLuminance > 0 ? maxLuminance : maxDisplayLuminance, .currentDisplayLuminance = currentDisplayLuminanceNits > 0 diff --git a/services/surfaceflinger/DisplayRenderArea.cpp b/services/surfaceflinger/DisplayRenderArea.cpp index 8f39e26e0f..e55cd3ea42 100644 --- a/services/surfaceflinger/DisplayRenderArea.cpp +++ b/services/surfaceflinger/DisplayRenderArea.cpp @@ -35,21 +35,24 @@ std::unique_ptr DisplayRenderArea::create(wp di const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool useIdentityTransform, + bool hintForSeamlessTransition, bool allowSecureLayers) { if (auto display = displayWeak.promote()) { // Using new to access a private constructor. return std::unique_ptr( new DisplayRenderArea(std::move(display), sourceCrop, reqSize, reqDataSpace, - useIdentityTransform, allowSecureLayers)); + useIdentityTransform, hintForSeamlessTransition, + allowSecureLayers)); } return nullptr; } DisplayRenderArea::DisplayRenderArea(sp display, const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace reqDataSpace, - bool useIdentityTransform, bool allowSecureLayers) - : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, allowSecureLayers, - applyDeviceOrientation(useIdentityTransform, *display)), + bool useIdentityTransform, bool hintForSeamlessTransition, + bool allowSecureLayers) + : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, hintForSeamlessTransition, + allowSecureLayers, applyDeviceOrientation(useIdentityTransform, *display)), mDisplay(std::move(display)), mSourceCrop(sourceCrop) {} diff --git a/services/surfaceflinger/DisplayRenderArea.h b/services/surfaceflinger/DisplayRenderArea.h index ce5410a90d..9a4981c881 100644 --- a/services/surfaceflinger/DisplayRenderArea.h +++ b/services/surfaceflinger/DisplayRenderArea.h @@ -30,6 +30,7 @@ public: static std::unique_ptr create(wp, const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace, bool useIdentityTransform, + bool hintForSeamlessTransition, bool allowSecureLayers = true); const ui::Transform& getTransform() const override; @@ -39,7 +40,8 @@ public: private: DisplayRenderArea(sp, const Rect& sourceCrop, ui::Size reqSize, - ui::Dataspace, bool useIdentityTransform, bool allowSecureLayers = true); + ui::Dataspace, bool useIdentityTransform, bool hintForSeamlessTransition, + bool allowSecureLayers = true); const sp mDisplay; const Rect mSourceCrop; diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp index 1b8ff28a76..f6b5afc6a8 100644 --- a/services/surfaceflinger/LayerRenderArea.cpp +++ b/services/surfaceflinger/LayerRenderArea.cpp @@ -40,8 +40,9 @@ void reparentForDrawing(const sp& oldParent, const sp& newParent, LayerRenderArea::LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool childrenOnly, bool allowSecureLayers, const ui::Transform& layerTransform, - const Rect& layerBufferSize) - : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, allowSecureLayers), + const Rect& layerBufferSize, bool hintForSeamlessTransition) + : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, hintForSeamlessTransition, + allowSecureLayers), mLayer(std::move(layer)), mLayerTransform(layerTransform), mLayerBufferSize(layerBufferSize), diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h index 9bb13b3d6a..aa609eea38 100644 --- a/services/surfaceflinger/LayerRenderArea.h +++ b/services/surfaceflinger/LayerRenderArea.h @@ -34,7 +34,8 @@ class LayerRenderArea : public RenderArea { public: LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool childrenOnly, bool allowSecureLayers, - const ui::Transform& layerTransform, const Rect& layerBufferSize); + const ui::Transform& layerTransform, const Rect& layerBufferSize, + bool hintForSeamlessTransition); const ui::Transform& getTransform() const override; bool isSecure() const override; diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 531d277ffc..8f658d5a09 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -277,10 +277,12 @@ void RegionSamplingThread::captureSample() { const Rect sampledBounds = sampleRegion.bounds(); constexpr bool kUseIdentityTransform = false; + constexpr bool kHintForSeamlessTransition = false; SurfaceFlinger::RenderAreaFuture renderAreaFuture = ftl::defer([=] { return DisplayRenderArea::create(displayWeak, sampledBounds, sampledBounds.getSize(), - ui::Dataspace::V0_SRGB, kUseIdentityTransform); + ui::Dataspace::V0_SRGB, kUseIdentityTransform, + kHintForSeamlessTransition); }); std::unordered_set, SpHash> listeners; diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index 910fce043c..71b85bd3b2 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -25,12 +25,14 @@ public: static float getCaptureFillValue(CaptureFill captureFill); RenderArea(ui::Size reqSize, CaptureFill captureFill, ui::Dataspace reqDataSpace, - bool allowSecureLayers = false, RotationFlags rotation = ui::Transform::ROT_0) + bool hintForSeamlessTransition, bool allowSecureLayers = false, + RotationFlags rotation = ui::Transform::ROT_0) : mAllowSecureLayers(allowSecureLayers), mReqSize(reqSize), mReqDataSpace(reqDataSpace), mCaptureFill(captureFill), - mRotationFlags(rotation) {} + mRotationFlags(rotation), + mHintForSeamlessTransition(hintForSeamlessTransition) {} static std::function>>()> fromTraverseLayersLambda( std::function traverseLayers) { @@ -90,6 +92,10 @@ public: // capture operation. virtual sp getParentLayer() const { return nullptr; } + // Returns whether the render result may be used for system animations that + // must preserve the exact colors of the display. + bool getHintForSeamlessTransition() const { return mHintForSeamlessTransition; } + protected: const bool mAllowSecureLayers; @@ -98,7 +104,7 @@ private: const ui::Dataspace mReqDataSpace; const CaptureFill mCaptureFill; const RotationFlags mRotationFlags; - const Rect mLayerStackSpaceRect; + const bool mHintForSeamlessTransition; }; } // namespace android diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index a1d5cd7379..b70b53d05a 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -36,6 +36,7 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp output->setLayerFilter({args.layerStack}); output->setRenderSurface(std::make_unique(std::move(args.buffer))); output->setDisplayBrightness(args.sdrWhitePointNits, args.displayBrightnessNits); + output->editState().clientTargetBrightness = args.targetBrightness; output->setDisplayColorProfile(std::make_unique( compositionengine::DisplayColorProfileCreationArgsBuilder() @@ -75,7 +76,6 @@ renderengine::DisplaySettings ScreenCaptureOutput::generateClientCompositionDisp auto clientCompositionDisplay = compositionengine::impl::Output::generateClientCompositionDisplaySettings(); clientCompositionDisplay.clip = mRenderArea.getSourceCrop(); - clientCompositionDisplay.targetLuminanceNits = -1; return clientCompositionDisplay; } diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h index 4e5a0ccfd2..3c307b0733 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.h +++ b/services/surfaceflinger/ScreenCaptureOutput.h @@ -33,6 +33,8 @@ struct ScreenCaptureOutputArgs { std::shared_ptr buffer; float sdrWhitePointNits; float displayBrightnessNits; + // Counterintuitively, when targetBrightness > 1.0 then dim the scene. + float targetBrightness; bool regionSampling; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c88bff5ed8..d406afff1f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6915,6 +6915,32 @@ status_t SurfaceFlinger::setSchedAttr(bool enabled) { return NO_ERROR; } +namespace { + +ui::Dataspace pickBestDataspace(ui::Dataspace requestedDataspace, const DisplayDevice* display, + bool capturingHdrLayers, bool hintForSeamlessTransition) { + if (requestedDataspace != ui::Dataspace::UNKNOWN || display == nullptr) { + return requestedDataspace; + } + + const auto& state = display->getCompositionDisplay()->getState(); + + const auto dataspaceForColorMode = ui::pickDataspaceFor(state.colorMode); + + if (capturingHdrLayers && !hintForSeamlessTransition) { + // For now since we only support 8-bit screenshots, just use HLG and + // assume that 1.0 >= display max luminance. This isn't quite as future + // proof as PQ is, but is good enough. + // Consider using PQ once we support 16-bit screenshots and we're able + // to consistently supply metadata to image encoders. + return ui::Dataspace::BT2020_HLG; + } + + return dataspaceForColorMode; +} + +} // namespace + status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, const sp& captureListener) { ATRACE_CALL(); @@ -6930,7 +6956,6 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, ui::LayerStack layerStack; ui::Size reqSize(args.width, args.height); std::unordered_set excludeLayerIds; - ui::Dataspace dataspace; { Mutex::Autolock lock(mStateLock); sp display = getDisplayDeviceLocked(args.displayToken); @@ -6952,17 +6977,12 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, return NAME_NOT_FOUND; } } - - // Allow the caller to specify a dataspace regardless of the display's color mode, e.g. if - // it wants sRGB regardless of the display's wide color mode. - dataspace = args.dataspace == ui::Dataspace::UNKNOWN - ? ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode) - : args.dataspace; } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, dataspace, - args.useIdentityTransform, args.captureSecureLayers); + return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, + ui::Dataspace::UNKNOWN, args.useIdentityTransform, + args.hintForSeamlessTransition, args.captureSecureLayers); }); GetLayerSnapshotsFunction getLayerSnapshots; @@ -6988,7 +7008,6 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, ui::LayerStack layerStack; wp displayWeak; ui::Size size; - ui::Dataspace dataspace; { Mutex::Autolock lock(mStateLock); @@ -7000,12 +7019,12 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, displayWeak = display; layerStack = display->getLayerStack(); size = display->getLayerStackSpaceRect().getSize(); - dataspace = ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode); } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, Rect(), size, dataspace, + return DisplayRenderArea::create(displayWeak, Rect(), size, ui::Dataspace::UNKNOWN, false /* useIdentityTransform */, + false /* hintForSeamlessTransition */, false /* captureSecureLayers */); }); @@ -7047,7 +7066,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, sp parent; Rect crop(args.sourceCrop); std::unordered_set excludeLayerIds; - ui::Dataspace dataspace; + ui::Dataspace dataspace = args.dataspace; // Call this before holding mStateLock to avoid any deadlocking. bool canCaptureBlackoutContent = hasCaptureBlackoutContentPermission(); @@ -7094,12 +7113,6 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return NAME_NOT_FOUND; } } - - // The dataspace is depended on the color mode of display, that could use non-native mode - // (ex. displayP3) to enhance the content, but some cases are checking native RGB in bytes, - // and failed if display is not in native mode. This provide a way to force using native - // colors when capture. - dataspace = args.dataspace; } // mStateLock // really small crop or frameScale @@ -7128,7 +7141,8 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return std::make_unique(*this, parent, crop, reqSize, dataspace, childrenOnly, args.captureSecureLayers, - layerTransform, layerBufferSize); + layerTransform, layerBufferSize, + args.hintForSeamlessTransition); }); GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { @@ -7314,29 +7328,48 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( return ftl::yield(base::unexpected(PERMISSION_DENIED)).share(); } - captureResults.buffer = buffer->getBuffer(); - auto dataspace = renderArea->getReqDataSpace(); + auto capturedBuffer = buffer; + + auto requestedDataspace = renderArea->getReqDataSpace(); auto parent = renderArea->getParentLayer(); auto renderIntent = RenderIntent::TONE_MAP_COLORIMETRIC; auto sdrWhitePointNits = DisplayDevice::sDefaultMaxLumiance; auto displayBrightnessNits = DisplayDevice::sDefaultMaxLumiance; - if (dataspace == ui::Dataspace::UNKNOWN && parent) { + captureResults.capturedDataspace = requestedDataspace; + + { Mutex::Autolock lock(mStateLock); - auto display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { - return display.getLayerStack() == layerStack; - }); - if (!display) { - // If the layer is not on a display, use the dataspace for the default display. - display = getDefaultDisplayDeviceLocked(); + const DisplayDevice* display = nullptr; + if (parent) { + display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { + return display.getLayerStack() == layerStack; + }).get(); + } + + if (display == nullptr) { + display = renderArea->getDisplayDevice().get(); + } + + if (display == nullptr) { + display = getDefaultDisplayDeviceLocked().get(); } - dataspace = ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode); - renderIntent = display->getCompositionDisplay()->getState().renderIntent; - sdrWhitePointNits = display->getCompositionDisplay()->getState().sdrWhitePointNits; - displayBrightnessNits = display->getCompositionDisplay()->getState().displayBrightnessNits; + if (display != nullptr) { + const auto& state = display->getCompositionDisplay()->getState(); + captureResults.capturedDataspace = + pickBestDataspace(requestedDataspace, display, captureResults.capturedHdrLayers, + renderArea->getHintForSeamlessTransition()); + sdrWhitePointNits = state.sdrWhitePointNits; + displayBrightnessNits = state.displayBrightnessNits; + + if (requestedDataspace == ui::Dataspace::UNKNOWN) { + renderIntent = state.renderIntent; + } + } } - captureResults.capturedDataspace = dataspace; + + captureResults.buffer = capturedBuffer->getBuffer(); ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK}; if (!layers.empty()) { @@ -7353,9 +7386,9 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( return layerFEs; }; - auto present = [this, buffer = std::move(buffer), dataspace, sdrWhitePointNits, - displayBrightnessNits, grayscale, layerFEs = copyLayerFEs(), layerStack, - regionSampling, renderArea = std::move(renderArea), + auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace, + sdrWhitePointNits, displayBrightnessNits, grayscale, layerFEs = copyLayerFEs(), + layerStack, regionSampling, renderArea = std::move(renderArea), renderIntent]() -> FenceResult { std::unique_ptr compositionEngine = mFactory.createCompositionEngine(); @@ -7364,6 +7397,16 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( compositionengine::Output::ColorProfile colorProfile{.dataspace = dataspace, .renderIntent = renderIntent}; + float targetBrightness = 1.0f; + if (dataspace == ui::Dataspace::BT2020_HLG) { + const float maxBrightnessNits = displayBrightnessNits / sdrWhitePointNits * 203; + // With a low dimming ratio, don't fit the entire curve. Otherwise mixed content + // will appear way too bright. + if (maxBrightnessNits < 1000.f) { + targetBrightness = 1000.f / maxBrightnessNits; + } + } + std::shared_ptr output = createScreenCaptureOutput( ScreenCaptureOutputArgs{.compositionEngine = *compositionEngine, .colorProfile = colorProfile, @@ -7372,6 +7415,7 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( .buffer = std::move(buffer), .sdrWhitePointNits = sdrWhitePointNits, .displayBrightnessNits = displayBrightnessNits, + .targetBrightness = targetBrightness, .regionSampling = regionSampling}); const float colorSaturation = grayscale ? 0 : 1; diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp index c3dcb85686..921cae4e41 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp @@ -177,7 +177,8 @@ void LayerFuzzer::invokeBufferStateLayer() { {mFdp.ConsumeIntegral(), mFdp.ConsumeIntegral()} /*reqSize*/, mFdp.PickValueInArray(kDataspaces), mFdp.ConsumeBool(), - mFdp.ConsumeBool(), getFuzzedTransform(), getFuzzedRect()); + mFdp.ConsumeBool(), getFuzzedTransform(), getFuzzedRect(), + mFdp.ConsumeBool()); layerArea.render([]() {} /*drawLayers*/); if (!ownsHandle) { diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 156007b862..6ca21bd458 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -200,7 +200,7 @@ void CompositionTest::captureScreenComposition() { constexpr bool regionSampling = false; auto renderArea = DisplayRenderArea::create(mDisplay, sourceCrop, sourceCrop.getSize(), - ui::Dataspace::V0_SRGB, ui::Transform::ROT_0); + ui::Dataspace::V0_SRGB, true, true); auto traverseLayers = [this](const LayerVector::Visitor& visitor) { return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(), -- cgit v1.2.3-59-g8ed1b From 792b150a708a61faedaaa5983fbc5c5185984f50 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 2 May 2023 00:03:08 +0000 Subject: Don't overdim SDR content in an HLG screenshot. Aligning HLG and PQ to 1.0 == 203 nits made SDR assets in screenshots too dim, since both the colorspace and the color transform applied dimming. Removing dimming application from the color transform is a larger change, so just compensate when configuring the screenshot in SurfaceFlinger instead. Bug: 280347733 Test: HwAccelerationTest Test: Navigate in and out of recents Change-Id: Idfdb74c0c3b977717b870b2bb9a469be37d27dc9 --- libs/shaders/shaders.cpp | 6 +++++- services/surfaceflinger/ScreenCaptureOutput.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'services/surfaceflinger/ScreenCaptureOutput.cpp') diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index b8f2be111e..c85517a976 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -103,10 +103,14 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, // tonemapping downstream. // BT. 2100-2 operates on normalized luminances, so renormalize to the input to // correctly adjust gamma. + // Note that following BT. 2408 for HLG OETF actually maps 0.75 == ~264.96 nits, + // rather than 203 nits, because 203 nits == OOTF(invOETF(0.75)), so even though + // we originally scaled by 203 nits we need to re-normalize to 264.96 nits when + // converting to the correct brightness range. shader.append(R"( float3 NormalizeLuminance(float3 xyz) { float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2); - return xyz * ootfGain / 203.0; + return xyz * ootfGain / 264.96; } )"); break; diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index b70b53d05a..09dac23410 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -94,6 +94,16 @@ ScreenCaptureOutput::generateClientCompositionRequests( } } + if (outputDataspace == ui::Dataspace::BT2020_HLG) { + for (auto& layer : clientCompositionLayers) { + auto transfer = layer.sourceDataspace & ui::Dataspace::TRANSFER_MASK; + if (transfer != static_cast(ui::Dataspace::TRANSFER_HLG) && + transfer != static_cast(ui::Dataspace::TRANSFER_ST2084)) { + layer.whitePointNits *= (1000.0f / 203.0f); + } + } + } + Rect sourceCrop = mRenderArea.getSourceCrop(); compositionengine::LayerFE::LayerSettings fillLayer; fillLayer.source.buffer.buffer = nullptr; -- cgit v1.2.3-59-g8ed1b