From d11bade4e5d7b7818090d775f0bb4dea9d719206 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Mon, 1 Aug 2022 16:18:03 -0700 Subject: SF: build with ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION Change-Id: I347b2cf57f1df426d11d07a84075419597d4a442 Test: presubmit --- services/surfaceflinger/RegionSamplingThread.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 2487dbd793..570525598e 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -132,7 +132,7 @@ RegionSamplingThread::~RegionSamplingThread() { void RegionSamplingThread::addListener(const Rect& samplingArea, const wp& stopLayer, const sp& listener) { sp asBinder = IInterface::asBinder(listener); - asBinder->linkToDeath(this); + asBinder->linkToDeath(sp::fromExisting(this)); std::lock_guard lock(mSamplingMutex); mDescriptors.emplace(wp(asBinder), Descriptor{samplingArea, stopLayer, listener}); } @@ -344,8 +344,8 @@ void RegionSamplingThread::captureSample() { const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE; sp graphicBuffer = - new GraphicBuffer(sampledBounds.getWidth(), sampledBounds.getHeight(), - PIXEL_FORMAT_RGBA_8888, 1, usage, "RegionSamplingThread"); + sp::make(sampledBounds.getWidth(), sampledBounds.getHeight(), + PIXEL_FORMAT_RGBA_8888, 1, usage, "RegionSamplingThread"); const status_t bufferStatus = graphicBuffer->initCheck(); LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "captureSample: Buffer failed to allocate: %d", bufferStatus); -- cgit v1.2.3-59-g8ed1b From 439b60968f1fe544b79a5c5ba4657fca9e82b54a Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Wed, 14 Sep 2022 22:24:17 +0000 Subject: Remove coordinate swapping in region sampling Captured screens for region sampling were originally rendered upside-down, which required swapping coordinates to sample from the correct area in the output buffer. Since then, several fixes to the screenshot path have landed, which treat screen orientation consistently, so the captured images are no longer upside-down. But, the coordinate swapping was never removed. Remove it now, as sampling during landscape is now broken. Bug: 133849373 Bug: 241967077 Test: Split screen in landscape with settings app in dark mode, calculator in light mode, and trigger back gestures Change-Id: I52c65032d33d01a4407dc1b30215e7edac6eb1ea --- services/surfaceflinger/RegionSamplingThread.cpp | 19 +++--------- .../tests/unittests/RegionSamplingTest.cpp | 34 ---------------------- 2 files changed, 4 insertions(+), 49 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 570525598e..8dd3b0f59d 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -203,25 +203,14 @@ float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t st return 0.0f; } - // (b/133849373) ROT_90 screencap images produced upside down - auto area = sample_area; - if (orientation & ui::Transform::ROT_90) { - area.top = height - area.top; - area.bottom = height - area.bottom; - std::swap(area.top, area.bottom); - - area.left = width - area.left; - area.right = width - area.right; - std::swap(area.left, area.right); - } - - const uint32_t pixelCount = (area.bottom - area.top) * (area.right - area.left); + const uint32_t pixelCount = + (sample_area.bottom - sample_area.top) * (sample_area.right - sample_area.left); uint32_t accumulatedLuma = 0; // Calculates luma with approximation of Rec. 709 primaries - for (int32_t row = area.top; row < area.bottom; ++row) { + for (int32_t row = sample_area.top; row < sample_area.bottom; ++row) { const uint32_t* rowBase = data + row * stride; - for (int32_t column = area.left; column < area.right; ++column) { + for (int32_t column = sample_area.left; column < sample_area.right; ++column) { uint32_t pixel = rowBase[column]; const uint32_t r = pixel & 0xFF; const uint32_t g = (pixel >> 8) & 0xFF; diff --git a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp index f19e55409c..409e1ef5d7 100644 --- a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp +++ b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp @@ -106,40 +106,6 @@ TEST_F(RegionSamplingTest, bounds_checking) { testing::Eq(0.0)); } -// workaround for b/133849373 -TEST_F(RegionSamplingTest, orientation_90) { - std::generate(buffer.begin(), buffer.end(), - [n = 0]() mutable { return (n++ > (kStride * kHeight >> 1)) ? kBlack : kWhite; }); - - Rect tl_region{0, 0, 4, 4}; - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0, - tl_region), - testing::Eq(1.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180, - tl_region), - testing::Eq(1.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90, - tl_region), - testing::Eq(0.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270, - tl_region), - testing::Eq(0.0)); - - Rect br_region{kWidth - 4, kHeight - 4, kWidth, kHeight}; - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0, - br_region), - testing::Eq(0.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180, - br_region), - testing::Eq(0.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90, - br_region), - testing::Eq(1.0)); - EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270, - br_region), - testing::Eq(1.0)); -} - } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues -- cgit v1.2.3-59-g8ed1b From 9d77a5cf9d05b87b547f139dedf50ccfbebb140e Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Fri, 28 Oct 2022 06:31:21 +0000 Subject: SF: Use layer id for region sampling stop layer Cleanup before layer refbase removal. We can lookup the stop layer by using a unique id. Bug: 238781169 Test: presubmit Change-Id: I33da6899adebc33c814656591f78187f08c53e80 --- services/surfaceflinger/RegionSamplingThread.cpp | 9 +++++---- services/surfaceflinger/RegionSamplingThread.h | 5 +++-- services/surfaceflinger/SurfaceFlinger.cpp | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 8dd3b0f59d..6e64e0a13b 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -40,6 +40,7 @@ #include "DisplayDevice.h" #include "DisplayRenderArea.h" +#include "FrontEnd/LayerCreationArgs.h" #include "Layer.h" #include "Scheduler/VsyncController.h" #include "SurfaceFlinger.h" @@ -129,12 +130,12 @@ RegionSamplingThread::~RegionSamplingThread() { } } -void RegionSamplingThread::addListener(const Rect& samplingArea, const wp& stopLayer, +void RegionSamplingThread::addListener(const Rect& samplingArea, uint32_t stopLayerId, const sp& listener) { sp asBinder = IInterface::asBinder(listener); asBinder->linkToDeath(sp::fromExisting(this)); std::lock_guard lock(mSamplingMutex); - mDescriptors.emplace(wp(asBinder), Descriptor{samplingArea, stopLayer, listener}); + mDescriptors.emplace(wp(asBinder), Descriptor{samplingArea, stopLayerId, listener}); } void RegionSamplingThread::removeListener(const sp& listener) { @@ -291,8 +292,8 @@ void RegionSamplingThread::captureSample() { if (stopLayerFound) return; // Likewise if we just found a stop layer, set the flag and abort - for (const auto& [area, stopLayer, listener] : descriptors) { - if (layer == stopLayer.promote().get()) { + for (const auto& [area, stopLayerId, listener] : descriptors) { + if (stopLayerId != UNASSIGNED_LAYER_ID && layer->getSequence() == stopLayerId) { stopLayerFound = true; return; } diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h index 686b4b1e1f..b62b15cb6d 100644 --- a/services/surfaceflinger/RegionSamplingThread.h +++ b/services/surfaceflinger/RegionSamplingThread.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -73,7 +74,7 @@ public: // Add a listener to receive luma notifications. The luma reported via listener will // report the median luma for the layers under the stopLayerHandle, in the samplingArea region. - void addListener(const Rect& samplingArea, const wp& stopLayer, + void addListener(const Rect& samplingArea, uint32_t stopLayerId, const sp& listener); // Remove the listener to stop receiving median luma notifications. void removeListener(const sp& listener); @@ -87,7 +88,7 @@ public: private: struct Descriptor { Rect area = Rect::EMPTY_RECT; - wp stopLayer; + uint32_t stopLayerId; sp listener; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index cfebec70cb..8fefcb1a62 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1580,9 +1580,10 @@ status_t SurfaceFlinger::addRegionSamplingListener(const Rect& samplingArea, // LayerHandle::getLayer promotes the layer object in a binder thread but we will not destroy // the layer here since the caller has a strong ref to the layer's handle. - // TODO (b/238781169): replace layer with layer id - const wp stopLayer = LayerHandle::getLayer(stopLayerHandle); - mRegionSamplingThread->addListener(samplingArea, stopLayer, listener); + const sp stopLayer = LayerHandle::getLayer(stopLayerHandle); + mRegionSamplingThread->addListener(samplingArea, + stopLayer ? stopLayer->getSequence() : UNASSIGNED_LAYER_ID, + listener); return NO_ERROR; } -- cgit v1.2.3-59-g8ed1b From 7aa0eb74f5d3ae176cc4b3d51ce69512aae6926f Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Tue, 24 Jan 2023 03:59:27 +0000 Subject: SF: Change screenshot code to use snapshots This prepares us for frontend changes which do not rely on the Layer object. Instead we change the traversal function to return a z-ordered list of snapshots. This cl also changes some of the logic to check the snapshot instead of the layer drawing state. Bug: 238781169 Test: presubmit Test: manually test hdr listeners Change-Id: If508f9380fdef0414bbf448ece767be3e0bba9cf --- services/surfaceflinger/RegionSamplingThread.cpp | 3 +- services/surfaceflinger/RenderArea.h | 15 +++ services/surfaceflinger/SurfaceFlinger.cpp | 121 ++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 12 +- .../tests/unittests/CompositionTest.cpp | 4 +- .../tests/unittests/TestableSurfaceFlinger.h | 2 +- 6 files changed, 84 insertions(+), 73 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 6e64e0a13b..839500f8a0 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -344,12 +344,13 @@ void RegionSamplingThread::captureSample() { renderengine::impl::ExternalTexture::Usage:: WRITEABLE); } + auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); constexpr bool kRegionSampling = true; constexpr bool kGrayscale = false; if (const auto fenceResult = - mFlinger.captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer, + mFlinger.captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, buffer, kRegionSampling, kGrayscale, nullptr) .get(); fenceResult.ok()) { diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index 387364c03a..3c20e3b1e2 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -34,6 +34,21 @@ public: mRotationFlags(rotation), mLayerStackSpaceRect(layerStackRect) {} + static std::function>>()> fromTraverseLayersLambda( + std::function traverseLayers) { + return [traverseLayers = std::move(traverseLayers)]() { + std::vector>> layers; + traverseLayers([&](Layer* layer) { + // Layer::prepareClientComposition uses the layer's snapshot to populate the + // resulting LayerSettings. Calling Layer::updateSnapshot ensures that LayerSettings + // are generated with the layer's current buffer and geometry. + layer->updateSnapshot(true /* updateGeometry */); + layers.emplace_back(layer, layer->copyCompositionEngineLayerFE()); + }); + return layers; + }; + } + virtual ~RenderArea() = default; // Invoke drawLayers to render layers into the render area. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1577404d23..bdc57c904b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -130,6 +130,7 @@ #include "FrameTracer/FrameTracer.h" #include "FrontEnd/LayerCreationArgs.h" #include "FrontEnd/LayerHandle.h" +#include "FrontEnd/LayerSnapshot.h" #include "HdrLayerInfoReporter.h" #include "Layer.h" #include "LayerProtoHelper.h" @@ -2515,30 +2516,30 @@ void SurfaceFlinger::updateLayerGeometry() { mLayersPendingRefresh.clear(); } -bool SurfaceFlinger::isHdrLayer(Layer* layer) const { +bool SurfaceFlinger::isHdrLayer(const frontend::LayerSnapshot& snapshot) const { // Even though the camera layer may be using an HDR transfer function or otherwise be "HDR" // the device may need to avoid boosting the brightness as a result of these layers to // reduce power consumption during camera recording if (mIgnoreHdrCameraLayers) { - auto buffer = layer->getBuffer(); - if (buffer && (buffer->getUsage() & GRALLOC_USAGE_HW_CAMERA_WRITE) != 0) { + if (snapshot.externalTexture && + (snapshot.externalTexture->getUsage() & GRALLOC_USAGE_HW_CAMERA_WRITE) != 0) { return false; } } - if (isHdrDataspace(layer->getDataSpace())) { + if (isHdrDataspace(snapshot.dataspace)) { return true; } // If the layer is not allowed to be dimmed, treat it as HDR. WindowManager may disable // dimming in order to keep animations invoking SDR screenshots of HDR layers seamless. // Treat such tagged layers as HDR so that DisplayManagerService does not try to change // the screen brightness - if (!layer->isDimmingEnabled()) { + if (!snapshot.dimmingEnabled) { return true; } // RANGE_EXTENDED layers may identify themselves as being "HDR" via a desired sdr/hdr ratio - if ((layer->getDataSpace() & (int32_t)Dataspace::RANGE_MASK) == + if ((snapshot.dataspace & (int32_t)Dataspace::RANGE_MASK) == (int32_t)Dataspace::RANGE_EXTENDED && - layer->getDesiredSdrHdrRatio() > 1.01f) { + snapshot.desiredSdrHdrRatio > 1.01f) { return true; } return false; @@ -2666,13 +2667,14 @@ void SurfaceFlinger::postComposition(nsecs_t callTime) { int32_t maxArea = 0; mDrawingState.traverse([&, compositionDisplay = compositionDisplay](Layer* layer) { const auto layerFe = layer->getCompositionEngineLayerFE(); - if (layer->isVisible() && - compositionDisplay->includesLayer(layer->getOutputFilter())) { - if (isHdrLayer(layer)) { + const frontend::LayerSnapshot& snapshot = *layer->getLayerSnapshot(); + if (snapshot.isVisible && + compositionDisplay->includesLayer(snapshot.outputFilter)) { + if (isHdrLayer(snapshot)) { const auto* outputLayer = compositionDisplay->getOutputLayerForLayer(layerFe); if (outputLayer) { - info.mergeDesiredRatio(layer->getDesiredSdrHdrRatio()); + info.mergeDesiredRatio(snapshot.desiredSdrHdrRatio); info.numberOfHdrLayers++; const auto displayFrame = outputLayer->getState().displayFrame; const int32_t area = displayFrame.width() * displayFrame.height(); @@ -6418,8 +6420,9 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, auto traverseLayers = [this, args, layerStack](const LayerVector::Visitor& visitor) { traverseLayersInLayerStack(layerStack, args.uid, visitor); }; + auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); - auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, reqSize, + auto future = captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, args.grayscale, captureListener); return fenceStatus(future.get()); @@ -6454,6 +6457,7 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) { traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, visitor); }; + auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); if (captureListener == nullptr) { ALOGE("capture screen must provide a capture listener callback"); @@ -6463,7 +6467,7 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, constexpr bool kAllowProtected = false; constexpr bool kGrayscale = false; - auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, size, + auto future = captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, size, ui::PixelFormat::RGBA_8888, kAllowProtected, kGrayscale, captureListener); return fenceStatus(future.get()); @@ -6481,7 +6485,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, ui::Size reqSize; sp parent; Rect crop(args.sourceCrop); - std::unordered_set, SpHash> excludeLayers; + std::unordered_set excludeLayerIds; ui::Dataspace dataspace; // Call this before holding mStateLock to avoid any deadlocking. @@ -6521,9 +6525,9 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY); for (const auto& handle : args.excludeHandles) { - sp excludeLayer = LayerHandle::getLayer(handle); - if (excludeLayer != nullptr) { - excludeLayers.emplace(excludeLayer); + uint32_t excludeLayer = LayerHandle::getLayerId(handle); + if (excludeLayer != UNASSIGNED_LAYER_ID) { + excludeLayerIds.emplace(excludeLayer); } else { ALOGW("Invalid layer handle passed as excludeLayer to captureLayers"); return NAME_NOT_FOUND; @@ -6552,7 +6556,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, args.captureSecureLayers); }); - auto traverseLayers = [parent, args, excludeLayers](const LayerVector::Visitor& visitor) { + auto traverseLayers = [parent, args, excludeLayerIds](const LayerVector::Visitor& visitor) { parent->traverseChildrenInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) { if (!layer->isVisible()) { return; @@ -6564,7 +6568,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, auto p = sp::fromExisting(layer); while (p != nullptr) { - if (excludeLayers.count(p) != 0) { + if (excludeLayerIds.count(p->sequence) != 0) { return; } p = p->getParent(); @@ -6573,20 +6577,21 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, visitor(layer); }); }; + auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); if (captureListener == nullptr) { ALOGE("capture screen must provide a capture listener callback"); return BAD_VALUE; } - auto future = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, reqSize, + auto future = captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, reqSize, args.pixelFormat, args.allowProtected, args.grayscale, captureListener); return fenceStatus(future.get()); } ftl::SharedFuture SurfaceFlinger::captureScreenCommon( - RenderAreaFuture renderAreaFuture, TraverseLayersFunction traverseLayers, + RenderAreaFuture renderAreaFuture, GetLayerSnapshotsFunction getLayerSnapshots, ui::Size bufferSize, ui::PixelFormat reqPixelFormat, bool allowProtected, bool grayscale, const sp& captureListener) { ATRACE_CALL(); @@ -6605,15 +6610,18 @@ ftl::SharedFuture SurfaceFlinger::captureScreenCommon( const bool supportsProtected = getRenderEngine().supportsProtectedContent(); bool hasProtectedLayer = false; if (allowProtected && supportsProtected) { - auto future = mScheduler->schedule([=]() { - bool protectedLayerFound = false; - traverseLayers([&](Layer* layer) { - protectedLayerFound = - protectedLayerFound || (layer->isVisible() && layer->isProtected()); - }); - return protectedLayerFound; - }); - hasProtectedLayer = future.get(); + hasProtectedLayer = mScheduler + ->schedule([=]() { + bool protectedLayerFound = false; + auto layers = getLayerSnapshots(); + for (auto& [layer, layerFe] : layers) { + protectedLayerFound |= + (layerFe->mSnapshot->isVisible && + layerFe->mSnapshot->hasProtectedContent); + } + return protectedLayerFound; + }) + .get(); } const uint32_t usage = GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_RENDER | @@ -6638,12 +6646,12 @@ ftl::SharedFuture SurfaceFlinger::captureScreenCommon( renderengine::impl::ExternalTexture>(buffer, getRenderEngine(), renderengine::impl::ExternalTexture::Usage:: WRITEABLE); - return captureScreenCommon(std::move(renderAreaFuture), traverseLayers, texture, + return captureScreenCommon(std::move(renderAreaFuture), getLayerSnapshots, texture, false /* regionSampling */, grayscale, captureListener); } ftl::SharedFuture SurfaceFlinger::captureScreenCommon( - RenderAreaFuture renderAreaFuture, TraverseLayersFunction traverseLayers, + RenderAreaFuture renderAreaFuture, GetLayerSnapshotsFunction getLayerSnapshots, const std::shared_ptr& buffer, bool regionSampling, bool grayscale, const sp& captureListener) { ATRACE_CALL(); @@ -6666,7 +6674,7 @@ ftl::SharedFuture SurfaceFlinger::captureScreenCommon( ftl::SharedFuture renderFuture; renderArea->render([&]() FTL_FAKE_GUARD(kMainThreadContext) { - renderFuture = renderScreenImpl(renderArea, traverseLayers, buffer, + renderFuture = renderScreenImpl(renderArea, getLayerSnapshots, buffer, canCaptureBlackoutContent, regionSampling, grayscale, captureResults); }); @@ -6694,18 +6702,26 @@ ftl::SharedFuture SurfaceFlinger::captureScreenCommon( } ftl::SharedFuture SurfaceFlinger::renderScreenImpl( - std::shared_ptr renderArea, TraverseLayersFunction traverseLayers, + std::shared_ptr renderArea, GetLayerSnapshotsFunction getLayerSnapshots, const std::shared_ptr& buffer, bool canCaptureBlackoutContent, bool regionSampling, bool grayscale, ScreenCaptureResults& captureResults) { ATRACE_CALL(); - size_t layerCount = 0; - traverseLayers([&](Layer* layer) { - layerCount++; - captureResults.capturedSecureLayers = - captureResults.capturedSecureLayers || (layer->isVisible() && layer->isSecure()); - }); + 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(); + captureResults.capturedSecureLayers |= (snapshot->isVisible && snapshot->isSecure); + 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 // use in situations like the Screen-rotation animation and place @@ -6739,31 +6755,6 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( } captureResults.capturedDataspace = dataspace; - const auto transform = renderArea->getTransform(); - const auto display = renderArea->getDisplayDevice(); - - std::vector>> layers; - layers.reserve(layerCount); - std::unordered_set filterForScreenshot; - traverseLayers([&](Layer* layer) { - captureResults.capturedHdrLayers |= isHdrLayer(layer); - // Layer::prepareClientComposition uses the layer's snapshot to populate the resulting - // LayerSettings. Calling Layer::updateSnapshot ensures that LayerSettings are - // generated with the layer's current buffer and geometry. - layer->updateSnapshot(true /* updateGeometry */); - - layers.emplace_back(layer, layer->copyCompositionEngineLayerFE()); - - sp& layerFE = layers.back().second; - - layerFE->mSnapshot->geomLayerTransform = - renderArea->getTransform() * layerFE->mSnapshot->geomLayerTransform; - - if (layer->needsFilteringForScreenshots(display.get(), transform)) { - filterForScreenshot.insert(layerFE.get()); - } - }); - ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK}; if (!layers.empty()) { const sp& layerFE = layers.back().second; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 6f092ed995..207dfe2e96 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -69,6 +69,7 @@ #include "FlagManager.h" #include "FrontEnd/DisplayInfo.h" #include "FrontEnd/LayerCreationArgs.h" +#include "FrontEnd/LayerSnapshot.h" #include "FrontEnd/TransactionHandler.h" #include "LayerVector.h" #include "Scheduler/RefreshRateSelector.h" @@ -346,6 +347,7 @@ private: friend class LayerRenderArea; friend class LayerTracing; friend class SurfaceComposerAIDL; + friend class DisplayRenderArea; // For unit tests friend class TestableSurfaceFlinger; @@ -353,7 +355,7 @@ private: friend class TunnelModeEnabledReporterTest; using TransactionSchedule = scheduler::TransactionSchedule; - using TraverseLayersFunction = std::function; + using GetLayerSnapshotsFunction = std::function>>()>; using RenderAreaFuture = ftl::Future>; using DumpArgs = Vector; using Dumper = std::function; @@ -787,16 +789,16 @@ private: // Boot animation, on/off animations and screen capture void startBootAnim(); - ftl::SharedFuture captureScreenCommon(RenderAreaFuture, TraverseLayersFunction, + ftl::SharedFuture captureScreenCommon(RenderAreaFuture, GetLayerSnapshotsFunction, ui::Size bufferSize, ui::PixelFormat, bool allowProtected, bool grayscale, const sp&); ftl::SharedFuture captureScreenCommon( - RenderAreaFuture, TraverseLayersFunction, + RenderAreaFuture, GetLayerSnapshotsFunction, const std::shared_ptr&, bool regionSampling, bool grayscale, const sp&); ftl::SharedFuture renderScreenImpl( - std::shared_ptr, TraverseLayersFunction, + std::shared_ptr, GetLayerSnapshotsFunction, const std::shared_ptr&, bool canCaptureBlackoutContent, bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); @@ -1085,7 +1087,7 @@ private: void updateInternalDisplayVsyncLocked(const sp& activeDisplay) REQUIRES(mStateLock, kMainThreadContext); - bool isHdrLayer(Layer* layer) const; + bool isHdrLayer(const frontend::LayerSnapshot& snapshot) const; ui::Rotation getPhysicalDisplayOrientation(DisplayId, bool isPrimary) const REQUIRES(mStateLock); diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index ba77600cda..0416e93f1e 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -238,6 +238,8 @@ void CompositionTest::captureScreenComposition() { CaptureArgs::UNSET_UID, visitor); }; + auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); + const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE; mCaptureScreenBuffer = @@ -246,7 +248,7 @@ void CompositionTest::captureScreenComposition() { HAL_PIXEL_FORMAT_RGBA_8888, 1, usage); - auto future = mFlinger.renderScreenImpl(std::move(renderArea), traverseLayers, + auto future = mFlinger.renderScreenImpl(std::move(renderArea), getLayerSnapshots, mCaptureScreenBuffer, forSystem, regionSampling); ASSERT_TRUE(future.valid()); const auto fenceResult = future.get(); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 51d20120cd..68c738fc9d 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -407,7 +407,7 @@ public: } auto renderScreenImpl(std::shared_ptr renderArea, - SurfaceFlinger::TraverseLayersFunction traverseLayers, + SurfaceFlinger::GetLayerSnapshotsFunction traverseLayers, const std::shared_ptr& buffer, bool forSystem, bool regionSampling) { ScreenCaptureResults captureResults; -- cgit v1.2.3-59-g8ed1b From c5a5b6e0b3b8a14fb14e31985e6f533dd9f8edf7 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Tue, 28 Feb 2023 00:19:07 +0000 Subject: [sf] support region sampling with new frontend Use the new screenshot function but add support for a layer filter used by region sampling code. Test: check gesture nav bar color Test: atest libgui_test Bug: 238781169 Change-Id: I1c9370fd59b290fd1451d5c77cd27c275c685090 --- services/surfaceflinger/RegionSamplingThread.cpp | 97 +++++++++++++++--------- services/surfaceflinger/SurfaceFlinger.cpp | 22 ++++-- services/surfaceflinger/SurfaceFlinger.h | 4 +- 3 files changed, 81 insertions(+), 42 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 839500f8a0..327ca3f0aa 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -285,47 +285,73 @@ void RegionSamplingThread::captureSample() { std::unordered_set, SpHash> listeners; - auto traverseLayers = [&](const LayerVector::Visitor& visitor) { - bool stopLayerFound = false; - auto filterVisitor = [&](Layer* layer) { - // We don't want to capture any layers beyond the stop layer - if (stopLayerFound) return; - - // Likewise if we just found a stop layer, set the flag and abort - for (const auto& [area, stopLayerId, listener] : descriptors) { - if (stopLayerId != UNASSIGNED_LAYER_ID && layer->getSequence() == stopLayerId) { - stopLayerFound = true; - return; - } + auto layerFilterFn = [&](const char* layerName, uint32_t layerId, const Rect& bounds, + const ui::Transform transform, bool& outStopTraversal) -> bool { + // Likewise if we just found a stop layer, set the flag and abort + for (const auto& [area, stopLayerId, listener] : descriptors) { + if (stopLayerId != UNASSIGNED_LAYER_ID && layerId == stopLayerId) { + outStopTraversal = true; + return false; } + } - // Compute the layer's position on the screen - const Rect bounds = Rect(layer->getBounds()); - const ui::Transform transform = layer->getTransform(); - constexpr bool roundOutwards = true; - Rect transformed = transform.transform(bounds, roundOutwards); - - // If this layer doesn't intersect with the larger sampledBounds, skip capturing it - Rect ignore; - if (!transformed.intersect(sampledBounds, &ignore)) return; - - // If the layer doesn't intersect a sampling area, skip capturing it - bool intersectsAnyArea = false; - for (const auto& [area, stopLayer, listener] : descriptors) { - if (transformed.intersect(area, &ignore)) { - intersectsAnyArea = true; - listeners.insert(listener); - } + // Compute the layer's position on the screen + constexpr bool roundOutwards = true; + Rect transformed = transform.transform(bounds, roundOutwards); + + // If this layer doesn't intersect with the larger sampledBounds, skip capturing it + Rect ignore; + if (!transformed.intersect(sampledBounds, &ignore)) return false; + + // If the layer doesn't intersect a sampling area, skip capturing it + bool intersectsAnyArea = false; + for (const auto& [area, stopLayer, listener] : descriptors) { + if (transformed.intersect(area, &ignore)) { + intersectsAnyArea = true; + listeners.insert(listener); } - if (!intersectsAnyArea) return; + } + if (!intersectsAnyArea) return false; - ALOGV("Traversing [%s] [%d, %d, %d, %d]", layer->getDebugName(), bounds.left, - bounds.top, bounds.right, bounds.bottom); - visitor(layer); - }; - mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, filterVisitor); + ALOGV("Traversing [%s] [%d, %d, %d, %d]", layerName, bounds.left, bounds.top, bounds.right, + bounds.bottom); + + return true; }; + std::function>>()> getLayerSnapshots; + if (mFlinger.mLayerLifecycleManagerEnabled) { + auto filterFn = [&](const frontend::LayerSnapshot& snapshot, + bool& outStopTraversal) -> bool { + const Rect bounds = + frontend::RequestedLayerState::reduce(Rect(snapshot.geomLayerBounds), + snapshot.transparentRegionHint); + const ui::Transform transform = snapshot.geomLayerTransform; + return layerFilterFn(snapshot.name.c_str(), snapshot.path.id, bounds, transform, + outStopTraversal); + }; + getLayerSnapshots = + mFlinger.getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, + filterFn); + } else { + auto traverseLayers = [&](const LayerVector::Visitor& visitor) { + bool stopLayerFound = false; + auto filterVisitor = [&](Layer* layer) { + // We don't want to capture any layers beyond the stop layer + if (stopLayerFound) return; + + if (!layerFilterFn(layer->getDebugName(), layer->getSequence(), + Rect(layer->getBounds()), layer->getTransform(), + stopLayerFound)) { + return; + } + visitor(layer); + }; + mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, filterVisitor); + }; + getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); + } + std::shared_ptr buffer = nullptr; if (mCachedBuffer && mCachedBuffer->getBuffer()->getWidth() == sampledBounds.getWidth() && mCachedBuffer->getBuffer()->getHeight() == sampledBounds.getHeight()) { @@ -344,7 +370,6 @@ void RegionSamplingThread::captureSample() { renderengine::impl::ExternalTexture::Usage:: WRITEABLE); } - auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); constexpr bool kRegionSampling = true; constexpr bool kGrayscale = false; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 3882d0c106..b5b7b1e62e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6782,7 +6782,8 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { - getLayerSnapshots = getLayerSnapshotsForScreenshots(layerStack, args.uid); + getLayerSnapshots = + getLayerSnapshotsForScreenshots(layerStack, args.uid, /*snapshotFilterFn=*/nullptr); } else { auto traverseLayers = [this, args, layerStack](const LayerVector::Visitor& visitor) { traverseLayersInLayerStack(layerStack, args.uid, visitor); @@ -6824,7 +6825,8 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { - getLayerSnapshots = getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID); + getLayerSnapshots = getLayerSnapshotsForScreenshots(layerStack, CaptureArgs::UNSET_UID, + /*snapshotFilterFn=*/nullptr); } else { auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) { traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, visitor); @@ -7852,12 +7854,18 @@ std::vector> SurfaceFlinger::moveSnapshotsToComposit } std::function>>()> -SurfaceFlinger::getLayerSnapshotsForScreenshots(std::optional layerStack, - uint32_t uid) { +SurfaceFlinger::getLayerSnapshotsForScreenshots( + std::optional layerStack, uint32_t uid, + std::function + snapshotFilterFn) { return [&, layerStack, uid]() { std::vector>> layers; + bool stopTraversal = false; mLayerSnapshotBuilder.forEachVisibleSnapshot( [&](std::unique_ptr& snapshot) { + if (stopTraversal) { + return; + } if (layerStack && snapshot->outputFilter.layerStack != *layerStack) { return; } @@ -7867,6 +7875,9 @@ SurfaceFlinger::getLayerSnapshotsForScreenshots(std::optional la if (!snapshot->hasSomethingToDraw()) { return; } + if (snapshotFilterFn && !snapshotFilterFn(*snapshot, stopTraversal)) { + return; + } auto it = mLegacyLayers.find(snapshot->sequence); LOG_ALWAYS_FATAL_IF(it == mLegacyLayers.end(), @@ -7905,7 +7916,8 @@ SurfaceFlinger::getLayerSnapshotsForScreenshots(uint32_t rootLayerId, uint32_t u .genericLayerMetadataKeyMap = getGenericLayerMetadataKeyMap()}; mLayerSnapshotBuilder.update(args); - auto getLayerSnapshotsFn = getLayerSnapshotsForScreenshots({}, uid); + auto getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots({}, uid, /*snapshotFilterFn=*/nullptr); std::vector>> layers = getLayerSnapshotsFn(); args.root = mLayerHierarchyBuilder.getHierarchy(); args.parentCrop.reset(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index adba9b86f9..ddb62325c5 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1387,7 +1387,9 @@ private: [](const auto& display) { return display.isRefreshRateOverlayEnabled(); }); } std::function>>()> getLayerSnapshotsForScreenshots( - std::optional layerStack, uint32_t uid); + std::optional layerStack, uint32_t uid, + std::function + snapshotFilterFn); std::function>>()> getLayerSnapshotsForScreenshots( uint32_t rootLayerId, uint32_t uid, std::unordered_set excludeLayerIds, bool childrenOnly, const FloatRect& parentCrop); -- cgit v1.2.3-59-g8ed1b From 0284463f01bad248b39fce18b44a62551c91a070 Mon Sep 17 00:00:00 2001 From: Ajinkya Chalke Date: Wed, 1 Mar 2023 12:10:14 +0000 Subject: Move exclude layer to CaptureArgs. Test: atest ScreenCaptureTest ScreenCaptureChildOnlyTest Bug: 267324693 Change-Id: I6ab421e2f9e5bc0ab1422d88ddf2628776347a6f --- libs/gui/LayerState.cpp | 27 +++++---- libs/gui/include/gui/DisplayCaptureArgs.h | 4 ++ libs/gui/include/gui/LayerCaptureArgs.h | 3 - services/surfaceflinger/RegionSamplingThread.cpp | 3 +- services/surfaceflinger/SurfaceFlinger.cpp | 70 ++++++++++++++++++++-- services/surfaceflinger/SurfaceFlinger.h | 7 ++- .../surfaceflinger/tests/ScreenCapture_test.cpp | 8 +++ .../tests/unittests/CompositionTest.cpp | 2 +- .../tests/unittests/TestableSurfaceFlinger.h | 4 +- 9 files changed, 104 insertions(+), 24 deletions(-) (limited to 'services/surfaceflinger/RegionSamplingThread.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index b391337d1d..cf171dcec9 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -897,6 +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); + } return NO_ERROR; } @@ -913,6 +918,15 @@ 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); + for (int i = 0; i < numExcludeHandles; i++) { + sp binder; + SAFE_PARCEL(input->readStrongBinder, &binder); + excludeHandles.emplace(binder); + } return NO_ERROR; } @@ -940,10 +954,6 @@ status_t LayerCaptureArgs::writeToParcel(Parcel* output) const { SAFE_PARCEL(CaptureArgs::writeToParcel, output); SAFE_PARCEL(output->writeStrongBinder, layerHandle); - SAFE_PARCEL(output->writeInt32, excludeHandles.size()); - for (auto el : excludeHandles) { - SAFE_PARCEL(output->writeStrongBinder, el); - } SAFE_PARCEL(output->writeBool, childrenOnly); return NO_ERROR; } @@ -953,15 +963,6 @@ status_t LayerCaptureArgs::readFromParcel(const Parcel* input) { SAFE_PARCEL(input->readStrongBinder, &layerHandle); - int32_t numExcludeHandles = 0; - SAFE_PARCEL_READ_SIZE(input->readInt32, &numExcludeHandles, input->dataSize()); - excludeHandles.reserve(numExcludeHandles); - for (int i = 0; i < numExcludeHandles; i++) { - sp binder; - SAFE_PARCEL(input->readStrongBinder, &binder); - excludeHandles.emplace(binder); - } - SAFE_PARCEL(input->readBool, &childrenOnly); return NO_ERROR; } diff --git a/libs/gui/include/gui/DisplayCaptureArgs.h b/libs/gui/include/gui/DisplayCaptureArgs.h index c826c17d2c..5c794aea37 100644 --- a/libs/gui/include/gui/DisplayCaptureArgs.h +++ b/libs/gui/include/gui/DisplayCaptureArgs.h @@ -22,9 +22,11 @@ #include #include #include +#include #include #include #include +#include namespace android::gui { @@ -55,6 +57,8 @@ struct CaptureArgs : public Parcelable { bool grayscale = false; + std::unordered_set, SpHash> excludeHandles; + virtual status_t writeToParcel(Parcel* output) const; virtual status_t readFromParcel(const Parcel* input); }; diff --git a/libs/gui/include/gui/LayerCaptureArgs.h b/libs/gui/include/gui/LayerCaptureArgs.h index 05ff9d5b7b..fae2bcc787 100644 --- a/libs/gui/include/gui/LayerCaptureArgs.h +++ b/libs/gui/include/gui/LayerCaptureArgs.h @@ -20,14 +20,11 @@ #include #include -#include -#include namespace android::gui { struct LayerCaptureArgs : CaptureArgs { sp layerHandle; - std::unordered_set, SpHash> excludeHandles; bool childrenOnly{false}; status_t writeToParcel(Parcel* output) const override; diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 327ca3f0aa..531d277ffc 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -347,7 +347,8 @@ void RegionSamplingThread::captureSample() { } visitor(layer); }; - mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, filterVisitor); + mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, {}, + filterVisitor); }; getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 076c6839c6..791b42512c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6888,6 +6888,7 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, wp displayWeak; ui::LayerStack layerStack; ui::Size reqSize(args.width, args.height); + std::unordered_set excludeLayerIds; ui::Dataspace dataspace; { Mutex::Autolock lock(mStateLock); @@ -6901,6 +6902,16 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, reqSize = display->getLayerStackSpaceRect().getSize(); } + for (const auto& handle : args.excludeHandles) { + uint32_t excludeLayer = LayerHandle::getLayerId(handle); + if (excludeLayer != UNASSIGNED_LAYER_ID) { + excludeLayerIds.emplace(excludeLayer); + } else { + ALOGW("Invalid layer handle passed as excludeLayer to captureDisplay"); + 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 @@ -6916,10 +6927,11 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { getLayerSnapshots = - getLayerSnapshotsForScreenshots(layerStack, args.uid, /*snapshotFilterFn=*/nullptr); + getLayerSnapshotsForScreenshots(layerStack, args.uid, std::move(excludeLayerIds)); } else { - auto traverseLayers = [this, args, layerStack](const LayerVector::Visitor& visitor) { - traverseLayersInLayerStack(layerStack, args.uid, visitor); + auto traverseLayers = [this, args, excludeLayerIds, + layerStack](const LayerVector::Visitor& visitor) { + traverseLayersInLayerStack(layerStack, args.uid, std::move(excludeLayerIds), visitor); }; getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); } @@ -6962,7 +6974,7 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, /*snapshotFilterFn=*/nullptr); } else { auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) { - traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, visitor); + traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, {}, visitor); }; getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); } @@ -7386,6 +7398,7 @@ void SurfaceFlinger::State::traverseInReverseZOrder(const LayerVector::Visitor& } void SurfaceFlinger::traverseLayersInLayerStack(ui::LayerStack layerStack, const int32_t uid, + std::unordered_set excludeLayerIds, const LayerVector::Visitor& visitor) { // We loop through the first level of layers without traversing, // as we need to determine which layers belong to the requested display. @@ -7404,6 +7417,17 @@ void SurfaceFlinger::traverseLayersInLayerStack(ui::LayerStack layerStack, const if (uid != CaptureArgs::UNSET_UID && layer->getOwnerUid() != uid) { return; } + + if (!excludeLayerIds.empty()) { + auto p = sp::fromExisting(layer); + while (p != nullptr) { + if (excludeLayerIds.count(p->sequence) != 0) { + return; + } + p = p->getParent(); + } + } + visitor(layer); }); } @@ -8058,6 +8082,44 @@ SurfaceFlinger::getLayerSnapshotsForScreenshots( }; } +std::function>>()> +SurfaceFlinger::getLayerSnapshotsForScreenshots(std::optional layerStack, + uint32_t uid, + std::unordered_set excludeLayerIds) { + return [&, layerStack, uid, excludeLayerIds = std::move(excludeLayerIds)]() { + if (excludeLayerIds.empty()) { + auto getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots(layerStack, uid, /*snapshotFilterFn=*/nullptr); + std::vector>> layers = getLayerSnapshotsFn(); + return layers; + } + + frontend::LayerSnapshotBuilder::Args + args{.root = mLayerHierarchyBuilder.getHierarchy(), + .layerLifecycleManager = mLayerLifecycleManager, + .forceUpdate = frontend::LayerSnapshotBuilder::ForceUpdateFlags::HIERARCHY, + .displays = mFrontEndDisplayInfos, + .displayChanges = true, + .globalShadowSettings = mDrawingState.globalShadowSettings, + .supportsBlur = mSupportsBlur, + .forceFullDamage = mForceFullDamage, + .excludeLayerIds = std::move(excludeLayerIds), + .supportedLayerGenericMetadata = + getHwComposer().getSupportedLayerGenericMetadata(), + .genericLayerMetadataKeyMap = getGenericLayerMetadataKeyMap()}; + mLayerSnapshotBuilder.update(args); + + auto getLayerSnapshotsFn = + getLayerSnapshotsForScreenshots(layerStack, uid, /*snapshotFilterFn=*/nullptr); + std::vector>> layers = getLayerSnapshotsFn(); + + args.excludeLayerIds.clear(); + mLayerSnapshotBuilder.update(args); + + return layers; + }; +} + std::function>>()> SurfaceFlinger::getLayerSnapshotsForScreenshots(uint32_t rootLayerId, uint32_t uid, std::unordered_set excludeLayerIds, diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 74d00dd60c..44847885ca 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -826,7 +826,9 @@ private: // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a // matching ownerUid - void traverseLayersInLayerStack(ui::LayerStack, const int32_t uid, const LayerVector::Visitor&); + void traverseLayersInLayerStack(ui::LayerStack, const int32_t uid, + std::unordered_set excludeLayerIds, + const LayerVector::Visitor&); void readPersistentProperties(); @@ -1380,6 +1382,9 @@ private: std::optional layerStack, uint32_t uid, std::function snapshotFilterFn); + std::function>>()> getLayerSnapshotsForScreenshots( + std::optional layerStack, uint32_t uid, + std::unordered_set excludeLayerIds); std::function>>()> getLayerSnapshotsForScreenshots( uint32_t rootLayerId, uint32_t uid, std::unordered_set excludeLayerIds, bool childrenOnly, const std::optional& optionalParentCrop); diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp index 976ee3519c..013694fd47 100644 --- a/services/surfaceflinger/tests/ScreenCapture_test.cpp +++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp @@ -222,6 +222,14 @@ TEST_F(ScreenCaptureTest, CaptureLayerExclude) { mCapture->checkPixel(0, 0, 200, 200, 200); } +TEST_F(ScreenCaptureTest, CaptureLayerExcludeThroughDisplayArgs) { + mCaptureArgs.excludeHandles = {mFGSurfaceControl->getHandle()}; + ScreenCapture::captureDisplay(&mCapture, mCaptureArgs); + mCapture->expectBGColor(0, 0); + // Doesn't capture FG layer which is at 64, 64 + mCapture->expectBGColor(64, 64); +} + // Like the last test but verifies that children are also exclude. TEST_F(ScreenCaptureTest, CaptureLayerExcludeTree) { auto fgHandle = mFGSurfaceControl->getHandle(); diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 19a93e1820..f5960614b0 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -204,7 +204,7 @@ void CompositionTest::captureScreenComposition() { auto traverseLayers = [this](const LayerVector::Visitor& visitor) { return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(), - CaptureArgs::UNSET_UID, visitor); + CaptureArgs::UNSET_UID, {}, visitor); }; auto getLayerSnapshots = RenderArea::fromTraverseLayersLambda(traverseLayers); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index fc9e653ba4..89a190e9ea 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -446,8 +446,10 @@ public: } auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid, + std::unordered_set excludeLayerIds, const LayerVector::Visitor& visitor) { - return mFlinger->SurfaceFlinger::traverseLayersInLayerStack(layerStack, uid, visitor); + return mFlinger->SurfaceFlinger::traverseLayersInLayerStack(layerStack, uid, + excludeLayerIds, visitor); } auto getDisplayNativePrimaries(const sp& displayToken, -- 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/RegionSamplingThread.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