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

Commit 9b0d9534 authored by Shuzhen Wang's avatar Shuzhen Wang
Browse files

Camera: Fix off-by-1 error for scaling crop region 

Make sure the scaling of coordinates based on zoomRatio is symmetric
relative to the center of the image. And use the center of the pixel
for calculation.

Test: testZoomRatio, testDigitalZoom, cameraservice_test
Test: vendor testing
Bug: 153959755
Change-Id: I966bd31bde5afd50462a018590c7a4fefc3eaac6
parent ec77ef69

services/camera/libcameraservice/device3/ZoomRatioMapper.cpp

+47 −34
Original line number Diff line number Diff line
@@ -243,10 +243,15 @@ status_t ZoomRatioMapper::separateZoomFromCropLocked(CameraMetadata* metadata, b 
            if (weight == 0) {

                continue;

            }

            // Top-left is inclusively clamped

            scaleCoordinates(entry.data.i32 + j, 1, zoomRatio, ClampInclusive);

            // Bottom-right is exclusively clamped

            scaleCoordinates(entry.data.i32 + j + 2, 1, zoomRatio, ClampExclusive);

            // Top left (inclusive)

            scaleCoordinates(entry.data.i32 + j, 1, zoomRatio, true /*clamp*/);

            // Bottom right (exclusive): Use adjacent inclusive pixel to

            // calculate.

            entry.data.i32[j+2] -= 1;

            entry.data.i32[j+3] -= 1;

            scaleCoordinates(entry.data.i32 + j + 2, 1, zoomRatio, true /*clamp*/);

            entry.data.i32[j+2] += 1;

            entry.data.i32[j+3] += 1;

        }

    }
