diff options
| author | 2023-03-18 01:54:43 +0000 | |
|---|---|---|
| committer | 2023-03-20 21:01:21 +0000 | |
| commit | fed7c12150bcef08ddfd5e77e38a4fdf6a9c971a (patch) | |
| tree | 37a7aa83fe7cab780e7d2efdc03dfb71cb5e984f | |
| parent | 2c1f96e0d825463dc1e420982d31d77bd442d9e7 (diff) | |
Take into account surface insets when replacing touch region.
When SurfaceFlinger computes window frames, the surface insets are
clamped to the input bounds. But when replacing the touchable region,
the surface insets are not taken into account.
This CL adds consideration for surface insets, so that touchable region
is computed correctly. This also fixes a ListPopupWindowTest on freeform
windowing devices.
Test: atest SurfaceFlinger_test on freeform windowing emulator and
aosp_cf_x86_64_phone target
Fixes: 272786078
Change-Id: Id8082cb1b39280e846c2bf5a36315df68d37d2e6
| -rw-r--r-- | libs/gui/tests/EndToEndNativeInputTest.cpp | 16 | ||||
| -rw-r--r-- | services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp | 119 | ||||
| -rw-r--r-- | services/surfaceflinger/Layer.cpp | 139 | ||||
| -rw-r--r-- | services/surfaceflinger/Layer.h | 4 |
4 files changed, 168 insertions, 110 deletions
diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index 3344e0b690..5f80c16f61 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -512,6 +512,22 @@ TEST_F(InputSurfacesTest, input_respects_surface_insets) { bgSurface->expectTap(1, 1); } +TEST_F(InputSurfacesTest, input_respects_surface_insets_with_replaceTouchableRegionWithCrop) { + std::unique_ptr<InputSurface> bgSurface = makeSurface(100, 100); + std::unique_ptr<InputSurface> fgSurface = makeSurface(100, 100); + bgSurface->showAt(100, 100); + + fgSurface->mInputInfo.surfaceInset = 5; + fgSurface->mInputInfo.replaceTouchableRegionWithCrop = true; + fgSurface->showAt(100, 100); + + injectTap(106, 106); + fgSurface->expectTap(1, 1); + + injectTap(101, 101); + bgSurface->expectTap(1, 1); +} + // Ensure a surface whose insets are cropped, handles the touch offset correctly. ref:b/120413463 TEST_F(InputSurfacesTest, input_respects_cropped_surface_insets) { std::unique_ptr<InputSurface> parentSurface = makeSurface(100, 100); diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index a16de1bcfb..2fc9682849 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -21,6 +21,7 @@ #include "LayerSnapshotBuilder.h" #include <gui/TraceUtils.h> +#include <ui/FloatRect.h> #include <numeric> #include "DisplayHardware/HWC2.h" #include "DisplayHardware/Hal.h" @@ -101,43 +102,52 @@ ui::Transform getInputTransform(const LayerSnapshot& snapshot) { } /** + * Returns the bounds used to fill the input frame and the touchable region. + * * Similar to getInputTransform, we need to update the bounds to include the transform. * This is because bounds don't include the buffer transform, where the input assumes * that's already included. */ -Rect getInputBounds(const LayerSnapshot& snapshot) { - if (!snapshot.hasBufferOrSidebandStream()) { - return snapshot.croppedBufferSize; +std::pair<FloatRect, bool> getInputBounds(const LayerSnapshot& snapshot, bool fillParentBounds) { + FloatRect inputBounds = snapshot.croppedBufferSize.toFloatRect(); + if (snapshot.hasBufferOrSidebandStream() && snapshot.croppedBufferSize.isValid() && + snapshot.localTransform.getType() != ui::Transform::IDENTITY) { + inputBounds = snapshot.localTransform.transform(inputBounds); + } + + bool inputBoundsValid = snapshot.croppedBufferSize.isValid(); + if (!inputBoundsValid) { + /** + * Input bounds are based on the layer crop or buffer size. But if we are using + * the layer bounds as the input bounds (replaceTouchableRegionWithCrop flag) then + * we can use the parent bounds as the input bounds if the layer does not have buffer + * or a crop. We want to unify this logic but because of compat reasons we cannot always + * use the parent bounds. A layer without a buffer can get input. So when a window is + * initially added, its touchable region can fill its parent layer bounds and that can + * have negative consequences. + */ + inputBounds = fillParentBounds ? snapshot.geomLayerBounds : FloatRect{}; } - if (snapshot.localTransform.getType() == ui::Transform::IDENTITY || - !snapshot.croppedBufferSize.isValid()) { - return snapshot.croppedBufferSize; - } - return snapshot.localTransform.transform(snapshot.croppedBufferSize); -} + // Clamp surface inset to the input bounds. + const float inset = static_cast<float>(snapshot.inputInfo.surfaceInset); + const float xSurfaceInset = std::clamp(inset, 0.f, inputBounds.getWidth() / 2.f); + const float ySurfaceInset = std::clamp(inset, 0.f, inputBounds.getHeight() / 2.f); -void fillInputFrameInfo(gui::WindowInfo& info, const ui::Transform& screenToDisplay, - const LayerSnapshot& snapshot) { - Rect tmpBounds = getInputBounds(snapshot); - if (!tmpBounds.isValid()) { - info.touchableRegion.clear(); - // A layer could have invalid input bounds and still expect to receive touch input if it has - // replaceTouchableRegionWithCrop. For that case, the input transform needs to be calculated - // correctly to determine the coordinate space for input events. Use an empty rect so that - // the layer will receive input in its own layer space. - tmpBounds = Rect::EMPTY_RECT; - } + // Apply the insets to the input bounds. + inputBounds.left += xSurfaceInset; + inputBounds.top += ySurfaceInset; + inputBounds.right -= xSurfaceInset; + inputBounds.bottom -= ySurfaceInset; + return {inputBounds, inputBoundsValid}; +} +Rect getInputBoundsInDisplaySpace(const LayerSnapshot& snapshot, const FloatRect& insetBounds, + const ui::Transform& screenToDisplay) { // InputDispatcher works in the display device's coordinate space. Here, we calculate the // frame and transform used for the layer, which determines the bounds and the coordinate space // within which the layer will receive input. - // - // The coordinate space within which each of the bounds are specified is explicitly documented - // in the variable name. For example "inputBoundsInLayer" is specified in layer space. A - // Transform converts one coordinate space to another, which is apparent in its naming. For - // example, "layerToDisplay" transforms layer space to display space. - // + // Coordinate space definitions: // - display: The display device's coordinate space. Correlates to pixels on the display. // - screen: The post-rotation coordinate space for the display, a.k.a. logical display space. @@ -145,37 +155,34 @@ void fillInputFrameInfo(gui::WindowInfo& info, const ui::Transform& screenToDisp // - input: The coordinate space in which this layer will receive input events. This could be // different than layer space if a surfaceInset is used, which changes the origin // of the input space. - const FloatRect inputBoundsInLayer = tmpBounds.toFloatRect(); - - // Clamp surface inset to the input bounds. - const auto surfaceInset = static_cast<float>(info.surfaceInset); - const float xSurfaceInset = - std::max(0.f, std::min(surfaceInset, inputBoundsInLayer.getWidth() / 2.f)); - const float ySurfaceInset = - std::max(0.f, std::min(surfaceInset, inputBoundsInLayer.getHeight() / 2.f)); - - // Apply the insets to the input bounds. - const FloatRect insetBoundsInLayer(inputBoundsInLayer.left + xSurfaceInset, - inputBoundsInLayer.top + ySurfaceInset, - inputBoundsInLayer.right - xSurfaceInset, - inputBoundsInLayer.bottom - ySurfaceInset); // Crop the input bounds to ensure it is within the parent's bounds. - const FloatRect croppedInsetBoundsInLayer = - snapshot.geomLayerBounds.intersect(insetBoundsInLayer); + const FloatRect croppedInsetBoundsInLayer = snapshot.geomLayerBounds.intersect(insetBounds); const ui::Transform layerToScreen = getInputTransform(snapshot); const ui::Transform layerToDisplay = screenToDisplay * layerToScreen; - const Rect roundedFrameInDisplay{layerToDisplay.transform(croppedInsetBoundsInLayer)}; + return Rect{layerToDisplay.transform(croppedInsetBoundsInLayer)}; +} + +void fillInputFrameInfo(gui::WindowInfo& info, const ui::Transform& screenToDisplay, + const LayerSnapshot& snapshot) { + auto [inputBounds, inputBoundsValid] = getInputBounds(snapshot, /*fillParentBounds=*/false); + if (!inputBoundsValid) { + info.touchableRegion.clear(); + } + + const Rect roundedFrameInDisplay = + getInputBoundsInDisplaySpace(snapshot, inputBounds, screenToDisplay); info.frameLeft = roundedFrameInDisplay.left; info.frameTop = roundedFrameInDisplay.top; info.frameRight = roundedFrameInDisplay.right; info.frameBottom = roundedFrameInDisplay.bottom; ui::Transform inputToLayer; - inputToLayer.set(insetBoundsInLayer.left, insetBoundsInLayer.top); - const ui::Transform inputToDisplay = layerToDisplay * inputToLayer; + inputToLayer.set(inputBounds.left, inputBounds.top); + const ui::Transform layerToScreen = getInputTransform(snapshot); + const ui::Transform inputToDisplay = screenToDisplay * layerToScreen * inputToLayer; // InputDispatcher expects a display-to-input transform. info.transform = inputToDisplay.inverse(); @@ -1008,12 +1015,26 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot, auto cropLayerSnapshot = getSnapshot(requested.touchCropId); if (snapshot.inputInfo.replaceTouchableRegionWithCrop) { - const Rect bounds(cropLayerSnapshot ? cropLayerSnapshot->transformedBounds - : snapshot.transformedBounds); - snapshot.inputInfo.touchableRegion = Region(displayInfo.transform.transform(bounds)); + Rect inputBoundsInDisplaySpace; + if (!cropLayerSnapshot) { + FloatRect inputBounds = getInputBounds(snapshot, /*fillParentBounds=*/true).first; + inputBoundsInDisplaySpace = + getInputBoundsInDisplaySpace(snapshot, inputBounds, displayInfo.transform); + } else { + FloatRect inputBounds = + getInputBounds(*cropLayerSnapshot, /*fillParentBounds=*/true).first; + inputBoundsInDisplaySpace = + getInputBoundsInDisplaySpace(*cropLayerSnapshot, inputBounds, + displayInfo.transform); + } + snapshot.inputInfo.touchableRegion = Region(inputBoundsInDisplaySpace); } else if (cropLayerSnapshot) { + FloatRect inputBounds = getInputBounds(*cropLayerSnapshot, /*fillParentBounds=*/true).first; + Rect inputBoundsInDisplaySpace = + getInputBoundsInDisplaySpace(*cropLayerSnapshot, inputBounds, + displayInfo.transform); snapshot.inputInfo.touchableRegion = snapshot.inputInfo.touchableRegion.intersect( - displayInfo.transform.transform(Rect{cropLayerSnapshot->transformedBounds})); + displayInfo.transform.transform(inputBoundsInDisplaySpace)); } // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0f2af2f809..3b41458475 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -52,8 +52,11 @@ #include <system/graphics-base-v1.0.h> #include <ui/DataspaceUtils.h> #include <ui/DebugUtils.h> +#include <ui/FloatRect.h> #include <ui/GraphicBuffer.h> #include <ui/PixelFormat.h> +#include <ui/Rect.h> +#include <ui/Transform.h> #include <utils/Errors.h> #include <utils/Log.h> #include <utils/NativeHandle.h> @@ -2297,62 +2300,21 @@ static Region transformTouchableRegionSafely(const ui::Transform& t, const Regio } void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& screenToDisplay) { - Rect tmpBounds = getInputBounds(); - if (!tmpBounds.isValid()) { + auto [inputBounds, inputBoundsValid] = getInputBounds(/*fillParentBounds=*/false); + if (!inputBoundsValid) { info.touchableRegion.clear(); - // A layer could have invalid input bounds and still expect to receive touch input if it has - // replaceTouchableRegionWithCrop. For that case, the input transform needs to be calculated - // correctly to determine the coordinate space for input events. Use an empty rect so that - // the layer will receive input in its own layer space. - tmpBounds = Rect::EMPTY_RECT; } - // InputDispatcher works in the display device's coordinate space. Here, we calculate the - // frame and transform used for the layer, which determines the bounds and the coordinate space - // within which the layer will receive input. - // - // The coordinate space within which each of the bounds are specified is explicitly documented - // in the variable name. For example "inputBoundsInLayer" is specified in layer space. A - // Transform converts one coordinate space to another, which is apparent in its naming. For - // example, "layerToDisplay" transforms layer space to display space. - // - // Coordinate space definitions: - // - display: The display device's coordinate space. Correlates to pixels on the display. - // - screen: The post-rotation coordinate space for the display, a.k.a. logical display space. - // - layer: The coordinate space of this layer. - // - input: The coordinate space in which this layer will receive input events. This could be - // different than layer space if a surfaceInset is used, which changes the origin - // of the input space. - const FloatRect inputBoundsInLayer = tmpBounds.toFloatRect(); - - // Clamp surface inset to the input bounds. - const auto surfaceInset = static_cast<float>(info.surfaceInset); - const float xSurfaceInset = - std::max(0.f, std::min(surfaceInset, inputBoundsInLayer.getWidth() / 2.f)); - const float ySurfaceInset = - std::max(0.f, std::min(surfaceInset, inputBoundsInLayer.getHeight() / 2.f)); - - // Apply the insets to the input bounds. - const FloatRect insetBoundsInLayer(inputBoundsInLayer.left + xSurfaceInset, - inputBoundsInLayer.top + ySurfaceInset, - inputBoundsInLayer.right - xSurfaceInset, - inputBoundsInLayer.bottom - ySurfaceInset); - - // Crop the input bounds to ensure it is within the parent's bounds. - const FloatRect croppedInsetBoundsInLayer = mBounds.intersect(insetBoundsInLayer); - - const ui::Transform layerToScreen = getInputTransform(); - const ui::Transform layerToDisplay = screenToDisplay * layerToScreen; - - const Rect roundedFrameInDisplay{layerToDisplay.transform(croppedInsetBoundsInLayer)}; + const Rect roundedFrameInDisplay = getInputBoundsInDisplaySpace(inputBounds, screenToDisplay); info.frameLeft = roundedFrameInDisplay.left; info.frameTop = roundedFrameInDisplay.top; info.frameRight = roundedFrameInDisplay.right; info.frameBottom = roundedFrameInDisplay.bottom; ui::Transform inputToLayer; - inputToLayer.set(insetBoundsInLayer.left, insetBoundsInLayer.top); - const ui::Transform inputToDisplay = layerToDisplay * inputToLayer; + inputToLayer.set(inputBounds.left, inputBounds.top); + const ui::Transform layerToScreen = getInputTransform(); + const ui::Transform inputToDisplay = screenToDisplay * layerToScreen * inputToLayer; // InputDispatcher expects a display-to-input transform. info.transform = inputToDisplay.inverse(); @@ -2485,13 +2447,23 @@ WindowInfo Layer::fillInputInfo(const InputDisplayArgs& displayArgs) { info.inputConfig |= WindowInfo::InputConfig::DROP_INPUT; } - auto cropLayer = mDrawingState.touchableRegionCrop.promote(); + sp<Layer> cropLayer = mDrawingState.touchableRegionCrop.promote(); if (info.replaceTouchableRegionWithCrop) { - const Rect bounds(cropLayer ? cropLayer->mScreenBounds : mScreenBounds); - info.touchableRegion = Region(displayTransform.transform(bounds)); + Rect inputBoundsInDisplaySpace; + if (!cropLayer) { + FloatRect inputBounds = getInputBounds(/*fillParentBounds=*/true).first; + inputBoundsInDisplaySpace = getInputBoundsInDisplaySpace(inputBounds, displayTransform); + } else { + FloatRect inputBounds = cropLayer->getInputBounds(/*fillParentBounds=*/true).first; + inputBoundsInDisplaySpace = + cropLayer->getInputBoundsInDisplaySpace(inputBounds, displayTransform); + } + info.touchableRegion = Region(inputBoundsInDisplaySpace); } else if (cropLayer != nullptr) { - info.touchableRegion = info.touchableRegion.intersect( - displayTransform.transform(Rect{cropLayer->mScreenBounds})); + FloatRect inputBounds = cropLayer->getInputBounds(/*fillParentBounds=*/true).first; + Rect inputBoundsInDisplaySpace = + cropLayer->getInputBoundsInDisplaySpace(inputBounds, displayTransform); + info.touchableRegion = info.touchableRegion.intersect(inputBoundsInDisplaySpace); } // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state @@ -2513,6 +2485,27 @@ WindowInfo Layer::fillInputInfo(const InputDisplayArgs& displayArgs) { return info; } +Rect Layer::getInputBoundsInDisplaySpace(const FloatRect& inputBounds, + const ui::Transform& screenToDisplay) { + // InputDispatcher works in the display device's coordinate space. Here, we calculate the + // frame and transform used for the layer, which determines the bounds and the coordinate space + // within which the layer will receive input. + + // Coordinate space definitions: + // - display: The display device's coordinate space. Correlates to pixels on the display. + // - screen: The post-rotation coordinate space for the display, a.k.a. logical display space. + // - layer: The coordinate space of this layer. + // - input: The coordinate space in which this layer will receive input events. This could be + // different than layer space if a surfaceInset is used, which changes the origin + // of the input space. + + // Crop the input bounds to ensure it is within the parent's bounds. + const FloatRect croppedInputBounds = mBounds.intersect(inputBounds); + const ui::Transform layerToScreen = getInputTransform(); + const ui::Transform layerToDisplay = screenToDisplay * layerToScreen; + return Rect{layerToDisplay.transform(croppedInputBounds)}; +} + sp<Layer> Layer::getClonedRoot() { if (mClonedChild != nullptr) { return sp<Layer>::fromExisting(this); @@ -3461,20 +3454,46 @@ ui::Transform Layer::getInputTransform() const { } /** + * Returns the bounds used to fill the input frame and the touchable region. + * * Similar to getInputTransform, we need to update the bounds to include the transform. * This is because bounds don't include the buffer transform, where the input assumes * that's already included. */ -Rect Layer::getInputBounds() const { - if (!hasBufferOrSidebandStream()) { - return getCroppedBufferSize(getDrawingState()); +std::pair<FloatRect, bool> Layer::getInputBounds(bool fillParentBounds) const { + Rect croppedBufferSize = getCroppedBufferSize(getDrawingState()); + FloatRect inputBounds = croppedBufferSize.toFloatRect(); + if (hasBufferOrSidebandStream() && croppedBufferSize.isValid() && + mDrawingState.transform.getType() != ui::Transform::IDENTITY) { + inputBounds = mDrawingState.transform.transform(inputBounds); + } + + bool inputBoundsValid = croppedBufferSize.isValid(); + if (!inputBoundsValid) { + /** + * Input bounds are based on the layer crop or buffer size. But if we are using + * the layer bounds as the input bounds (replaceTouchableRegionWithCrop flag) then + * we can use the parent bounds as the input bounds if the layer does not have buffer + * or a crop. We want to unify this logic but because of compat reasons we cannot always + * use the parent bounds. A layer without a buffer can get input. So when a window is + * initially added, its touchable region can fill its parent layer bounds and that can + * have negative consequences. + */ + inputBounds = fillParentBounds ? mBounds : FloatRect{}; } - Rect bufferBounds = getCroppedBufferSize(getDrawingState()); - if (mDrawingState.transform.getType() == ui::Transform::IDENTITY || !bufferBounds.isValid()) { - return bufferBounds; - } - return mDrawingState.transform.transform(bufferBounds); + // Clamp surface inset to the input bounds. + const float inset = static_cast<float>(mDrawingState.inputInfo.surfaceInset); + const float xSurfaceInset = std::clamp(inset, 0.f, inputBounds.getWidth() / 2.f); + const float ySurfaceInset = std::clamp(inset, 0.f, inputBounds.getHeight() / 2.f); + + // Apply the insets to the input bounds. + inputBounds.left += xSurfaceInset; + inputBounds.top += ySurfaceInset; + inputBounds.right -= xSurfaceInset; + inputBounds.bottom -= ySurfaceInset; + + return {inputBounds, inputBoundsValid}; } bool Layer::simpleBufferUpdate(const layer_state_t& s) const { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 2fb122cac3..9ed5899607 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -570,6 +570,8 @@ public: FloatRect getBounds(const Region& activeTransparentRegion) const; FloatRect getBounds() const; + Rect getInputBoundsInDisplaySpace(const FloatRect& insetBounds, + const ui::Transform& displayTransform); // Compute bounds for the layer and cache the results. void computeBounds(FloatRect parentBounds, ui::Transform parentTransform, float shadowRadius); @@ -931,7 +933,7 @@ protected: * "replaceTouchableRegionWithCrop" is specified. In this case, the layer will receive input * in this layer's space, regardless of the specified crop layer. */ - Rect getInputBounds() const; + std::pair<FloatRect, bool> getInputBounds(bool fillParentBounds) const; // constant sp<SurfaceFlinger> mFlinger; |