Support floating point values for layer crop

Flag: EXEMPT bug fix
Fixes: 310950423
Test: presubmit
Change-Id: I05feb4881a95bc8caad90a3d632b3c7881909bf3

22 files changed, 105 insertions, 73 deletions

diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index b10996951b..422c57bc87 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -69,7 +69,7 @@ layer_state_t::layer_state_t()
         color(0),
         bufferTransform(0),
         transformToDisplayInverse(false),
-        crop(Rect::INVALID_RECT),
+        crop({0, 0, -1, -1}),
         dataspace(ui::Dataspace::UNKNOWN),
         surfaceDamageRegion(),
         api(-1),
@@ -109,7 +109,10 @@ status_t layer_state_t::write(Parcel& output) const
     SAFE_PARCEL(output.writeUint32, flags);
     SAFE_PARCEL(output.writeUint32, mask);
     SAFE_PARCEL(matrix.write, output);
-    SAFE_PARCEL(output.write, crop);
+    SAFE_PARCEL(output.writeFloat, crop.top);
+    SAFE_PARCEL(output.writeFloat, crop.left);
+    SAFE_PARCEL(output.writeFloat, crop.bottom);
+    SAFE_PARCEL(output.writeFloat, crop.right);
     SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, relativeLayerSurfaceControl);
     SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, parentSurfaceControlForChild);
     SAFE_PARCEL(output.writeFloat, color.r);
@@ -218,7 +221,10 @@ status_t layer_state_t::read(const Parcel& input)
     SAFE_PARCEL(input.readUint32, &mask);
 
     SAFE_PARCEL(matrix.read, input);