@@ -258,7 +263,7 @@ status_t ZoomRatioMapper::separateZoomFromCropLocked(CameraMetadata* metadata, b 
    if (isResult) {

        for (auto pts : kResultPointsToCorrectNoClamp) {

            entry = metadata->find(pts);

            scaleCoordinates(entry.data.i32, entry.count / 2, zoomRatio, ClampOff);

            scaleCoordinates(entry.data.i32, entry.count / 2, zoomRatio, false /*clamp*/);

        }

    }
@@ -282,10 +287,15 @@ status_t ZoomRatioMapper::combineZoomAndCropLocked(CameraMetadata* metadata, boo 
            if (weight == 0) {

                continue;

            }

            // Top-left is inclusively clamped

            scaleCoordinates(entry.data.i32 + j, 1, 1.0 / zoomRatio, ClampInclusive);

            // Bottom-right is exclusively clamped

            scaleCoordinates(entry.data.i32 + j + 2, 1, 1.0 / zoomRatio, ClampExclusive);

            // Top-left (inclusive)

            scaleCoordinates(entry.data.i32 + j, 1, 1.0 / zoomRatio, true /*clamp*/);

            // Bottom-right (exclusive): Use adjacent inclusive pixel to

            // calculate.

            entry.data.i32[j+2] -= 1;

            entry.data.i32[j+3] -= 1;

            scaleCoordinates(entry.data.i32 + j + 2, 1, 1.0 / zoomRatio, true /*clamp*/);

            entry.data.i32[j+2] += 1;

            entry.data.i32[j+3] += 1;

        }

    }

    for (auto rect : kRectsToCorrect) {
@@ -295,7 +305,7 @@ status_t ZoomRatioMapper::combineZoomAndCropLocked(CameraMetadata* metadata, boo 
    if (isResult) {

        for (auto pts : kResultPointsToCorrectNoClamp) {

            entry = metadata->find(pts);

            scaleCoordinates(entry.data.i32, entry.count / 2, 1.0 / zoomRatio, ClampOff);

            scaleCoordinates(entry.data.i32, entry.count / 2, 1.0 / zoomRatio, false /*clamp*/);

        }

    }
@@ -309,28 +319,31 @@ status_t ZoomRatioMapper::combineZoomAndCropLocked(CameraMetadata* metadata, boo 
}



void ZoomRatioMapper::scaleCoordinates(int32_t* coordPairs, int coordCount,

        float scaleRatio, ClampMode clamp) {

        float scaleRatio, bool clamp) {

    // A pixel's coordinate is represented by the position of its top-left corner.

    // To avoid the rounding error, we use the coordinate for the center of the

    // pixel instead:

    // 1. First shift the coordinate system half pixel both horizontally and

    // vertically, so that [x, y] is the center of the pixel, not the top-left corner.

    // 2. Do zoom operation to scale the coordinate relative to the center of

    // the active array (shifted by 0.5 pixel as well).

    // 3. Shift the coordinate system back by directly using the pixel center

    // coordinate.

    for (int i = 0; i < coordCount * 2; i += 2) {

        float x = coordPairs[i];

        float y = coordPairs[i + 1];

        float xCentered = x - mArrayWidth / 2;

        float yCentered = y - mArrayHeight / 2;

        float xCentered = x - (mArrayWidth - 2) / 2;

        float yCentered = y - (mArrayHeight - 2) / 2;

        float scaledX = xCentered * scaleRatio;

        float scaledY = yCentered * scaleRatio;

        scaledX += mArrayWidth / 2;

        scaledY += mArrayHeight / 2;

        scaledX += (mArrayWidth - 2) / 2;

        scaledY += (mArrayHeight - 2) / 2;

        coordPairs[i] = static_cast<int32_t>(std::round(scaledX));

        coordPairs[i+1] = static_cast<int32_t>(std::round(scaledY));

        // Clamp to within activeArray/preCorrectionActiveArray

        coordPairs[i] = static_cast<int32_t>(scaledX);

        coordPairs[i+1] = static_cast<int32_t>(scaledY);

        if (clamp != ClampOff) {

            int32_t right, bottom;

            if (clamp == ClampInclusive) {

                right = mArrayWidth - 1;

                bottom = mArrayHeight - 1;

            } else {

                right = mArrayWidth;

                bottom = mArrayHeight;

            }

        if (clamp) {

            int32_t right = mArrayWidth - 1;

            int32_t bottom = mArrayHeight - 1;

            coordPairs[i] =

                    std::min(right, std::max(0, coordPairs[i]));

            coordPairs[i+1] =
@@ -343,25 +356,25 @@ void ZoomRatioMapper::scaleCoordinates(int32_t* coordPairs, int coordCount, 
void ZoomRatioMapper::scaleRects(int32_t* rects, int rectCount,

        float scaleRatio) {

    for (int i = 0; i < rectCount * 4; i += 4) {

        // Map from (l, t, width, height) to (l, t, r, b).

        // [l, t] is inclusive, and [r, b] is exclusive.

        // Map from (l, t, width, height) to (l, t, l+width-1, t+height-1),

        // where both top-left and bottom-right are inclusive.

        int32_t coords[4] = {

            rects[i],

            rects[i + 1],

            rects[i] + rects[i + 2],

            rects[i + 1] + rects[i + 3]

            rects[i] + rects[i + 2] - 1,

            rects[i + 1] + rects[i + 3] - 1

        };



        // top-left

        scaleCoordinates(coords, 1, scaleRatio, ClampInclusive);

        scaleCoordinates(coords, 1, scaleRatio, true /*clamp*/);

        // bottom-right

        scaleCoordinates(coords+2, 1, scaleRatio, ClampExclusive);

        scaleCoordinates(coords+2, 1, scaleRatio, true /*clamp*/);



        // Map back to (l, t, width, height)

        rects[i] = coords[0];

        rects[i + 1] = coords[1];

        rects[i + 2] = coords[2] - coords[0];

        rects[i + 3] = coords[3] - coords[1];

        rects[i + 2] = coords[2] - coords[0] + 1;

        rects[i + 3] = coords[3] - coords[1] + 1;

    }

}

services/camera/libcameraservice/device3/ZoomRatioMapper.h

+1 −7
Original line number Diff line number Diff line
@@ -65,14 +65,8 @@ class ZoomRatioMapper : private CoordinateMapper { 
    status_t updateCaptureResult(CameraMetadata *request, bool requestedZoomRatioIs1);



  public: // Visible for testing. Do not use concurently.

    enum ClampMode {

        ClampOff,

        ClampInclusive,

        ClampExclusive,

    };



    void scaleCoordinates(int32_t* coordPairs, int coordCount,

            float scaleRatio, ClampMode clamp);

            float scaleRatio, bool clamp);



    bool isValid() { return mIsValid; }

  private:

services/camera/libcameraservice/tests/ZoomRatioTest.cpp

+41 −40
Original line number Diff line number Diff line
@@ -171,35 +171,35 @@ void subScaleCoordinatesTest(bool usePreCorrectArray) { 


    std::array<int32_t, 16> originalCoords = {

            0, 0, // top-left

            width, 0, // top-right

            0, height, // bottom-left

            width, height, // bottom-right

            width / 2, height / 2, // center

            width / 4, height / 4, // top-left after 2x

            width / 3, height * 2 / 3, // bottom-left after 3x zoom

            width * 7 / 8, height / 2, // middle-right after 1.33x zoom

            width - 1, 0, // top-right

            0, height - 1, // bottom-left

            width - 1, height - 1, // bottom-right

            (width - 1) / 2, (height - 1) / 2, // center

            (width - 1) / 4, (height - 1) / 4, // top-left after 2x

            (width - 1) / 3, (height - 1) * 2 / 3, // bottom-left after 3x zoom

            (width - 1) * 7 / 8, (height - 1) / 2, // middle-right after 1.33x zoom

    };



    // Verify 1.0x zoom doesn't change the coordinates

    auto coords = originalCoords;

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 1.0f, ZoomRatioMapper::ClampOff);

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 1.0f, false /*clamp*/);

    for (size_t i = 0; i < coords.size(); i++) {

        EXPECT_EQ(coords[i], originalCoords[i]);

    }



    // Verify 2.0x zoom work as expected (no clamping)

    std::array<float, 16> expected2xCoords = {

            - width / 2.0f, - height / 2.0f,// top-left

            width * 3 / 2.0f, - height / 2.0f, // top-right

            - width / 2.0f, height * 3 / 2.0f, // bottom-left

            width * 3 / 2.0f, height * 3 / 2.0f, // bottom-right

            width / 2.0f, height / 2.0f, // center

            - (width - 1) / 2.0f, - (height - 1) / 2.0f,// top-left

            (width - 1) * 3 / 2.0f, - (height - 1) / 2.0f, // top-right

            - (width - 1) / 2.0f, (height - 1) * 3 / 2.0f, // bottom-left

            (width - 1) * 3 / 2.0f, (height - 1) * 3 / 2.0f, // bottom-right

            (width - 1) / 2.0f, (height - 1) / 2.0f, // center

            0, 0, // top-left after 2x

            width / 6.0f, height - height / 6.0f, // bottom-left after 3x zoom

            width + width / 4.0f, height / 2.0f, // middle-right after 1.33x zoom

            (width - 1) / 6.0f, (height - 1) * 5.0f / 6.0f, // bottom-left after 3x zoom

            (width - 1) * 5.0f / 4.0f, (height - 1) / 2.0f, // middle-right after 1.33x zoom

    };

    coords = originalCoords;

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, ZoomRatioMapper::ClampOff);

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, false /*clamp*/);

    for (size_t i = 0; i < coords.size(); i++) {

        EXPECT_LE(std::abs(coords[i] - expected2xCoords[i]), kMaxAllowedPixelError);

    }
@@ -207,16 +207,16 @@ void subScaleCoordinatesTest(bool usePreCorrectArray) { 
    // Verify 2.0x zoom work as expected (with inclusive clamping)

    std::array<float, 16> expected2xCoordsClampedInc = {

            0, 0, // top-left

            static_cast<float>(width) - 1, 0, // top-right

            0, static_cast<float>(height) - 1, // bottom-left

            static_cast<float>(width) - 1, static_cast<float>(height) - 1, // bottom-right

            width / 2.0f, height / 2.0f, // center

            width - 1.0f, 0, // top-right

            0, height - 1.0f, // bottom-left

            width - 1.0f, height - 1.0f, // bottom-right

            (width - 1) / 2.0f, (height - 1) / 2.0f, // center

            0, 0, // top-left after 2x

            width / 6.0f, height - height / 6.0f , // bottom-left after 3x zoom

            static_cast<float>(width) - 1,  height / 2.0f, // middle-right after 1.33x zoom

            (width - 1) / 6.0f, (height - 1) * 5.0f / 6.0f , // bottom-left after 3x zoom

            width - 1.0f,  (height - 1) / 2.0f, // middle-right after 1.33x zoom

    };

    coords = originalCoords;

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, ZoomRatioMapper::ClampInclusive);

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, true /*clamp*/);

    for (size_t i = 0; i < coords.size(); i++) {

        EXPECT_LE(std::abs(coords[i] - expected2xCoordsClampedInc[i]), kMaxAllowedPixelError);

    }
@@ -224,33 +224,33 @@ void subScaleCoordinatesTest(bool usePreCorrectArray) { 
    // Verify 2.0x zoom work as expected (with exclusive clamping)

    std::array<float, 16> expected2xCoordsClampedExc = {

            0, 0, // top-left

            static_cast<float>(width), 0, // top-right

            0, static_cast<float>(height), // bottom-left

            static_cast<float>(width), static_cast<float>(height), // bottom-right

            width - 1.0f, 0, // top-right

            0, height - 1.0f, // bottom-left

            width - 1.0f, height - 1.0f, // bottom-right

            width / 2.0f, height / 2.0f, // center

            0, 0, // top-left after 2x

            width / 6.0f, height - height / 6.0f , // bottom-left after 3x zoom

            static_cast<float>(width),  height / 2.0f, // middle-right after 1.33x zoom

            (width - 1) / 6.0f, (height - 1) * 5.0f / 6.0f , // bottom-left after 3x zoom

            width - 1.0f,  height / 2.0f, // middle-right after 1.33x zoom

    };

    coords = originalCoords;

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, ZoomRatioMapper::ClampExclusive);

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 2.0f, true /*clamp*/);

    for (size_t i = 0; i < coords.size(); i++) {

        EXPECT_LE(std::abs(coords[i] - expected2xCoordsClampedExc[i]), kMaxAllowedPixelError);

    }



    // Verify 0.33x zoom work as expected

    std::array<float, 16> expectedZoomOutCoords = {

            width / 3.0f, height / 3.0f, // top-left

            width * 2 / 3.0f, height / 3.0f, // top-right

            width / 3.0f, height * 2 / 3.0f, // bottom-left

            width * 2 / 3.0f, height * 2 / 3.0f, // bottom-right

            width / 2.0f, height / 2.0f, // center

            width * 5 / 12.0f, height * 5 / 12.0f, // top-left after 2x

            width * 4 / 9.0f, height * 5 / 9.0f, // bottom-left after 3x zoom-in

            width * 5 / 8.0f, height / 2.0f, // middle-right after 1.33x zoom-in

            (width - 1) / 3.0f, (height - 1) / 3.0f, // top-left

            (width - 1) * 2 / 3.0f, (height - 1) / 3.0f, // top-right

            (width - 1) / 3.0f, (height - 1) * 2 / 3.0f, // bottom-left

            (width - 1) * 2 / 3.0f, (height - 1) * 2 / 3.0f, // bottom-right

            (width - 1) / 2.0f, (height - 1) / 2.0f, // center

            (width - 1) * 5 / 12.0f, (height - 1) * 5 / 12.0f, // top-left after 2x

            (width - 1) * 4 / 9.0f, (height - 1) * 5 / 9.0f, // bottom-left after 3x zoom-in

            (width - 1) * 5 / 8.0f, (height - 1) / 2.0f, // middle-right after 1.33x zoom-in

    };

    coords = originalCoords;

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 1.0f/3, ZoomRatioMapper::ClampOff);

    mapper.scaleCoordinates(coords.data(), coords.size()/2, 1.0f/3, false /*clamp*/);

    for (size_t i = 0; i < coords.size(); i++) {

        EXPECT_LE(std::abs(coords[i] - expectedZoomOutCoords[i]), kMaxAllowedPixelError);

    }
@@ -323,7 +323,8 @@ void subCropOverZoomRangeTest(bool usePreCorrectArray) { 
    entry = metadata.find(ANDROID_SCALER_CROP_REGION);

    ASSERT_EQ(entry.count, 4U);

    for (int i = 0; i < 4; i++) {

        EXPECT_EQ(entry.data.i32[i], testDefaultCropSize[index][i]);

        EXPECT_LE(std::abs(entry.data.i32[i] - testDefaultCropSize[index][i]),

                kMaxAllowedPixelError);

    }

    entry = metadata.find(ANDROID_CONTROL_ZOOM_RATIO);

    EXPECT_NEAR(entry.data.f[0], 2.0f, kMaxAllowedRatioError);
@@ -335,7 +336,7 @@ void subCropOverZoomRangeTest(bool usePreCorrectArray) { 
    entry = metadata.find(ANDROID_SCALER_CROP_REGION);

    ASSERT_EQ(entry.count, 4U);

    for (int i = 0; i < 4; i++) {

        EXPECT_EQ(entry.data.i32[i], test2xCropRegion[index][i]);

        EXPECT_LE(std::abs(entry.data.i32[i] - test2xCropRegion[index][i]), kMaxAllowedPixelError);

    }



    // Letter boxing crop region, zoomRatio is 1.0