From 6b44267a3beb457e220cad0666c039d3a765cdb2 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 9 Jul 2013 21:24:52 -0700 Subject: [PATCH] fix SF buffer cropping When a buffer had a crop (meaning its content is scaled to the window size) and a window crop was defined, the resulting crop couldn't be expressed properly because h/w composer's API was limited to integers, since this is fixed in h/w composer 1.3, we take adventage of this to make sure we get the correct crop. this bug could result in the buffer being scaled by an incorrect ratio and be slightly offset; moreover, it would produce different results from the GL code path, which is always correct. Change-Id: I8e20e00b6e26177d14f4ab4d2cd581e26c818892 --- .../DisplayHardware/FloatRect.h | 42 +++++++++++++++++++ .../DisplayHardware/HWComposer.cpp | 22 ++++++++-- .../DisplayHardware/HWComposer.h | 3 +- services/surfaceflinger/Layer.cpp | 41 +++++++----------- services/surfaceflinger/Layer.h | 9 +--- 5 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 services/surfaceflinger/DisplayHardware/FloatRect.h diff --git a/services/surfaceflinger/DisplayHardware/FloatRect.h b/services/surfaceflinger/DisplayHardware/FloatRect.h new file mode 100644 index 000000000..b08a95105 --- /dev/null +++ b/services/surfaceflinger/DisplayHardware/FloatRect.h @@ -0,0 +1,42 @@ +/* + * Copyright 2013 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. + */ + +#ifndef ANDROID_SF_FLOAT_RECT +#define ANDROID_SF_FLOAT_RECT + +#include + +namespace android { + +class FloatRect +{ +public: + float left; + float top; + float right; + float bottom; + + inline FloatRect() { } + inline FloatRect(const Rect& other) + : left(other.left), top(other.top), right(other.right), bottom(other.bottom) { } + + inline float getWidth() const { return right - left; } + inline float getHeight() const { return bottom - top; } +}; + +}; // namespace android + +#endif // ANDROID_SF_FLOAT_RECT diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 5082192cf..1cdabc4b5 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -880,10 +881,25 @@ public: getLayer()->transform = transform; } virtual void setFrame(const Rect& frame) { - reinterpret_cast(getLayer()->displayFrame) = frame; + getLayer()->displayFrame = reinterpret_cast(frame); } - virtual void setCrop(const Rect& crop) { - reinterpret_cast(getLayer()->sourceCrop) = crop; + virtual void setCrop(const FloatRect& crop) { + if (hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_3)) { + getLayer()->sourceCropf = reinterpret_cast(crop); + } else { + /* + * Since h/w composer didn't support a flot crop rect before version 1.3, + * using integer coordinates instead produces a different output from the GL code in + * Layer::drawWithOpenGL(). The difference can be large if the buffer crop to + * window size ratio is large and a window crop is defined + * (i.e.: if we scale the buffer a lot and we also crop it with a window crop). + */ + hwc_rect_t& r = getLayer()->sourceCrop; + r.left = int(ceilf(crop.left)); + r.top = int(ceilf(crop.top)); + r.right = int(floorf(crop.right)); + r.bottom= int(floorf(crop.bottom)); + } } virtual void setVisibleRegionScreen(const Region& reg) { // Region::getSharedBuffer creates a reference to the underlying diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index a20da08c0..0462bcc58 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -47,6 +47,7 @@ namespace android { class GraphicBuffer; class Fence; +class FloatRect; class Region; class String8; class SurfaceFlinger; @@ -158,7 +159,7 @@ public: virtual void setBlending(uint32_t blending) = 0; virtual void setTransform(uint32_t transform) = 0; virtual void setFrame(const Rect& frame) = 0; - virtual void setCrop(const Rect& crop) = 0; + virtual void setCrop(const FloatRect& crop) = 0; virtual void setVisibleRegionScreen(const Region& reg) = 0; virtual void setBuffer(const sp& buffer) = 0; virtual void setAcquireFenceFd(int fenceFd) = 0; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 3605dd451..34003b8aa 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -257,10 +257,6 @@ Rect Layer::getContentCrop() const { return crop; } -uint32_t Layer::getContentTransform() const { - return mCurrentTransform; -} - static Rect reduce(const Rect& win, const Region& exclude) { if (CC_LIKELY(exclude.isEmpty())) { return win; @@ -281,18 +277,10 @@ Rect Layer::computeBounds() const { return reduce(win, s.activeTransparentRegion); } -Rect Layer::computeCrop(const sp& hw) const { - /* - * The way we compute the crop (aka. texture coordinates when we have a - * Layer) produces a different output from the GL code in - * drawWithOpenGL() due to HWC being limited to integers. The difference - * can be large if getContentTransform() contains a large scale factor. - * See comments in drawWithOpenGL() for more details. - */ - +FloatRect Layer::computeCrop(const sp& hw) const { // the content crop is the area of the content that gets scaled to the // layer's size. - Rect crop(getContentCrop()); + FloatRect crop(getContentCrop()); // the active.crop is the area of the window that gets cropped, but not // scaled in any ways. @@ -300,10 +288,10 @@ Rect Layer::computeCrop(const sp& hw) const { // apply the projection's clipping to the window crop in // layerstack space, and convert-back to layer space. - // if there are no window scaling (or content scaling) involved, - // this operation will map to full pixels in the buffer. - // NOTE: should we revert to GL composition if a scaling is involved - // since it cannot be represented in the HWC API? + // if there are no window scaling involved, this operation will map to full + // pixels in the buffer. + // FIXME: the 3 lines below can produce slightly incorrect clipping when we have + // a viewport clipping and a window transform. we should use floating point to fix this. Rect activeCrop(s.transform.transform(s.active.crop)); activeCrop.intersect(hw->getViewport(), &activeCrop); activeCrop = s.transform.inverse().transform(activeCrop); @@ -319,7 +307,7 @@ Rect Layer::computeCrop(const sp& hw) const { // Transform the window crop to match the buffer coordinate system, // which means using the inverse of the current transform set on the // SurfaceFlingerConsumer. - uint32_t invTransform = getContentTransform(); + uint32_t invTransform = mCurrentTransform; int winWidth = s.active.w; int winHeight = s.active.h; if (invTransform & NATIVE_WINDOW_TRANSFORM_ROT_90) { @@ -331,15 +319,14 @@ Rect Layer::computeCrop(const sp& hw) const { const Rect winCrop = activeCrop.transform( invTransform, s.active.w, s.active.h); - // the code below essentially performs a scaled intersection - // of crop and winCrop - float xScale = float(crop.width()) / float(winWidth); - float yScale = float(crop.height()) / float(winHeight); + // below, crop is intersected with winCrop expressed in crop's coordinate space + float xScale = crop.getWidth() / float(winWidth); + float yScale = crop.getHeight() / float(winHeight); - int insetL = int(ceilf( winCrop.left * xScale)); - int insetT = int(ceilf( winCrop.top * yScale)); - int insetR = int(ceilf((winWidth - winCrop.right ) * xScale)); - int insetB = int(ceilf((winHeight - winCrop.bottom) * yScale)); + float insetL = winCrop.left * xScale; + float insetT = winCrop.top * yScale; + float insetR = (winWidth - winCrop.right ) * xScale; + float insetB = (winHeight - winCrop.bottom) * yScale; crop.left += insetL; crop.top += insetT; diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index e7e958549..0ceb15ee3 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -43,6 +43,7 @@ #include "Transform.h" #include "DisplayHardware/HWComposer.h" +#include "DisplayHardware/FloatRect.h" namespace android { @@ -279,12 +280,6 @@ public: */ Rect getContentCrop() const; - /* - * returns the transform bits (90 rotation / h-flip / v-flip) of the - * layer's content - */ - uint32_t getContentTransform() const; - // ----------------------------------------------------------------------- void clearWithOpenGL(const sp& hw, const Region& clip) const; @@ -334,7 +329,7 @@ private: bool needsFiltering(const sp& hw) const; uint32_t getEffectiveUsage(uint32_t usage) const; - Rect computeCrop(const sp& hw) const; + FloatRect computeCrop(const sp& hw) const; bool isCropped() const; static bool getOpacityForFormat(uint32_t format);