Skip to content

Commit 614024b

Browse files
nairvis-beepAndroid (Google) Code Review
authored andcommitted
Merge "Support floating point values for layer crop" into main
2 parents aa2d404 + a9123c8 commit 614024b

22 files changed

Lines changed: 105 additions & 73 deletions

libs/gui/LayerState.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ layer_state_t::layer_state_t()
6969
color(0),
7070
bufferTransform(0),
7171
transformToDisplayInverse(false),
72-
crop(Rect::INVALID_RECT),
72+
crop({0, 0, -1, -1}),
7373
dataspace(ui::Dataspace::UNKNOWN),
7474
surfaceDamageRegion(),
7575
api(-1),
@@ -109,7 +109,10 @@ status_t layer_state_t::write(Parcel& output) const
109109
SAFE_PARCEL(output.writeUint32, flags);
110110
SAFE_PARCEL(output.writeUint32, mask);
111111
SAFE_PARCEL(matrix.write, output);
112-
SAFE_PARCEL(output.write, crop);
112+
SAFE_PARCEL(output.writeFloat, crop.top);
113+
SAFE_PARCEL(output.writeFloat, crop.left);
114+
SAFE_PARCEL(output.writeFloat, crop.bottom);
115+
SAFE_PARCEL(output.writeFloat, crop.right);
113116
SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, relativeLayerSurfaceControl);
114117
SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, parentSurfaceControlForChild);
115118
SAFE_PARCEL(output.writeFloat, color.r);
@@ -218,7 +221,10 @@ status_t layer_state_t::read(const Parcel& input)
218221
SAFE_PARCEL(input.readUint32, &mask);
219222

220223
SAFE_PARCEL(matrix.read, input);
221-
SAFE_PARCEL(input.read, crop);
224+
SAFE_PARCEL(input.readFloat, &crop.top);
225+
SAFE_PARCEL(input.readFloat, &crop.left);
226+
SAFE_PARCEL(input.readFloat, &crop.bottom);
227+
SAFE_PARCEL(input.readFloat, &crop.right);
222228

223229
SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &relativeLayerSurfaceControl);
224230
SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &parentSurfaceControlForChild);

libs/gui/SurfaceComposerClient.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1645,6 +1645,11 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setMatri
16451645

