diff --git a/services/surfaceflinger/CleanSpec.mk b/services/surfaceflinger/CleanSpec.mk new file mode 100644 index 000000000..c46eaebbe --- /dev/null +++ b/services/surfaceflinger/CleanSpec.mk @@ -0,0 +1,51 @@ +# Copyright (C) 2016 The CyanogenMod Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# If you don't need to do a full clean build but would like to touch +# a file or delete some intermediate files, add a clean step to the end +# of the list. These steps will only be run once, if they haven't been +# run before. +# +# E.g.: +# $(call add-clean-step, touch -c external/sqlite/sqlite3.h) +# $(call add-clean-step, rm -rf $(PRODUCT_OUT)/obj/STATIC_LIBRARIES/libz_intermediates) +# +# Always use "touch -c" and "rm -f" or "rm -rf" to gracefully deal with +# files that are missing or have been moved. +# +# Use $(PRODUCT_OUT) to get to the "out/target/product/blah/" directory. +# Use $(OUT_DIR) to refer to the "out" directory. +# +# If you need to re-do something that's already mentioned, just copy +# the command and add it to the bottom of the list. E.g., if a change +# that you made last week required touching a file and a change you +# made today requires touching the same file, just copy the old +# touch step and add it to the end of the list. +# +# ************************************************ +# NEWER CLEAN STEPS MUST BE AT THE END OF THE LIST +# ************************************************ + +# For example: +#$(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/APPS/AndroidTests_intermediates) +#$(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/JAVA_LIBRARIES/core_intermediates) +#$(call add-clean-step, find $(OUT_DIR) -type f -name "IGTalkSession*" -print0 | xargs -0 rm -f) +#$(call add-clean-step, rm -rf $(PRODUCT_OUT)/data/*) + +# ************************************************ +# NEWER CLEAN STEPS MUST BE AT THE END OF THE LIST +# ************************************************ +$(call add-clean-step, rm -rf $(PRODUCT_OUT)/obj/SHARED_LIBRARIES/libsurfaceflinger_intermediates) +$(call add-clean-step, rm -rf $(PRODUCT_OUT)/obj/SHARED_LIBRARIES/libsurfaceflinger_ddmconnection_intermediates) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 6dd8bad32..c346a2ffa 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -104,9 +104,12 @@ Layer::Layer(SurfaceFlinger* flinger, const sp& client, mName = name; + mCurrentState.active.x = 0; + mCurrentState.active.y = 0; mCurrentState.active.w = w; mCurrentState.active.h = h; mCurrentState.active.crop.makeInvalid(); + mCurrentState.active.isPositionPending = false; mCurrentState.z = 0; mCurrentState.alpha = 0xFF; mCurrentState.blur = 0xFF; @@ -1027,6 +1030,17 @@ uint32_t Layer::doTransaction(uint32_t flags) { if (flags & eDontUpdateGeometryState) { } else { Layer::State& editCurrentState(getCurrentState()); + // If a position change was requested, and we have the correct + // buffer size, no need to delay, update state now. + if (editCurrentState.requested.isPositionPending) { + float requestedX = editCurrentState.requested.x; + float requestedY = editCurrentState.requested.y; + if (requestedX != editCurrentState.active.x || + requestedY != editCurrentState.active.y) { + editCurrentState.requested.isPositionPending = false; + editCurrentState.transform.set(requestedX, requestedY); + } + } editCurrentState.active = c.requested; } @@ -1064,10 +1078,15 @@ uint32_t Layer::setTransactionFlags(uint32_t flags) { } bool Layer::setPosition(float x, float y) { - if (mCurrentState.transform.tx() == x && mCurrentState.transform.ty() == y) + if ((mCurrentState.transform.tx() == x && mCurrentState.transform.ty() == y + && !mCurrentState.requested.isPositionPending) || + (mCurrentState.requested.isPositionPending && mCurrentState.requested.x == x + && mCurrentState.requested.y == y)) return false; mCurrentState.sequence++; - mCurrentState.transform.set(x, y); + mCurrentState.requested.x = x; + mCurrentState.requested.y = y; + mCurrentState.requested.isPositionPending = true; setTransactionFlags(eTransactionNeeded); return true; } @@ -1290,6 +1309,19 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) (bufWidth == front.requested.w && bufHeight == front.requested.h)) { + + // If a position change was requested along with a resize. + // Now that we have the correct buffer size, update the position as well. + if (current.requested.isPositionPending) { + float requestedX = current.requested.x; + float requestedY = current.requested.y; + if (requestedX != current.active.x || requestedY != current.active.y) { + front.transform.set(requestedX, requestedY); + current.transform.set(requestedX, requestedY); + current.requested.isPositionPending = false; + } + } + // Here we pretend the transaction happened by updating the // current and drawing states. Drawing state is only accessed // in this thread, no need to have it locked diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 196ef3eb1..02d6f1639 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -95,11 +95,15 @@ public: }; struct Geometry { + float x; + float y; uint32_t w; uint32_t h; + bool isPositionPending; Rect crop; inline bool operator ==(const Geometry& rhs) const { - return (w == rhs.w && h == rhs.h && crop == rhs.crop); + return (w == rhs.w && h == rhs.h && crop == rhs.crop && x == rhs.x && y == rhs.y + && isPositionPending == rhs.isPositionPending); } inline bool operator !=(const Geometry& rhs) const { return !operator ==(rhs); diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index dcde51201..2ef2a502c 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -249,4 +249,50 @@ TEST_F(LayerUpdateTest, LayerResizeWorks) { } } +// Ensure that if we move and resize a surface in the same +// transaction, we don't reposition the surface and draw +// using the incorrect buffer size +TEST_F(LayerUpdateTest, LayerMoveAndResizeWorks) { + sp sc; + { + SCOPED_TRACE("before resize and reposition"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel( 0, 12, 63, 63, 195); + sc->checkPixel( 75, 75, 195, 63, 63); + sc->checkPixel(145, 145, 63, 63, 195); + } + + ALOGD("resizing and repositioning"); + SurfaceComposerClient::openGlobalTransaction(); + ASSERT_EQ(NO_ERROR, mFGSurfaceControl->setPosition(64, 0)); + ASSERT_EQ(NO_ERROR, mFGSurfaceControl->setSize(64, 128)); + SurfaceComposerClient::closeGlobalTransaction(true); + + ALOGD("resized and repositioned"); + { + // This should not reflect the new size, position or color because SurfaceFlinger + // has not yet received a buffer of the correct size. + SCOPED_TRACE("after resize, before redraw"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel( 0, 12, 63, 63, 195); + sc->checkPixel( 75, 75, 195, 63, 63); + sc->checkPixel(145, 145, 63, 63, 195); + } + + ALOGD("drawing"); + fillSurfaceRGBA8(mFGSurfaceControl, 63, 195, 63); + waitForPostedBuffers(); + ALOGD("drawn"); + { + // This should reflect the new size, position and the new color. + SCOPED_TRACE("after redraw"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel( 64, 0, 63, 195, 63); + // This should pass to imply that we didn't have a frame where the + // surface was moved but not yet resized even though the operations + // were part of the same transaction + sc->checkPixel( 64, 75, 63, 195, 63); + sc->checkPixel(145, 145, 63, 63, 195); + } +} }