diff --git a/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp b/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp index 84da45a069..a87de7742c 100644 --- a/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp +++ b/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp @@ -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(std::round(scaledX)); + coordPairs[i+1] = static_cast(std::round(scaledY)); // Clamp to within activeArray/preCorrectionActiveArray - coordPairs[i] = static_cast(scaledX); - coordPairs[i+1] = static_cast(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; } } diff --git a/services/camera/libcameraservice/device3/ZoomRatioMapper.h b/services/camera/libcameraservice/device3/ZoomRatioMapper.h index aa3d9139ac..698f87fb22 100644 --- a/services/camera/libcameraservice/device3/ZoomRatioMapper.h +++ b/services/camera/libcameraservice/device3/ZoomRatioMapper.h @@ -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: diff --git a/services/camera/libcameraservice/tests/ZoomRatioTest.cpp b/services/camera/libcameraservice/tests/ZoomRatioTest.cpp index 300da099a3..4e949916b8 100644 --- a/services/camera/libcameraservice/tests/ZoomRatioTest.cpp +++ b/services/camera/libcameraservice/tests/ZoomRatioTest.cpp @@ -171,35 +171,35 @@ void subScaleCoordinatesTest(bool usePreCorrectArray) { std::array 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 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 expected2xCoordsClampedInc = { 0, 0, // top-left - static_cast(width) - 1, 0, // top-right - 0, static_cast(height) - 1, // bottom-left - static_cast(width) - 1, static_cast(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(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 expected2xCoordsClampedExc = { 0, 0, // top-left - static_cast(width), 0, // top-right - 0, static_cast(height), // bottom-left - static_cast(width), static_cast(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(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 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