16461646
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop(
16471647
const sp<SurfaceControl>& sc, const Rect& crop) {
1648+
return setCrop(sc, crop.toFloatRect());
1649+
}
1650+
1651+
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop(
1652+
const sp<SurfaceControl>& sc, const FloatRect& crop) {
16481653
layer_state_t* s = getLayerState(sc);
16491654
if (!s) {
16501655
mStatus = BAD_INDEX;

libs/gui/include/gui/LayerState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ struct layer_state_t {
330330
Region transparentRegion;
331331
uint32_t bufferTransform;
332332
bool transformToDisplayInverse;
333-
Rect crop;
333+
FloatRect crop;
334334
std::shared_ptr<BufferData> bufferData = nullptr;
335335
ui::Dataspace dataspace;
336336
HdrMetadata hdrMetadata;

libs/gui/include/gui/SurfaceComposerClient.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ class SurfaceComposerClient : public RefBase
549549
Transaction& setMatrix(const sp<SurfaceControl>& sc,
550550
float dsdx, float dtdx, float dtdy, float dsdy);
551551
Transaction& setCrop(const sp<SurfaceControl>& sc, const Rect& crop);
552+
Transaction& setCrop(const sp<SurfaceControl>& sc, const FloatRect& crop);
552553
Transaction& setCornerRadius(const sp<SurfaceControl>& sc, float cornerRadius);
553554
Transaction& setBackgroundBlurRadius(const sp<SurfaceControl>& sc,
554555
int backgroundBlurRadius);

libs/gui/tests/SamplingDemo.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class Button : public gui::BnRegionSamplingListener {
4646

4747
SurfaceComposerClient::Transaction{}
4848
.setLayer(mButton, 0x7fffffff)
49-
.setCrop(mButton, {0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING})
49+
.setCrop(mButton,
50+
Rect{0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING})
5051
.setPosition(mButton, samplingArea.left + BUTTON_PADDING,
5152
samplingArea.top + BUTTON_PADDING)
5253
.setColor(mButton, half3{1, 1, 1})
@@ -59,7 +60,8 @@ class Button : public gui::BnRegionSamplingListener {
5960
SurfaceComposerClient::Transaction{}
6061
.setLayer(mButtonBlend, 0x7ffffffe)
6162
.setCrop(mButtonBlend,
62-
{0, 0, width - 2 * SAMPLE_AREA_PADDING, height - 2 * SAMPLE_AREA_PADDING})
63+
Rect{0, 0, width - 2 * SAMPLE_AREA_PADDING,
64+
height - 2 * SAMPLE_AREA_PADDING})
6365
.setPosition(mButtonBlend, samplingArea.left + SAMPLE_AREA_PADDING,
6466
samplingArea.top + SAMPLE_AREA_PADDING)
6567
.setColor(mButtonBlend, half3{1, 1, 1})
@@ -75,7 +77,7 @@ class Button : public gui::BnRegionSamplingListener {
7577

7678
SurfaceComposerClient::Transaction{}
7779
.setLayer(mSamplingArea, 0x7ffffffd)
78-
.setCrop(mSamplingArea, {0, 0, 100, 32})
80+
.setCrop(mSamplingArea, Rect{0, 0, 100, 32})
7981
.setPosition(mSamplingArea, 490, 1606)
8082
.setColor(mSamplingArea, half3{0, 1, 0})
8183
.setAlpha(mSamplingArea, 0.1)

libs/ui/include/ui/FloatRect.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ class FloatRect {
5151
float bottom = 0.0f;
5252

5353
constexpr bool isEmpty() const { return !(left < right && top < bottom); }
54+
55+
// a valid rectangle has a non negative width and height
56+
inline bool isValid() const { return (getWidth() >= 0) && (getHeight() >= 0); }
5457
};
5558

5659
inline bool operator==(const FloatRect& a, const FloatRect& b) {

services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ struct LayerFECompositionState {
155155
uint32_t geomBufferTransform{0};
156156
Rect geomBufferSize;
157157
Rect geomContentCrop;
158-
Rect geomCrop;
158+
FloatRect geomCrop;
159159

160160
GenericLayerMetadataMap metadata;
161161

services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <compositionengine/impl/OutputLayerCompositionState.h>
2525
#include <cstdint>
2626
#include "system/graphics-base-v1.0.h"
27+
#include "ui/FloatRect.h"
2728

2829
#include <ui/HdrRenderTypeUtils.h>
2930

@@ -185,35 +186,35 @@ Rect OutputLayer::calculateOutputDisplayFrame() const {
185186
const auto& layerState = *getLayerFE().getCompositionState();
186187
const auto& outputState = getOutput().getState();
187188

189+
// Convert from layer space to layerStackSpace
188190
// apply the layer's transform, followed by the display's global transform
189191
// here we're guaranteed that the layer's transform preserves rects
190-
Region activeTransparentRegion = layerState.transparentRegionHint;
191192
const ui::Transform& layerTransform = layerState.geomLayerTransform;
192-
const ui::Transform& inverseLayerTransform = layerState.geomInverseLayerTransform;
193-
const Rect& bufferSize = layerState.geomBufferSize;
194-
Rect activeCrop = layerState.geomCrop;
195-
if (!activeCrop.isEmpty() && bufferSize.isValid()) {
196-
activeCrop = layerTransform.transform(activeCrop);
197-
if (!activeCrop.intersect(outputState.layerStackSpace.getContent(), &activeCrop)) {
198-
activeCrop.clear();
199-
}
200-
activeCrop = inverseLayerTransform.transform(activeCrop, true);
201-
// This needs to be here as transform.transform(Rect) computes the
202-
// transformed rect and then takes the bounding box of the result before
203-
// returning. This means
204-
// transform.inverse().transform(transform.transform(Rect)) != Rect
205-
// in which case we need to make sure the final rect is clipped to the
206-
// display bounds.
207-
if (!activeCrop.intersect(bufferSize, &activeCrop)) {
208-
activeCrop.clear();
209-
}
193+
Region activeTransparentRegion = layerTransform.transform(layerState.transparentRegionHint);
194+
if (!layerState.geomCrop.isEmpty() && layerState.geomBufferSize.isValid()) {
195+
FloatRect activeCrop = layerTransform.transform(layerState.geomCrop);
196+
activeCrop = activeCrop.intersect(outputState.layerStackSpace.getContent().toFloatRect());
197+
const FloatRect& bufferSize =
198+
layerTransform.transform(layerState.geomBufferSize.toFloatRect());
199+
activeCrop = activeCrop.intersect(bufferSize);
200+
210201
// mark regions outside the crop as transparent
211-
activeTransparentRegion.orSelf(Rect(0, 0, bufferSize.getWidth(), activeCrop.top));
212-
activeTransparentRegion.orSelf(
213-
Rect(0, activeCrop.bottom, bufferSize.getWidth(), bufferSize.getHeight()));
214-
activeTransparentRegion.orSelf(Rect(0, activeCrop.top, activeCrop.left, activeCrop.bottom));
215-
activeTransparentRegion.orSelf(
216-
Rect(activeCrop.right, activeCrop.top, bufferSize.getWidth(), activeCrop.bottom));
202+
Rect topRegion = Rect(layerTransform.transform(
203+
FloatRect(0, 0, layerState.geomBufferSize.getWidth(), layerState.geomCrop.top)));
204+
Rect bottomRegion = Rect(layerTransform.transform(
205+
FloatRect(0, layerState.geomCrop.bottom, layerState.geomBufferSize.getWidth(),
206+
layerState.geomBufferSize.getHeight())));
207+
Rect leftRegion = Rect(layerTransform.transform(FloatRect(0, layerState.geomCrop.top,
208+
layerState.geomCrop.left,
209+
layerState.geomCrop.bottom)));
210+
Rect rightRegion = Rect(layerTransform.transform(
211+
FloatRect(layerState.geomCrop.right, layerState.geomCrop.top,
212+
layerState.geomBufferSize.getWidth(), layerState.geomCrop.bottom)));
213+
214+
activeTransparentRegion.orSelf(topRegion);
215+
activeTransparentRegion.orSelf(bottomRegion);
216+
activeTransparentRegion.orSelf(leftRegion);
217+
activeTransparentRegion.orSelf(rightRegion);
217218
}
218219

219220
// reduce uses a FloatRect to provide more accuracy during the
@@ -231,13 +232,14 @@ Rect OutputLayer::calculateOutputDisplayFrame() const {
231232
geomLayerBounds.right += outset;
232233
geomLayerBounds.bottom += outset;
233234
}
234-
Rect frame{layerTransform.transform(reduce(geomLayerBounds, activeTransparentRegion))};
235-
if (!frame.intersect(outputState.layerStackSpace.getContent(), &frame)) {
236-
frame.clear();
237-
}
238-
const ui::Transform displayTransform{outputState.transform};
239235

240-
return displayTransform.transform(frame);
236+
geomLayerBounds = layerTransform.transform(geomLayerBounds);
237+
FloatRect frame = reduce(geomLayerBounds, activeTransparentRegion);
238+
frame = frame.intersect(outputState.layerStackSpace.getContent().toFloatRect());
239+
240+
// convert from layerStackSpace to displaySpace
241+
const ui::Transform displayTransform{outputState.transform};
242+
return Rect(displayTransform.transform(frame));
241243
}
242244

243245
uint32_t OutputLayer::calculateOutputRelativeBufferTransform(

services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "MockHWC2.h"
3131
#include "MockHWComposer.h"
3232
#include "RegionMatcher.h"
33+
#include "ui/FloatRect.h"
3334

3435
#include <aidl/android/hardware/graphics/composer3/Composition.h>
3536

@@ -270,7 +271,7 @@ struct OutputLayerDisplayFrameTest : public OutputLayerTest {
270271
mLayerFEState.geomLayerTransform = ui::Transform{TR_IDENT};
271272
mLayerFEState.geomBufferSize = Rect{0, 0, 1920, 1080};
272273
mLayerFEState.geomBufferUsesDisplayInverseTransform = false;
273-
mLayerFEState.geomCrop = Rect{0, 0, 1920, 1080};
274+
mLayerFEState.geomCrop = FloatRect{0, 0, 1920, 1080};
274275
mLayerFEState.geomLayerBounds = FloatRect{0.f, 0.f, 1920.f, 1080.f};
275276

276277
mOutputState.layerStackSpace.setContent(Rect{0, 0, 1920, 1080});
@@ -296,20 +297,20 @@ TEST_F(OutputLayerDisplayFrameTest, fullActiveTransparentRegionReturnsEmptyFrame
296297
}
297298

298299
TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrame) {
299-
mLayerFEState.geomCrop = Rect{100, 200, 300, 500};
300+
mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500};
300301
const Rect expected{100, 200, 300, 500};
301302
EXPECT_THAT(calculateOutputDisplayFrame(), expected);
302303
}
303304

304305
TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrameRotated) {
305-
mLayerFEState.geomCrop = Rect{100, 200, 300, 500};
306+
mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500};
306307
mLayerFEState.geomLayerTransform.set(HAL_TRANSFORM_ROT_90, 1920, 1080);
307308
const Rect expected{1420, 100, 1720, 300};
308309
EXPECT_THAT(calculateOutputDisplayFrame(), expected);
309310
}
310311

311312
TEST_F(OutputLayerDisplayFrameTest, emptyGeomCropIsNotUsedToComputeFrame) {
312-
mLayerFEState.geomCrop = Rect{};
313+
mLayerFEState.geomCrop = FloatRect{};
313314
const Rect expected{0, 0, 1920, 1080};
314315
EXPECT_THAT(calculateOutputDisplayFrame(), expected);
315316
}

services/surfaceflinger/FrontEnd/LayerSnapshot.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState {
7272
bool premultipliedAlpha;
7373
ui::Transform parentTransform;
7474
Rect bufferSize;
75-
Rect croppedBufferSize;
75+
FloatRect croppedBufferSize;
7676
std::shared_ptr<renderengine::ExternalTexture> externalTexture;
7777
gui::LayerMetadata layerMetadata;
7878
gui::LayerMetadata relativeLayerMetadata;

0 commit comments

Comments
 (0)