From af29d23eaa62d7e9b86e2605dded68e767bae3d0 Mon Sep 17 00:00:00 2001 From: Nader Jawad Date: Mon, 17 Apr 2023 19:53:58 -0700 Subject: [PATCH] Fixed issue where SurfaceView transforms were applied twice Updated stretch overscroll logic to compute the stretch region for SurfaceView after applying transforms to the the corresponding View. Removed logic that would compute the transform again after finding the nearest stretch container. Fixes: 262162781 Test: Added CTS test to HorizontalScrollViewTest and ScrollViewTest Change-Id: I6baf99dd13e20f7db43ffcf6542fd56a0b332f58 --- libs/hwui/DamageAccumulator.cpp | 43 ++--------- libs/hwui/DamageAccumulator.h | 4 +- libs/hwui/jni/android_graphics_RenderNode.cpp | 74 +++++++++---------- 3 files changed, 45 insertions(+), 76 deletions(-) diff --git a/libs/hwui/DamageAccumulator.cpp b/libs/hwui/DamageAccumulator.cpp index 9db47c3ba0904..a8d170d00ef7a 100644 --- a/libs/hwui/DamageAccumulator.cpp +++ b/libs/hwui/DamageAccumulator.cpp @@ -207,27 +207,6 @@ static void applyTransforms(DirtyStack* frame, DirtyStack* end) { } } -static void computeTransformImpl(const DirtyStack* frame, const DirtyStack* end, - Matrix4* outMatrix) { - while (frame != end) { - switch (frame->type) { - case TransformRenderNode: - frame->renderNode->applyViewPropertyTransforms(*outMatrix); - break; - case TransformMatrix4: - outMatrix->multiply(*frame->matrix4); - break; - case TransformNone: - // nothing to be done - break; - default: - LOG_ALWAYS_FATAL("Tried to compute transform with an invalid type: %d", - frame->type); - } - frame = frame->prev; - } -} - void DamageAccumulator::applyRenderNodeTransform(DirtyStack* frame) { if (frame->pendingDirty.isEmpty()) { return; @@ -282,9 +261,6 @@ void DamageAccumulator::finish(SkRect* totalDirty) { DamageAccumulator::StretchResult DamageAccumulator::findNearestStretchEffect() const { DirtyStack* frame = mHead; - const auto& headProperties = mHead->renderNode->properties(); - float startWidth = headProperties.getWidth(); - float startHeight = headProperties.getHeight(); while (frame->prev != frame) { if (frame->type == TransformRenderNode) { const auto& renderNode = frame->renderNode; @@ -295,21 +271,16 @@ DamageAccumulator::StretchResult DamageAccumulator::findNearestStretchEffect() c const float height = (float) frameRenderNodeProperties.getHeight(); if (!effect.isEmpty()) { Matrix4 stretchMatrix; - computeTransformImpl(mHead, frame, &stretchMatrix); - Rect stretchRect = Rect(0.f, 0.f, startWidth, startHeight); + computeTransformImpl(frame, &stretchMatrix); + Rect stretchRect = Rect(0.f, 0.f, width, height); stretchMatrix.mapRect(stretchRect); return StretchResult{ - .stretchEffect = &effect, - .childRelativeBounds = SkRect::MakeLTRB( - stretchRect.left, - stretchRect.top, - stretchRect.right, - stretchRect.bottom - ), - .width = width, - .height = height - }; + .stretchEffect = &effect, + .parentBounds = SkRect::MakeLTRB(stretchRect.left, stretchRect.top, + stretchRect.right, stretchRect.bottom), + .width = width, + .height = height}; } } frame = frame->prev; diff --git a/libs/hwui/DamageAccumulator.h b/libs/hwui/DamageAccumulator.h index 90a35174d929d..c4249af392d32 100644 --- a/libs/hwui/DamageAccumulator.h +++ b/libs/hwui/DamageAccumulator.h @@ -70,9 +70,9 @@ public: const StretchEffect* stretchEffect; /** - * Bounds of the child relative to the stretch container + * Bounds of the stretching container */ - const SkRect childRelativeBounds; + const SkRect parentBounds; /** * Width of the stretch container diff --git a/libs/hwui/jni/android_graphics_RenderNode.cpp b/libs/hwui/jni/android_graphics_RenderNode.cpp index ac1f92dee5076..24a785c18711d 100644 --- a/libs/hwui/jni/android_graphics_RenderNode.cpp +++ b/libs/hwui/jni/android_graphics_RenderNode.cpp @@ -584,13 +584,15 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, uirenderer::Rect bounds(props.getWidth(), props.getHeight()); bool useStretchShader = Properties::getStretchEffectBehavior() != StretchEffectBehavior::UniformScale; - if (useStretchShader && info.stretchEffectCount) { + // Compute the transform bounds first before calculating the stretch + transform.mapRect(bounds); + + bool hasStretch = useStretchShader && info.stretchEffectCount; + if (hasStretch) { handleStretchEffect(info, bounds); } - transform.mapRect(bounds); - - if (CC_LIKELY(transform.isPureTranslate())) { + if (CC_LIKELY(transform.isPureTranslate()) && !hasStretch) { // snap/round the computed bounds, so they match the rounding behavior // of the clear done in SurfaceView#draw(). bounds.snapGeometryToPixelBoundaries(false); @@ -665,45 +667,42 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, return env; } - void stretchTargetBounds(const StretchEffect& stretchEffect, - float width, float height, - const SkRect& childRelativeBounds, - uirenderer::Rect& bounds) { - float normalizedLeft = childRelativeBounds.left() / width; - float normalizedTop = childRelativeBounds.top() / height; - float normalizedRight = childRelativeBounds.right() / width; - float normalizedBottom = childRelativeBounds.bottom() / height; - float reverseLeft = width * - (stretchEffect.computeStretchedPositionX(normalizedLeft) - - normalizedLeft); - float reverseTop = height * - (stretchEffect.computeStretchedPositionY(normalizedTop) - - normalizedTop); - float reverseRight = width * - (stretchEffect.computeStretchedPositionX(normalizedRight) - - normalizedLeft); - float reverseBottom = height * - (stretchEffect.computeStretchedPositionY(normalizedBottom) - - normalizedTop); - bounds.left = reverseLeft; - bounds.top = reverseTop; - bounds.right = reverseRight; - bounds.bottom = reverseBottom; - } - void handleStretchEffect(const TreeInfo& info, uirenderer::Rect& targetBounds) { // Search up to find the nearest stretcheffect parent const DamageAccumulator::StretchResult result = info.damageAccumulator->findNearestStretchEffect(); const StretchEffect* effect = result.stretchEffect; - if (!effect) { + if (effect) { + // Compute the number of pixels that the stretching container + // scales by. + // Then compute the scale factor that the child would need + // to scale in order to occupy the same pixel bounds. + auto& parentBounds = result.parentBounds; + auto parentWidth = parentBounds.width(); + auto parentHeight = parentBounds.height(); + auto& stretchDirection = effect->getStretchDirection(); + auto stretchX = stretchDirection.x(); + auto stretchY = stretchDirection.y(); + auto stretchXPixels = parentWidth * std::abs(stretchX); + auto stretchYPixels = parentHeight * std::abs(stretchY); + SkMatrix stretchMatrix; + + auto childScaleX = 1 + (stretchXPixels / targetBounds.getWidth()); + auto childScaleY = 1 + (stretchYPixels / targetBounds.getHeight()); + auto pivotX = stretchX > 0 ? targetBounds.left : targetBounds.right; + auto pivotY = stretchY > 0 ? targetBounds.top : targetBounds.bottom; + stretchMatrix.setScale(childScaleX, childScaleY, pivotX, pivotY); + SkRect rect = SkRect::MakeLTRB(targetBounds.left, targetBounds.top, + targetBounds.right, targetBounds.bottom); + SkRect dst = stretchMatrix.mapRect(rect); + targetBounds.left = dst.left(); + targetBounds.top = dst.top(); + targetBounds.right = dst.right(); + targetBounds.bottom = dst.bottom(); + } else { return; } - const auto& childRelativeBounds = result.childRelativeBounds; - stretchTargetBounds(*effect, result.width, result.height, - childRelativeBounds,targetBounds); - if (Properties::getStretchEffectBehavior() == StretchEffectBehavior::Shader) { JNIEnv* env = jnienv(); @@ -714,9 +713,8 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, gPositionListener.clazz, gPositionListener.callApplyStretch, mListener, info.canvasContext.getFrameNumber(), result.width, result.height, stretchDirection.fX, stretchDirection.fY, effect->maxStretchAmountX, - effect->maxStretchAmountY, childRelativeBounds.left(), - childRelativeBounds.top(), childRelativeBounds.right(), - childRelativeBounds.bottom()); + effect->maxStretchAmountY, targetBounds.left, targetBounds.top, + targetBounds.right, targetBounds.bottom); if (!keepListening) { env->DeleteGlobalRef(mListener); mListener = nullptr;