-    SAFE_PARCEL(input.read, crop);
+    SAFE_PARCEL(input.readFloat, &crop.top);
+    SAFE_PARCEL(input.readFloat, &crop.left);
+    SAFE_PARCEL(input.readFloat, &crop.bottom);
+    SAFE_PARCEL(input.readFloat, &crop.right);
 
     SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &relativeLayerSurfaceControl);
     SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &parentSurfaceControlForChild);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index df58df43be..21a375ec1c 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1652,6 +1652,11 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setMatri
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop(
         const sp<SurfaceControl>& sc, const Rect& crop) {
+    return setCrop(sc, crop.toFloatRect());
+}
+
+SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop(
+        const sp<SurfaceControl>& sc, const FloatRect& crop) {
     layer_state_t* s = getLayerState(sc);
     if (!s) {
         mStatus = BAD_INDEX;
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 2cdde3255e..00065c81d8 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -330,7 +330,7 @@ struct layer_state_t {
     Region transparentRegion;
     uint32_t bufferTransform;
     bool transformToDisplayInverse;
-    Rect crop;
+    FloatRect crop;
     std::shared_ptr<BufferData> bufferData = nullptr;
     ui::Dataspace dataspace;
     HdrMetadata hdrMetadata;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 4f9af16826..2b99c0feb3 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -549,6 +549,7 @@ public:
         Transaction& setMatrix(const sp<SurfaceControl>& sc,
                 float dsdx, float dtdx, float dtdy, float dsdy);
         Transaction& setCrop(const sp<SurfaceControl>& sc, const Rect& crop);
+        Transaction& setCrop(const sp<SurfaceControl>& sc, const FloatRect& crop);
         Transaction& setCornerRadius(const sp<SurfaceControl>& sc, float cornerRadius);
         Transaction& setBackgroundBlurRadius(const sp<SurfaceControl>& sc,
                                              int backgroundBlurRadius);
diff --git a/libs/gui/tests/SamplingDemo.cpp b/libs/gui/tests/SamplingDemo.cpp
index f98437b4f8..8fea689c91 100644
--- a/libs/gui/tests/SamplingDemo.cpp
+++ b/libs/gui/tests/SamplingDemo.cpp
@@ -46,7 +46,8 @@ public:
 
         SurfaceComposerClient::Transaction{}
                 .setLayer(mButton, 0x7fffffff)
-                .setCrop(mButton, {0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING})
+                .setCrop(mButton,
+                         Rect{0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING})
                 .setPosition(mButton, samplingArea.left + BUTTON_PADDING,
                              samplingArea.top + BUTTON_PADDING)
                 .setColor(mButton, half3{1, 1, 1})
@@ -59,7 +60,8 @@ public:
         SurfaceComposerClient::Transaction{}
                 .setLayer(mButtonBlend, 0x7ffffffe)
                 .setCrop(mButtonBlend,
-                         {0, 0, width - 2 * SAMPLE_AREA_PADDING, height - 2 * SAMPLE_AREA_PADDING})
+                         Rect{0, 0, width - 2 * SAMPLE_AREA_PADDING,
+                              height - 2 * SAMPLE_AREA_PADDING})
                 .setPosition(mButtonBlend, samplingArea.left + SAMPLE_AREA_PADDING,
                              samplingArea.top + SAMPLE_AREA_PADDING)
                 .setColor(mButtonBlend, half3{1, 1, 1})
@@ -75,7 +77,7 @@ public:
 
             SurfaceComposerClient::Transaction{}
                     .setLayer(mSamplingArea, 0x7ffffffd)
-                    .setCrop(mSamplingArea, {0, 0, 100, 32})
+                    .setCrop(mSamplingArea, Rect{0, 0, 100, 32})
                     .setPosition(mSamplingArea, 490, 1606)
                     .setColor(mSamplingArea, half3{0, 1, 0})
                     .setAlpha(mSamplingArea, 0.1)
diff --git a/libs/ui/include/ui/FloatRect.h b/libs/ui/include/ui/FloatRect.h
index 4c9c7b7ef1..4366db539f 100644
--- a/libs/ui/include/ui/FloatRect.h
+++ b/libs/ui/include/ui/FloatRect.h
@@ -51,6 +51,9 @@ public:
     float bottom = 0.0f;
 
     constexpr bool isEmpty() const { return !(left < right && top < bottom); }
+
+    // a valid rectangle has a non negative width and height
+    inline bool isValid() const { return (getWidth() >= 0) && (getHeight() >= 0); }
 };
 
 inline bool operator==(const FloatRect& a, const FloatRect& b) {
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
index d1429a2ec6..14a8fd6ad7 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
@@ -155,7 +155,7 @@ struct LayerFECompositionState {
     uint32_t geomBufferTransform{0};
     Rect geomBufferSize;
     Rect geomContentCrop;
-    Rect geomCrop;
+    FloatRect geomCrop;
 
     GenericLayerMetadataMap metadata;
 
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 091c207464..8e5c99126d 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -24,6 +24,7 @@
 #include <compositionengine/impl/OutputLayerCompositionState.h>
 #include <cstdint>
 #include "system/graphics-base-v1.0.h"
+#include "ui/FloatRect.h"
 
 #include <ui/HdrRenderTypeUtils.h>
 
@@ -185,35 +186,35 @@ Rect OutputLayer::calculateOutputDisplayFrame() const {
     const auto& layerState = *getLayerFE().getCompositionState();
     const auto& outputState = getOutput().getState();
 
+    // Convert from layer space to layerStackSpace
     // apply the layer's transform, followed by the display's global transform
     // here we're guaranteed that the layer's transform preserves rects
-    Region activeTransparentRegion = layerState.transparentRegionHint;
     const ui::Transform& layerTransform = layerState.geomLayerTransform;
-    const ui::Transform& inverseLayerTransform = layerState.geomInverseLayerTransform;
-    const Rect& bufferSize = layerState.geomBufferSize;
-    Rect activeCrop = layerState.geomCrop;
-    if (!activeCrop.isEmpty() && bufferSize.isValid()) {
-        activeCrop = layerTransform.transform(activeCrop);
-        if (!activeCrop.intersect(outputState.layerStackSpace.getContent(), &activeCrop)) {
-            activeCrop.clear();
-        }
-        activeCrop = inverseLayerTransform.transform(activeCrop, true);
-        // This needs to be here as transform.transform(Rect) computes the
-        // transformed rect and then takes the bounding box of the result before
-        // returning. This means
-        // transform.inverse().transform(transform.transform(Rect)) != Rect
-        // in which case we need to make sure the final rect is clipped to the
-        // display bounds.
-        if (!activeCrop.intersect(bufferSize, &activeCrop)) {
-            activeCrop.clear();
-        }
+    Region activeTransparentRegion = layerTransform.transform(layerState.transparentRegionHint);
+    if (!layerState.geomCrop.isEmpty() && layerState.geomBufferSize.isValid()) {
+        FloatRect activeCrop = layerTransform.transform(layerState.geomCrop);
+        activeCrop = activeCrop.intersect(outputState.layerStackSpace.getContent().toFloatRect());
+        const FloatRect& bufferSize =
+                layerTransform.transform(layerState.geomBufferSize.toFloatRect());
+        activeCrop = activeCrop.intersect(bufferSize);
+
         // mark regions outside the crop as transparent
-        activeTransparentRegion.orSelf(Rect(0, 0, bufferSize.getWidth(), activeCrop.top));
-        activeTransparentRegion.orSelf(
-                Rect(0, activeCrop.bottom, bufferSize.getWidth(), bufferSize.getHeight()));
-        activeTransparentRegion.orSelf(Rect(0, activeCrop.top, activeCrop.left, activeCrop.bottom));
-        activeTransparentRegion.orSelf(
-                Rect(activeCrop.right, activeCrop.top, bufferSize.getWidth(), activeCrop.bottom));
+        Rect topRegion = Rect(layerTransform.transform(
+                FloatRect(0, 0, layerState.geomBufferSize.getWidth(), layerState.geomCrop.top)));
+        Rect bottomRegion = Rect(layerTransform.transform(
+                FloatRect(0, layerState.geomCrop.bottom, layerState.geomBufferSize.getWidth(),
+                          layerState.geomBufferSize.getHeight())));
+        Rect leftRegion = Rect(layerTransform.transform(FloatRect(0, layerState.geomCrop.top,
+                                                                 layerState.geomCrop.left,
+                                                                 layerState.geomCrop.bottom)));
+        Rect rightRegion = Rect(layerTransform.transform(
+                FloatRect(layerState.geomCrop.right, layerState.geomCrop.top,
+                          layerState.geomBufferSize.getWidth(), layerState.geomCrop.bottom)));
+
+        activeTransparentRegion.orSelf(topRegion);
+        activeTransparentRegion.orSelf(bottomRegion);
+        activeTransparentRegion.orSelf(leftRegion);
+        activeTransparentRegion.orSelf(rightRegion);
     }
 
     // reduce uses a FloatRect to provide more accuracy during the
@@ -229,13 +230,14 @@ Rect OutputLayer::calculateOutputDisplayFrame() const {
         geomLayerBounds.right += outset;
         geomLayerBounds.bottom += outset;
     }
-    Rect frame{layerTransform.transform(reduce(geomLayerBounds, activeTransparentRegion))};
-    if (!frame.intersect(outputState.layerStackSpace.getContent(), &frame)) {
-        frame.clear();
-    }
-    const ui::Transform displayTransform{outputState.transform};
 
-    return displayTransform.transform(frame);
+    geomLayerBounds = layerTransform.transform(geomLayerBounds);
+    FloatRect frame = reduce(geomLayerBounds, activeTransparentRegion);
+    frame = frame.intersect(outputState.layerStackSpace.getContent().toFloatRect());
+
+    // convert from layerStackSpace to displaySpace
+    const ui::Transform displayTransform{outputState.transform};
+    return Rect(displayTransform.transform(frame));
 }
 
 uint32_t OutputLayer::calculateOutputRelativeBufferTransform(
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 1c54469cc0..95cdcd5b8d 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -30,6 +30,7 @@
 #include "MockHWC2.h"
 #include "MockHWComposer.h"
 #include "RegionMatcher.h"
+#include "ui/FloatRect.h"
 
 #include <aidl/android/hardware/graphics/composer3/Composition.h>
 
@@ -270,7 +271,7 @@ struct OutputLayerDisplayFrameTest : public OutputLayerTest {
         mLayerFEState.geomLayerTransform = ui::Transform{TR_IDENT};
         mLayerFEState.geomBufferSize = Rect{0, 0, 1920, 1080};
         mLayerFEState.geomBufferUsesDisplayInverseTransform = false;
-        mLayerFEState.geomCrop = Rect{0, 0, 1920, 1080};
+        mLayerFEState.geomCrop = FloatRect{0, 0, 1920, 1080};
         mLayerFEState.geomLayerBounds = FloatRect{0.f, 0.f, 1920.f, 1080.f};
 
         mOutputState.layerStackSpace.setContent(Rect{0, 0, 1920, 1080});
@@ -296,20 +297,20 @@ TEST_F(OutputLayerDisplayFrameTest, fullActiveTransparentRegionReturnsEmptyFrame
 }
 
 TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrame) {
-    mLayerFEState.geomCrop = Rect{100, 200, 300, 500};
+    mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500};
     const Rect expected{100, 200, 300, 500};
     EXPECT_THAT(calculateOutputDisplayFrame(), expected);
 }
 
 TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrameRotated) {
-    mLayerFEState.geomCrop = Rect{100, 200, 300, 500};
+    mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500};
     mLayerFEState.geomLayerTransform.set(HAL_TRANSFORM_ROT_90, 1920, 1080);
     const Rect expected{1420, 100, 1720, 300};
     EXPECT_THAT(calculateOutputDisplayFrame(), expected);
 }
 
 TEST_F(OutputLayerDisplayFrameTest, emptyGeomCropIsNotUsedToComputeFrame) {
-    mLayerFEState.geomCrop = Rect{};
+    mLayerFEState.geomCrop = FloatRect{};
     const Rect expected{0, 0, 1920, 1080};
     EXPECT_THAT(calculateOutputDisplayFrame(), expected);
 }
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h
index 398e64a4f1..b7d4cc5d06 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshot.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.h
@@ -72,7 +72,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState {
     bool premultipliedAlpha;
     ui::Transform parentTransform;
     Rect bufferSize;
-    Rect croppedBufferSize;
+    FloatRect croppedBufferSize;
     std::shared_ptr<renderengine::ExternalTexture> externalTexture;
     gui::LayerMetadata layerMetadata;
     gui::LayerMetadata relativeLayerMetadata;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index ee605b7a3e..10e212e7b5 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -116,7 +116,7 @@ ui::Transform getInputTransform(const LayerSnapshot& snapshot) {
  * that's already included.
  */
 std::pair<FloatRect, bool> getInputBounds(const LayerSnapshot& snapshot, bool fillParentBounds) {
-    FloatRect inputBounds = snapshot.croppedBufferSize.toFloatRect();
+    FloatRect inputBounds = snapshot.croppedBufferSize;
     if (snapshot.hasBufferOrSidebandStream() && snapshot.croppedBufferSize.isValid() &&
         snapshot.localTransform.getType() != ui::Transform::IDENTITY) {
         inputBounds = snapshot.localTransform.transform(inputBounds);
@@ -220,7 +220,7 @@ void handleDropInputMode(LayerSnapshot& snapshot, const LayerSnapshot& parentSna
     }
 
     // Check if the parent has cropped the buffer
-    Rect bufferSize = snapshot.croppedBufferSize;
+    FloatRect bufferSize = snapshot.croppedBufferSize;
     if (!bufferSize.isValid()) {
         snapshot.inputInfo.inputConfig |= gui::WindowInfo::InputConfig::DROP_INPUT_IF_OBSCURED;
         return;
@@ -970,7 +970,7 @@ void LayerSnapshotBuilder::updateRoundedCorner(LayerSnapshot& snapshot,
         parentRoundedCorner.radius.y *= t.getScaleY();
     }
 
-    FloatRect layerCropRect = snapshot.croppedBufferSize.toFloatRect();
+    FloatRect layerCropRect = snapshot.croppedBufferSize;
     const vec2 radius(requested.cornerRadius, requested.cornerRadius);
     RoundedCornerState layerSettings(layerCropRect, radius);
     const bool layerSettingsValid = layerSettings.hasRoundedCorners() && !layerCropRect.isEmpty();
@@ -1061,7 +1061,7 @@ void LayerSnapshotBuilder::updateLayerBounds(LayerSnapshot& snapshot,
             requested.externalTexture ? snapshot.bufferSize.toFloatRect() : parentBounds;
     snapshot.geomLayerCrop = parentBounds;
     if (!requested.crop.isEmpty()) {
-        snapshot.geomLayerCrop = snapshot.geomLayerCrop.intersect(requested.crop.toFloatRect());
+        snapshot.geomLayerCrop = snapshot.geomLayerCrop.intersect(requested.crop);
     }
     snapshot.geomLayerBounds = snapshot.geomLayerBounds.intersect(snapshot.geomLayerCrop);
     snapshot.transformedBounds = snapshot.geomLayerTransform.transform(snapshot.geomLayerBounds);
@@ -1072,10 +1072,10 @@ void LayerSnapshotBuilder::updateLayerBounds(LayerSnapshot& snapshot,
             snapshot.geomLayerTransform.transform(geomLayerBoundsWithoutTransparentRegion);
     snapshot.parentTransform = parentSnapshot.geomLayerTransform;
 
-    // Subtract the transparent region and snap to the bounds
-    const Rect bounds =
-            RequestedLayerState::reduce(snapshot.croppedBufferSize, requested.transparentRegion);
     if (requested.potentialCursor) {
+        // Subtract the transparent region and snap to the bounds
+        const Rect bounds = RequestedLayerState::reduce(Rect(snapshot.croppedBufferSize),
+                                                        requested.transparentRegion);
         snapshot.cursorFrame = snapshot.geomLayerTransform.transform(bounds);
     }
 }
@@ -1192,7 +1192,8 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot,
         snapshot.inputInfo.inputConfig |= InputConfig::TRUSTED_OVERLAY;
     }
 
-    snapshot.inputInfo.contentSize = snapshot.croppedBufferSize.getSize();
+    snapshot.inputInfo.contentSize = {snapshot.croppedBufferSize.getHeight(),
+                                      snapshot.croppedBufferSize.getWidth()};
 
     // If the layer is a clone, we need to crop the input region to cloned root to prevent
     // touches from going outside the cloned area.
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
index 5734ccf38f..1eff9d699c 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
@@ -96,7 +96,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args)
     LLOGV(layerId, "Created %s flags=%d", getDebugString().c_str(), flags);
     color.a = 1.0f;
 
-    crop.makeInvalid();
+    crop = {0, 0, -1, -1};
     z = 0;
     layerStack = ui::DEFAULT_LAYER_STACK;
     transformToDisplayInverse = false;
@@ -473,10 +473,10 @@ Rect RequestedLayerState::getBufferSize(uint32_t displayRotationFlags) const {
     return Rect(0, 0, static_cast<int32_t>(bufWidth), static_cast<int32_t>(bufHeight));
 }
 
-Rect RequestedLayerState::getCroppedBufferSize(const Rect& bufferSize) const {
-    Rect size = bufferSize;
+FloatRect RequestedLayerState::getCroppedBufferSize(const Rect& bufferSize) const {
+    FloatRect size = bufferSize.toFloatRect();
     if (!crop.isEmpty() && size.isValid()) {
-        size.intersect(crop, &size);
+        size = size.intersect(crop);
     } else if (!crop.isEmpty()) {
         size = crop;
     }
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
index 1d96dff336..3220e86ce4 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
@@ -79,7 +79,7 @@ struct RequestedLayerState : layer_state_t {
     bool isHiddenByPolicy() const;
     half4 getColor() const;
     Rect getBufferSize(uint32_t displayRotationFlags) const;
-    Rect getCroppedBufferSize(const Rect& bufferSize) const;
+    FloatRect getCroppedBufferSize(const Rect& bufferSize) const;
     Rect getBufferCrop() const;
     std::string getDebugString() const;
     std::string getDebugStringShort() const;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index dcb0812b67..9fdcbd0f1b 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -138,7 +138,7 @@ Layer::Layer(const surfaceflinger::LayerCreationArgs& args)
                 args.metadata.getInt32(gui::METADATA_WINDOW_TYPE, 0))) {
     ALOGV("Creating Layer %s", getDebugName());
 
-    mDrawingState.crop.makeInvalid();
+    mDrawingState.crop = {0, 0, -1, -1};
     mDrawingState.sequence = 0;
     mDrawingState.transform.set(0, 0);
     mDrawingState.frameNumber = 0;
@@ -316,7 +316,7 @@ bool Layer::computeTrustedPresentationState(const FloatRect& bounds, const Float
 
 Rect Layer::getCroppedBufferSize(const State& s) const {
     Rect size = getBufferSize(s);
-    Rect crop = getCrop(s);
+    Rect crop = Rect(getCrop(s));
     if (!crop.isEmpty() && size.isValid()) {
         size.intersect(crop, &size);
     } else if (!crop.isEmpty()) {
@@ -373,7 +373,7 @@ void Layer::setTransactionFlags(uint32_t mask) {
     mTransactionFlags |= mask;
 }
 
-bool Layer::setCrop(const Rect& crop) {
+bool Layer::setCrop(const FloatRect& crop) {
     if (mDrawingState.crop == crop) return false;
     mDrawingState.sequence++;
     mDrawingState.crop = crop;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 9bc557e917..57093ae602 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -95,7 +95,7 @@ public:
     struct State {
         int32_t sequence; // changes when visible regions can change
         // Crop is expressed in layer space coordinate.
-        Rect crop;
+        FloatRect crop;
         LayerMetadata metadata;
 
         ui::Dataspace dataspace;
@@ -172,7 +172,7 @@ public:
     // be delayed until the resize completes.
 
     // Buffer space
-    bool setCrop(const Rect& crop);
+    bool setCrop(const FloatRect& crop);
 
     bool setTransform(uint32_t /*transform*/);
     bool setTransformToDisplayInverse(bool /*transformToDisplayInverse*/);
@@ -198,7 +198,7 @@ public:
     Region getVisibleRegion(const DisplayDevice*) const;
     void updateLastLatchTime(nsecs_t latchtime);
 
-    Rect getCrop(const Layer::State& s) const { return s.crop; }
+    Rect getCrop(const Layer::State& s) const { return Rect(s.crop); }
 
     // from graphics API
     static ui::Dataspace translateDataspace(ui::Dataspace dataspace);
diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp
index 5eea45b436..a0fbab0728 100644
--- a/services/surfaceflinger/LayerProtoHelper.cpp
+++ b/services/surfaceflinger/LayerProtoHelper.cpp
@@ -106,6 +106,13 @@ void LayerProtoHelper::readFromProto(const perfetto::protos::RectProto& proto, R
     outRect.right = proto.right();
 }
 
+void LayerProtoHelper::readFromProto(const perfetto::protos::RectProto& proto, FloatRect& outRect) {
+    outRect.left = proto.left();
+    outRect.top = proto.top();
+    outRect.bottom = proto.bottom();
+    outRect.right = proto.right();
+}
+
 void LayerProtoHelper::writeToProto(
         const FloatRect& rect,
         std::function<perfetto::protos::FloatRectProto*()> getFloatRectProto) {
@@ -427,7 +434,7 @@ void LayerProtoHelper::writeSnapshotToProto(perfetto::protos::LayerProto* layerI
                                        layerInfo->mutable_color_transform());
     }
 
-    LayerProtoHelper::writeToProto(snapshot.croppedBufferSize.toFloatRect(),
+    LayerProtoHelper::writeToProto(snapshot.croppedBufferSize,
                                    [&]() { return layerInfo->mutable_source_bounds(); });
     LayerProtoHelper::writeToProto(snapshot.transformedBounds,
                                    [&]() { return layerInfo->mutable_screen_bounds(); });
@@ -455,7 +462,7 @@ void LayerProtoHelper::writeSnapshotToProto(perfetto::protos::LayerProto* layerI
         return layerInfo->mutable_requested_position();
     });
 
-    LayerProtoHelper::writeToProto(requestedState.crop,
+    LayerProtoHelper::writeToProto(Rect(requestedState.crop),
                                    [&]() { return layerInfo->mutable_crop(); });
 
     layerInfo->set_is_opaque(snapshot.contentOpaque);
diff --git a/services/surfaceflinger/LayerProtoHelper.h b/services/surfaceflinger/LayerProtoHelper.h
index 41ea68420f..3ca553a903 100644
--- a/services/surfaceflinger/LayerProtoHelper.h
+++ b/services/surfaceflinger/LayerProtoHelper.h
@@ -44,6 +44,7 @@ public:
                              std::function<perfetto::protos::RectProto*()> getRectProto);
     static void writeToProto(const Rect& rect, perfetto::protos::RectProto* rectProto);
     static void readFromProto(const perfetto::protos::RectProto& proto, Rect& outRect);
+    static void readFromProto(const perfetto::protos::RectProto& proto, FloatRect& outRect);
     static void writeToProto(const FloatRect& rect,
                              std::function<perfetto::protos::FloatRectProto*()> getFloatRectProto);
     static void writeToProto(const Region& region,
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index b1895989ec..f39a4d2af4 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -147,7 +147,7 @@ perfetto::protos::LayerState TransactionProtoParser::toProto(
         proto.set_transform_to_display_inverse(layer.transformToDisplayInverse);
     }
     if (layer.what & layer_state_t::eCropChanged) {
-        LayerProtoHelper::writeToProto(layer.crop, proto.mutable_crop());
+        LayerProtoHelper::writeToProto(Rect(layer.crop), proto.mutable_crop());
     }
     if (layer.what & layer_state_t::eBufferChanged) {
         perfetto::protos::LayerState_BufferData* bufferProto = proto.mutable_buffer_data();
diff --git a/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h b/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h
index ae380ad459..b472047bd6 100644
--- a/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h
+++ b/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h
@@ -182,7 +182,7 @@ public:
         mLifecycleManager.applyTransactions(setZTransaction(id, z));
     }
 
-    void setCrop(uint32_t id, const Rect& crop) {
+    void setCrop(uint32_t id, const FloatRect& crop) {
         std::vector<TransactionState> transactions;
         transactions.emplace_back();
         transactions.back().states.push_back({});
@@ -193,6 +193,8 @@ public:
         mLifecycleManager.applyTransactions(transactions);
     }
 
+    void setCrop(uint32_t id, const Rect& crop) { setCrop(id, crop.toFloatRect()); }
+
     void setFlags(uint32_t id, uint32_t mask, uint32_t flags) {
         std::vector<TransactionState> transactions;
         transactions.emplace_back();
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 23d3c168bd..4f72424bd7 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -467,7 +467,7 @@ struct BaseLayerProperties {
                                                          LayerProperties::FORMAT,
                                                          LayerProperties::USAGE |
                                                                  GraphicBuffer::USAGE_HW_TEXTURE);
-        layer.crop = Rect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH);
+        layer.crop = FloatRect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH);
         layer.externalTexture = buffer;
         layer.bufferData->acquireFence = Fence::NO_FENCE;
         layer.dataspace = ui::Dataspace::UNKNOWN;
@@ -664,7 +664,8 @@ struct SidebandLayerProperties : public BaseLayerProperties<SidebandLayerPropert
                 NativeHandle::create(reinterpret_cast<native_handle_t*>(DEFAULT_SIDEBAND_STREAM),
                                      false);
         layer.sidebandStream = stream;
-        layer.crop = Rect(0, 0, SidebandLayerProperties::HEIGHT, SidebandLayerProperties::WIDTH);
+        layer.crop =
+                FloatRect(0, 0, SidebandLayerProperties::HEIGHT, SidebandLayerProperties::WIDTH);
     }
 
     static void setupHwcSetSourceCropBufferCallExpectations(CompositionTest* test) {
@@ -828,7 +829,7 @@ struct EffectLayerVariant : public BaseLayerVariant<LayerProperties> {
             return frontend::RequestedLayerState(args);
         });
 
-        layer.crop = Rect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH);
+        layer.crop = FloatRect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH);
         return layer;
     }
 
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 75d2fa3c7f..a35ae15c03 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -162,12 +162,12 @@ TEST_F(LayerSnapshotTest, croppedByParent) {
     info.info.logicalHeight = 100;
     info.info.logicalWidth = 200;
     mFrontEndDisplayInfos.emplace_or_replace(ui::LayerStack::fromValue(1), info);
-    Rect layerCrop(0, 0, 10, 20);
+    FloatRect layerCrop(0, 0, 10, 20);
     setCrop(11, layerCrop);
     EXPECT_TRUE(mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Geometry));
     UPDATE_AND_VERIFY_WITH_DISPLAY_CHANGES(mSnapshotBuilder, STARTING_ZORDER);
     EXPECT_EQ(getSnapshot(11)->geomCrop, layerCrop);
-    EXPECT_EQ(getSnapshot(111)->geomLayerBounds, layerCrop.toFloatRect());
+    EXPECT_EQ(getSnapshot(111)->geomLayerBounds, layerCrop);
     float maxHeight = static_cast<float>(info.info.logicalHeight * 10);
     float maxWidth = static_cast<float>(info.info.logicalWidth * 10);
 
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index fab1f6d451..1e8cd0a0ca 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -387,7 +387,7 @@ public:
 
         state.state.what = what;
         if (what & layer_state_t::eCropChanged) {
-            state.state.crop = Rect(1, 2, 3, 4);
+            state.state.crop = FloatRect(1, 2, 3, 4);
         }
         if (what & layer_state_t::eFlagsChanged) {
             state.state.flags = layer_state_t::eEnableBackpressure;