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

Commit bb27bcd9 authored by Kevin DuBois's avatar Kevin DuBois
Browse files

SF: fix off-by one in luma mean calculation 

There was an off by one error in luma mean calculation that could
lead to segfault. Flaw was figured out in writing the 5 attached
unit tests for the function. Condition would be obscure in the wild,
but an exactly half-white, half-another-color in the sampled region
could present condition.

Fixes: 129858549
Test: libsurfaceflinger_unittest (5 new tests)

Change-Id: I920cd9cac15122178ec9258e33c9bc35b1bb9357
parent 945a7006

services/surfaceflinger/RegionSamplingThread.cpp

+2 −2
Original line number Diff line number Diff line
@@ -266,6 +266,7 @@ float getLuma(float r, float g, float b) { 
    constexpr auto rec709_blue_primary = 0.0722f;

    return rec709_red_primary * r + rec709_green_primary * g + rec709_blue_primary * b;

}

} // anonymous namespace



float sampleArea(const uint32_t* data, int32_t stride, const Rect& area) {

    std::array<int32_t, 256> brightnessBuckets = {};
@@ -286,14 +287,13 @@ float sampleArea(const uint32_t* data, int32_t stride, const Rect& area) { 


    int32_t accumulated = 0;

    size_t bucket = 0;

    while (bucket++ < brightnessBuckets.size()) {

    for (; bucket < brightnessBuckets.size(); bucket++) {

        accumulated += brightnessBuckets[bucket];

        if (accumulated > majoritySampleNum) break;

    }



    return bucket / 255.0f;

}

} // anonymous namespace



std::vector<float> RegionSamplingThread::sampleBuffer(

        const sp<GraphicBuffer>& buffer, const Point& leftTop,

services/surfaceflinger/RegionSamplingThread.h

+2 −0
Original line number Diff line number Diff line
@@ -37,6 +37,8 @@ class Scheduler; 
class SurfaceFlinger;

struct SamplingOffsetCallback;



float sampleArea(const uint32_t* data, int32_t stride, const Rect& area);



class RegionSamplingThread : public IBinder::DeathRecipient {

public:

    struct TimingTunables {

services/surfaceflinger/tests/unittests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ cc_test { 
        "SchedulerUtilsTest.cpp",

        "RefreshRateConfigsTest.cpp",

        "RefreshRateStatsTest.cpp",

        "RegionSamplingTest.cpp",

        "TimeStatsTest.cpp",

        "mock/DisplayHardware/MockComposer.cpp",

        "mock/DisplayHardware/MockDisplay.cpp",

services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp

0 → 100644
+77 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2019 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.

 */



#undef LOG_TAG

#define LOG_TAG "RegionSamplingTest"



#include <gmock/gmock.h>

#include <gtest/gtest.h>

#include <array>

#include <limits>



#include "RegionSamplingThread.h"



namespace android {



struct RegionSamplingTest : testing::Test {

public:

    static uint32_t constexpr kBlack = 0;

    static uint32_t constexpr kWhite = std::numeric_limits<uint32_t>::max();

    static int constexpr kWidth = 98;

    static int constexpr kStride = 100;

    static int constexpr kHeight = 29;

    std::array<uint32_t, kHeight * kStride> buffer;

    Rect const whole_area{0, 0, kWidth, kHeight};

};



TEST_F(RegionSamplingTest, calculate_mean_white) {

    std::fill(buffer.begin(), buffer.end(), kWhite);

    EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(1.0f));

}



TEST_F(RegionSamplingTest, calculate_mean_black) {

    std::fill(buffer.begin(), buffer.end(), kBlack);

    EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(0.0f));

}



TEST_F(RegionSamplingTest, calculate_mean_partial_region) {

    auto const halfway_down = kHeight >> 1;

    auto const half = halfway_down * kStride;

    Rect const partial_region = {whole_area.left, whole_area.top, whole_area.right,

                                 whole_area.top + halfway_down};

    std::fill(buffer.begin(), buffer.begin() + half, 0);

    std::fill(buffer.begin() + half, buffer.end(), kWhite);

    EXPECT_THAT(sampleArea(buffer.data(), kStride, partial_region), testing::FloatEq(0.0f));

}



TEST_F(RegionSamplingTest, calculate_mean_mixed_values) {

    std::generate(buffer.begin(), buffer.end(), [n = 0]() mutable {

        uint32_t const pixel = (n % std::numeric_limits<uint8_t>::max()) << ((n % 3) * CHAR_BIT);

        n++;

        return pixel;

    });

    EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatNear(0.083f, 0.01f));

}



TEST_F(RegionSamplingTest, bimodal_tiebreaker) {

    std::generate(buffer.begin(), buffer.end(),

                  [n = 0]() mutable { return (n++ % 2) ? kBlack : kWhite; });

    // presently there's no tiebreaking strategy in place, accept either of the means

    EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area),

                testing::AnyOf(testing::FloatEq(1.0), testing::FloatEq(0.0f)));

}



} // namespace android