Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 29431017 authored by Liangliang Sui's avatar Liangliang Sui Committed by John Reck
Browse files

Fix potential crash issues. 

The `SkiaPipeline#renderLayerImpl` function may return `false` in the
middle due to the condition `(properties.getClipToBounds() &&
layerCanvas->quickReject(bounds))`. However, in such a case, neither the
`layerCanvas` state nor the `LightingInfo` is restored. This could
potentially lead to a failure in the `SkASSERT(saveCount == 1)` check
when the function is entered again for the same `RenderNode` and
`SkSurface`.

Cherry-picked from https://r.android.com/3559406



Test: build
Flag: EXEMPT bugfix

Signed-off-by: default avatarLiangliang Sui <coolsui.coding@gmail.com>
Change-Id: I5e4c84b6b2dda72755927a7d51078a446fb0ecbd
parent 067d0d32

libs/hwui/pipeline/skia/SkiaPipeline.cpp

+3 −15
Original line number Diff line number Diff line
@@ -44,7 +44,6 @@ 


#include <sstream>



#include "LightingInfo.h"

#include "VectorDrawable.h"

#include "include/gpu/GpuTypes.h"  // from Skia

#include "thread/CommonPool.h"
@@ -88,20 +87,12 @@ bool SkiaPipeline::renderLayerImpl(RenderNode* layerNode, const Rect& layerDamag 


    SkCanvas* layerCanvas = layerNode->getLayerSurface()->getCanvas();



    int saveCount = layerCanvas->save();

    SkASSERT(saveCount == 1);

    SkASSERT(layerCanvas->getSaveCount() == 1);

    SkAutoCanvasRestore saver(layerCanvas, true);



    layerCanvas->androidFramework_setDeviceClipRestriction(layerDamage.toSkIRect());



    // TODO: put localized light center calculation and storage to a drawable related code.

    // It does not seem right to store something localized in a global state

    // fix here and in recordLayers

    const Vector3 savedLightCenter(LightingInfo::getLightCenterRaw());

    Vector3 transformedLightCenter(savedLightCenter);

    // map current light center into RenderNode's coordinate space

    layerNode->getSkiaLayer()->inverseTransformInWindow.mapPoint3d(transformedLightCenter);

    LightingInfo::setLightCenterRaw(transformedLightCenter);



    AutoLightingInfoRestore restoreLightingInfo(layerNode);

    const RenderProperties& properties = layerNode->properties();

    const SkRect bounds = SkRect::MakeWH(properties.getWidth(), properties.getHeight());

    if (properties.getClipToBounds() && layerCanvas->quickReject(bounds)) {
@@ -116,9 +107,6 @@ bool SkiaPipeline::renderLayerImpl(RenderNode* layerNode, const Rect& layerDamag 


    RenderNodeDrawable root(layerNode, layerCanvas, false);

    root.forceDraw(layerCanvas);

    layerCanvas->restoreToCount(saveCount);



    LightingInfo::setLightCenterRaw(savedLightCenter);

    return true;

}

libs/hwui/pipeline/skia/SkiaPipeline.h

+21 −1
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include <SkSurface.h>



#include "Lighting.h"

#include "LightingInfo.h"

#include "hwui/AnimatedImageDrawable.h"

#include "renderthread/CanvasContext.h"

#include "renderthread/HardwareBufferRenderParams.h"
@@ -122,6 +123,25 @@ private: 
    };

    CaptureMode mCaptureMode = CaptureMode::None;



    class AutoLightingInfoRestore {

    public:

        AutoLightingInfoRestore(RenderNode* node)

                : mSavedLightCenter(LightingInfo::getLightCenterRaw()) {

            // TODO: put localized light center calculation and storage to a drawable related code.

            // It does not seem right to store something localized in a global state

            // fix here and in recordLayers

            Vector3 transformedLightCenter(mSavedLightCenter);

            // map current light center into RenderNode's coordinate space

            node->getSkiaLayer()->inverseTransformInWindow.mapPoint3d(transformedLightCenter);

            LightingInfo::setLightCenterRaw(transformedLightCenter);

        }



        ~AutoLightingInfoRestore() { LightingInfo::setLightCenterRaw(mSavedLightCenter); }



    private:

        Vector3 mSavedLightCenter;

    };



    /**

     * mCapturedFile - the filename to write a recorded SKP to in either MultiFrameSKP or

     * SingleFrameSKP mode.