From 0694d0f3b3d016b9eedda13c447e8e7735a4a177 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Sun, 24 Oct 2010 14:53:05 -0700 Subject: [PATCH] fix [3118445] Transform * Transform does not work as expected The problem wasn't in the multiply operator, but rather in the code that built the transform from the HAL bitmask. We now use the multiply operator to build the Transform from the bitmask, which guarantees, it'll always be correct. Also added a simple test for Transform. Change-Id: I09bf3b0e51d92f59d83ea91c4cc94fc2aa0bf227 --- services/surfaceflinger/Transform.cpp | 60 ++++++------------- services/surfaceflinger/Transform.h | 9 +-- .../surfaceflinger/tests/transform/Android.mk | 19 ++++++ .../tests/transform/TransformTest.cpp | 45 ++++++++++++++ 4 files changed, 87 insertions(+), 46 deletions(-) create mode 100644 services/surfaceflinger/tests/transform/Android.mk create mode 100644 services/surfaceflinger/tests/transform/TransformTest.cpp diff --git a/services/surfaceflinger/Transform.cpp b/services/surfaceflinger/Transform.cpp index 5e27cc9bd..f1284295b 100644 --- a/services/surfaceflinger/Transform.cpp +++ b/services/surfaceflinger/Transform.cpp @@ -159,56 +159,32 @@ status_t Transform::set(uint32_t flags, float w, float h) return BAD_VALUE; } - mType = flags << 8; - float sx = (flags & FLIP_H) ? -1 : 1; - float sy = (flags & FLIP_V) ? -1 : 1; - float a=0, b=0, c=0, d=0, x=0, y=0; - int xmask = 0; - - // computation of x,y - // x y - // 0 0 0 - // w 0 ROT90 - // w h FLIPH|FLIPV - // 0 h FLIPH|FLIPV|ROT90 - - if (flags & ROT_90) { - mType |= ROTATE; - b = -sy; - c = sx; - xmask = 1; - } else { - a = sx; - d = sy; - } - + Transform H, V, R; if (flags & FLIP_H) { - mType ^= SCALE; - xmask ^= 1; + H.mType = (FLIP_H << 8) | SCALE; + H.mType |= isZero(w) ? IDENTITY : TRANSLATE; + mat33& M(H.mMatrix); + M[0][0] = -1; + M[2][0] = w; } if (flags & FLIP_V) { - mType ^= SCALE; - y = h; + V.mType = (FLIP_V << 8) | SCALE; + V.mType |= isZero(h) ? IDENTITY : TRANSLATE; + mat33& M(V.mMatrix); + M[1][1] = -1; + M[2][1] = h; } - if ((flags & ROT_180) == ROT_180) { - mType |= ROTATE; + if (flags & ROT_90) { + R.mType = (ROT_90 << 8) | ROTATE; + R.mType |= isZero(w) ? IDENTITY : TRANSLATE; + mat33& M(R.mMatrix); + M[0][0] = 0; M[1][0] =-1; M[2][0] = w; + M[0][1] = 1; M[1][1] = 0; } - if (xmask) { - x = w; - } - - if (!isZero(x) || !isZero(y)) { - mType |= TRANSLATE; - } - - mat33& M(mMatrix); - M[0][0] = a; M[1][0] = b; M[2][0] = x; - M[0][1] = c; M[1][1] = d; M[2][1] = y; - M[0][2] = 0; M[1][2] = 0; M[2][2] = 1; - + *this = ((H*V)*R); return NO_ERROR; } diff --git a/services/surfaceflinger/Transform.h b/services/surfaceflinger/Transform.h index 20fa11a46..8fa5b86ff 100644 --- a/services/surfaceflinger/Transform.h +++ b/services/surfaceflinger/Transform.h @@ -23,6 +23,8 @@ #include #include +#include + namespace android { class Region; @@ -37,12 +39,11 @@ public: explicit Transform(uint32_t orientation); ~Transform(); - // FIXME: must match OVERLAY_TRANSFORM_*, pull from hardware.h enum orientation_flags { ROT_0 = 0x00000000, - FLIP_H = 0x00000001, - FLIP_V = 0x00000002, - ROT_90 = 0x00000004, + FLIP_H = HAL_TRANSFORM_FLIP_H, + FLIP_V = HAL_TRANSFORM_FLIP_V, + ROT_90 = HAL_TRANSFORM_ROT_90, ROT_180 = FLIP_H|FLIP_V, ROT_270 = ROT_180|ROT_90, ROT_INVALID = 0x80 diff --git a/services/surfaceflinger/tests/transform/Android.mk b/services/surfaceflinger/tests/transform/Android.mk new file mode 100644 index 000000000..6219dae8b --- /dev/null +++ b/services/surfaceflinger/tests/transform/Android.mk @@ -0,0 +1,19 @@ +LOCAL_PATH:= $(call my-dir) +include $(CLEAR_VARS) + +LOCAL_SRC_FILES:= \ + TransformTest.cpp \ + ../../Transform.cpp + +LOCAL_SHARED_LIBRARIES := \ + libcutils \ + libutils \ + libui \ + +LOCAL_MODULE:= test-transform + +LOCAL_MODULE_TAGS := tests + +LOCAL_C_INCLUDES += ../.. + +include $(BUILD_EXECUTABLE) diff --git a/services/surfaceflinger/tests/transform/TransformTest.cpp b/services/surfaceflinger/tests/transform/TransformTest.cpp new file mode 100644 index 000000000..e112c4ee0 --- /dev/null +++ b/services/surfaceflinger/tests/transform/TransformTest.cpp @@ -0,0 +1,45 @@ +/* + * Copyright (C) 2007 The Android Open Source 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. + */ + +#include +#include "../../Transform.h" + +using namespace android; + +int main(int argc, char **argv) +{ + Transform tr90(Transform::ROT_90); + Transform trFH(Transform::FLIP_H); + Transform trFV(Transform::FLIP_V); + + Transform tr90FH(Transform::ROT_90 | Transform::FLIP_H); + Transform tr90FV(Transform::ROT_90 | Transform::FLIP_V); + + tr90.dump("tr90"); + trFH.dump("trFH"); + trFV.dump("trFV"); + + tr90FH.dump("tr90FH"); + tr90FV.dump("tr90FV"); + + (trFH*tr90).dump("trFH*tr90"); + (trFV*tr90).dump("trFV*tr90"); + + (tr90*trFH).dump("tr90*trFH"); + (tr90*trFV).dump("tr90*trFV"); + + return 0; +}