From 575b55f2dc67ac51992410257e354f2d92f19084 Mon Sep 17 00:00:00 2001 From: wysaid Date: Sat, 28 Feb 2026 23:15:30 +0800 Subject: [PATCH 1/6] feat: add setZoomRatio API to ICameraProvider, CameraXProvider and Camera1Provider - ICameraProvider: add isZoomSupported(), getMinZoomRatio(), getMaxZoomRatio(), and setZoomRatio(float) with default no-op implementations so existing code requires no changes - CameraXProvider: implement via CameraControl.setZoomRatio() and CameraInfo.getZoomState(); always supported - Camera1Provider: implement by mapping float ratio to the nearest Camera.Parameters integer zoom index; guarded by isZoomSupported() Closes #529 --- .../org/wysaid/camera/Camera1Provider.java | 48 ++++++++++++++++++ .../org/wysaid/camera/CameraXProvider.java | 35 +++++++++++++ .../org/wysaid/camera/ICameraProvider.java | 50 +++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/library/src/main/java/org/wysaid/camera/Camera1Provider.java b/library/src/main/java/org/wysaid/camera/Camera1Provider.java index 32dfb15e..fb6f5470 100644 --- a/library/src/main/java/org/wysaid/camera/Camera1Provider.java +++ b/library/src/main/java/org/wysaid/camera/Camera1Provider.java @@ -139,6 +139,54 @@ public void setFocusMode(String focusMode) { CameraInstance.getInstance().setFocusMode(focusMode); } + // ========== Zoom ========== + + @Override + public boolean isZoomSupported() { + android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + return params != null && params.isZoomSupported(); + } + + @Override + public float getMinZoomRatio() { + return 1.0f; // Camera1 always starts at 1.0x (zoom index 0) + } + + @Override + public float getMaxZoomRatio() { + android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + if (params == null || !params.isZoomSupported()) return 1.0f; + java.util.List ratios = params.getZoomRatios(); + return ratios.get(params.getMaxZoom()) / 100.0f; + } + + @Override + public void setZoomRatio(float ratio) { + android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + if (params == null || !params.isZoomSupported()) return; + + java.util.List zoomRatios = params.getZoomRatios(); + int targetHundredths = Math.round(ratio * 100.0f); + + // Find the closest discrete zoom index for the requested ratio. + int bestIndex = 0; + int bestDiff = Integer.MAX_VALUE; + for (int i = 0; i < zoomRatios.size(); i++) { + int diff = Math.abs(zoomRatios.get(i) - targetHundredths); + if (diff < bestDiff) { + bestDiff = diff; + bestIndex = i; + } + } + + try { + params.setZoom(bestIndex); + CameraInstance.getInstance().setParams(params); + } catch (Exception e) { + Log.e(LOG_TAG, "Camera1: setZoomRatio failed: " + e.toString()); + } + } + // ========== Capture ========== /** diff --git a/library/src/main/java/org/wysaid/camera/CameraXProvider.java b/library/src/main/java/org/wysaid/camera/CameraXProvider.java index 9a833227..6abd4da1 100644 --- a/library/src/main/java/org/wysaid/camera/CameraXProvider.java +++ b/library/src/main/java/org/wysaid/camera/CameraXProvider.java @@ -10,6 +10,8 @@ import android.util.Size; import android.view.Surface; +import androidx.camera.core.ZoomState; + import androidx.camera.camera2.interop.Camera2Interop; import androidx.camera.camera2.interop.ExperimentalCamera2Interop; @@ -340,6 +342,39 @@ public void setFocusMode(String focusMode) { Log.i(LOG_TAG, "CameraX: setFocusMode ignored (CameraX manages focus automatically): " + focusMode); } + // ========== Zoom ========== + + @Override + public boolean isZoomSupported() { + return true; // CameraX always supports zoom via ZoomControl + } + + @Override + public float getMinZoomRatio() { + if (mCamera == null) return 1.0f; + ZoomState state = mCamera.getCameraInfo().getZoomState().getValue(); + return state != null ? state.getMinZoomRatio() : 1.0f; + } + + @Override + public float getMaxZoomRatio() { + if (mCamera == null) return 1.0f; + ZoomState state = mCamera.getCameraInfo().getZoomState().getValue(); + return state != null ? state.getMaxZoomRatio() : 1.0f; + } + + @Override + public void setZoomRatio(float ratio) { + if (mCamera == null) { + Log.w(LOG_TAG, "CameraX: setZoomRatio called but camera not bound."); + return; + } + mCamera.getCameraControl().setZoomRatio(ratio) + .addListener(() -> { + // Zoom applied; nothing extra needed. + }, ContextCompat.getMainExecutor(mContext)); + } + // ========== Capture ========== @Override diff --git a/library/src/main/java/org/wysaid/camera/ICameraProvider.java b/library/src/main/java/org/wysaid/camera/ICameraProvider.java index 7b772bad..0cd26337 100644 --- a/library/src/main/java/org/wysaid/camera/ICameraProvider.java +++ b/library/src/main/java/org/wysaid/camera/ICameraProvider.java @@ -205,6 +205,56 @@ default void startPreview(SurfaceTexture texture, PreviewSizeReadyCallback callb */ void setFocusMode(String focusMode); + // ========== Zoom ========== + + /** + * Whether the camera hardware supports zoom. + * + *

CameraX always returns {@code true} (handled by the framework). + * Camera1 returns the result of {@link android.hardware.Camera.Parameters#isZoomSupported()}. + * + * @return {@code true} if zoom is supported, {@code false} otherwise. + */ + default boolean isZoomSupported() { + return false; + } + + /** + * Get the minimum zoom ratio supported by the camera hardware. + * Always 1.0 (no zoom, native field of view). + * + * @return minimum zoom ratio, typically 1.0f. + */ + default float getMinZoomRatio() { + return 1.0f; + } + + /** + * Get the maximum zoom ratio supported by the camera hardware. + * + * @return maximum zoom ratio (e.g. 10.0f); 1.0f when zoom is unsupported. + */ + default float getMaxZoomRatio() { + return 1.0f; + } + + /** + * Set the zoom ratio. + * + *

CameraX applies the ratio asynchronously via {@code CameraControl.setZoomRatio()}. + * Camera1 maps the float ratio to the closest supported integer zoom index + * ({@link android.hardware.Camera.Parameters#setZoom(int)}) — slight rounding may occur + * because Camera1 only exposes discrete steps. + * + *

Values are clamped to [{@link #getMinZoomRatio()}, {@link #getMaxZoomRatio()}]. + * Calling this method when {@link #isZoomSupported()} returns {@code false} is a no-op. + * + * @param ratio The desired zoom ratio. 1.0f means no zoom. + */ + default void setZoomRatio(float ratio) { + // Default no-op; implementations override. + } + // ========== Capture ========== /** From 6568b70975e0e0cab7f19772b84b7ba119a39d21 Mon Sep 17 00:00:00 2001 From: wysaid Date: Sat, 28 Feb 2026 23:35:55 +0800 Subject: [PATCH 2/6] fix: clamp zoom ratio and handle future errors in CameraXProvider.setZoomRatio - isZoomSupported() now returns false when camera is not yet bound or ZoomState is unavailable, giving callers a reliable capability check - setZoomRatio() clamps the incoming ratio to [min, max] before passing to CameraControl, honoring the ICameraProvider contract - observe the returned ListenableFuture to log errors instead of silently discarding failures --- .../org/wysaid/camera/CameraXProvider.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/library/src/main/java/org/wysaid/camera/CameraXProvider.java b/library/src/main/java/org/wysaid/camera/CameraXProvider.java index 6abd4da1..e197659a 100644 --- a/library/src/main/java/org/wysaid/camera/CameraXProvider.java +++ b/library/src/main/java/org/wysaid/camera/CameraXProvider.java @@ -346,7 +346,9 @@ public void setFocusMode(String focusMode) { @Override public boolean isZoomSupported() { - return true; // CameraX always supports zoom via ZoomControl + if (mCamera == null) return false; + ZoomState state = mCamera.getCameraInfo().getZoomState().getValue(); + return state != null && state.getMaxZoomRatio() > state.getMinZoomRatio(); } @Override @@ -369,10 +371,36 @@ public void setZoomRatio(float ratio) { Log.w(LOG_TAG, "CameraX: setZoomRatio called but camera not bound."); return; } - mCamera.getCameraControl().setZoomRatio(ratio) - .addListener(() -> { - // Zoom applied; nothing extra needed. - }, ContextCompat.getMainExecutor(mContext)); + + ZoomState zoomState = mCamera.getCameraInfo().getZoomState().getValue(); + if (zoomState == null) { + Log.w(LOG_TAG, "CameraX: setZoomRatio called but ZoomState not yet available."); + return; + } + + float minZoom = zoomState.getMinZoomRatio(); + float maxZoom = zoomState.getMaxZoomRatio(); + float clampedRatio = Math.max(minZoom, Math.min(ratio, maxZoom)); + + if (clampedRatio != ratio) { + Log.w(LOG_TAG, "CameraX: Requested zoom ratio " + ratio + + " is out of range [" + minZoom + ", " + maxZoom + "], clamped to " + + clampedRatio + "."); + } + + final ListenableFuture future = + mCamera.getCameraControl().setZoomRatio(clampedRatio); + future.addListener(() -> { + try { + future.get(); + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + Log.e(LOG_TAG, "CameraX: setZoomRatio failed: " + (cause != null ? cause : e)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + Log.e(LOG_TAG, "CameraX: setZoomRatio interrupted.", e); + } + }, ContextCompat.getMainExecutor(mContext)); } // ========== Capture ========== From 06488b1698902aeb395bacc6dbc265c58b376605 Mon Sep 17 00:00:00 2001 From: wysaid Date: Sat, 28 Feb 2026 23:44:31 +0800 Subject: [PATCH 3/6] style: use Camera.Parameters and List instead of fully qualified names in Camera1Provider --- .../main/java/org/wysaid/camera/Camera1Provider.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/library/src/main/java/org/wysaid/camera/Camera1Provider.java b/library/src/main/java/org/wysaid/camera/Camera1Provider.java index fb6f5470..2c59ea79 100644 --- a/library/src/main/java/org/wysaid/camera/Camera1Provider.java +++ b/library/src/main/java/org/wysaid/camera/Camera1Provider.java @@ -4,6 +4,8 @@ import android.hardware.Camera; import android.util.Log; +import java.util.List; + import org.wysaid.common.Common; /** @@ -143,7 +145,7 @@ public void setFocusMode(String focusMode) { @Override public boolean isZoomSupported() { - android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = CameraInstance.getInstance().getParams(); return params != null && params.isZoomSupported(); } @@ -154,18 +156,18 @@ public float getMinZoomRatio() { @Override public float getMaxZoomRatio() { - android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = CameraInstance.getInstance().getParams(); if (params == null || !params.isZoomSupported()) return 1.0f; - java.util.List ratios = params.getZoomRatios(); + List ratios = params.getZoomRatios(); return ratios.get(params.getMaxZoom()) / 100.0f; } @Override public void setZoomRatio(float ratio) { - android.hardware.Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = CameraInstance.getInstance().getParams(); if (params == null || !params.isZoomSupported()) return; - java.util.List zoomRatios = params.getZoomRatios(); + List zoomRatios = params.getZoomRatios(); int targetHundredths = Math.round(ratio * 100.0f); // Find the closest discrete zoom index for the requested ratio. From 1f328f1d9feda1a3bd8d9d2d02e7810c80e8bc5a Mon Sep 17 00:00:00 2001 From: wysaid Date: Sat, 28 Feb 2026 23:47:17 +0800 Subject: [PATCH 4/6] =?UTF-8?q?docs:=20update=20pr-review=20policy=20?= =?UTF-8?q?=E2=80=94=20auto-fix=20safe=20P3=20issues,=20defer=20complex=20?= =?UTF-8?q?ones?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/skills/pr-review/SKILL.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/skills/pr-review/SKILL.md b/.github/skills/pr-review/SKILL.md index a80516ce..c51ab5e4 100644 --- a/.github/skills/pr-review/SKILL.md +++ b/.github/skills/pr-review/SKILL.md @@ -56,13 +56,17 @@ Before acting on any review comment or suggestion, classify it by importance: |-------|----------|--------| | **P1 — Must Fix** | Bug, security issue, broken behavior, API contract violation, CI failure | Fix immediately | | **P2 — Should Fix** | Correctness risk, meaningful maintainability improvement, clear code smell with real impact | Fix with brief justification | -| **P3 — Optional** | Style preference, minor naming nitpick, debatable design choice, "could be cleaner" | **Do NOT auto-fix** — surface to user for decision | +| **P3 — Conditional** | Style preference, minor naming, "could be cleaner" | Fix if it aligns with best practices **and** the change is safe + trivial; defer if complex or has any potential functional impact | | **P4 — Reject** | Contradicts project conventions, introduces unnecessary complexity, or is factually wrong | Reject with explanation | **Rules:** -- **Do not implement P3 suggestions automatically.** Note them in the final summary and let the user decide. +- For **P3**, apply this two-question test before touching the code: + 1. **Best practice?** — Does the change follow language/framework conventions (e.g. prefer imports over FQNs, use existing imports, standard patterns)? + 2. **Safe & trivial?** — Is the diff mechanical with zero risk of behavioral change and low effort? + - Both **yes** → fix it silently, mark thread resolved. + - Either **no** → do NOT modify code; record in the summary table and let the user decide. - **Do not add comments to code** unless the comment explains non-obvious logic that is truly necessary. Never add comments just to acknowledge a review suggestion was applied. -- When in doubt about importance, prefer the lower severity (P3/P4) and defer to the user rather than making the change. +- When the two-question test is ambiguous, prefer deferring rather than guessing. ## Procedure @@ -80,7 +84,7 @@ Before acting on any review comment or suggestion, classify it by importance: 3. **Address review comments** — apply the Review Restraint Policy to each comment: - P1/P2: implement the fix, then **immediately resolve the thread** using the GraphQL mutation above - Outdated threads: resolve them regardless of priority (no action needed, just mark resolved) - - P3: record in summary, do **not** modify code, ask user + - P3: apply the two-question test — fix + resolve if both answers are yes; otherwise record in summary and defer to user - P4: record rejection reason in summary 4. **Commit & push** — single commit covering all non-workflow fixes (workflow fixes are pushed incrementally during step 2) 5. **Final verification** — once all fixes are applied, confirm every check is green: `GH_PAGER= gh pr checks ` @@ -94,8 +98,9 @@ Before acting on any review comment or suggestion, classify it by importance: ```markdown | # | Source (comment / CI) | Issue description | Priority | Action taken | Reason if not fixed | |---|-----------------------|-------------------|----------|--------------|---------------------| -| 1 | Reviewer @xxx | e.g. rename foo() | P3 | Not fixed | Style preference, no functional impact — deferred to user | -| 2 | CI: lint | e.g. null-check | P1 | Fixed | — | +| 1 | Reviewer @xxx | use import instead of FQN | P3 | Fixed | Best practice + trivial mechanical change | +| 2 | Reviewer @xxx | rename internal var foo→bar | P3 | Not fixed | Non-standard opinion, no best-practice backing — deferred to user | +| 3 | CI: lint | null-check missing | P1 | Fixed | — | ``` All P3/P4 items that were **not** fixed must appear in this table with a clear reason, so the user can make an informed decision. From 3a0c4a38fded43c6c5459bec2a7bb24c6b4c90f7 Mon Sep 17 00:00:00 2001 From: wy Date: Sun, 1 Mar 2026 00:54:13 +0800 Subject: [PATCH 5/6] fix: add defensive guards in Camera1Provider zoom methods - getMaxZoomRatio: guard against null/empty getZoomRatios() list and use Math.min to avoid ArrayIndexOutOfBoundsException on vendor devices where list size < getMaxZoom() + 1 - setZoomRatio: reject NaN/Infinity/non-positive ratio early; guard against null/empty zoomRatios list; use long arithmetic to avoid int overflow in diff computation Addresses coderabbitai review comments (Critical + Major). --- .../java/org/wysaid/camera/Camera1Provider.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/library/src/main/java/org/wysaid/camera/Camera1Provider.java b/library/src/main/java/org/wysaid/camera/Camera1Provider.java index 2c59ea79..cd87bc27 100644 --- a/library/src/main/java/org/wysaid/camera/Camera1Provider.java +++ b/library/src/main/java/org/wysaid/camera/Camera1Provider.java @@ -159,7 +159,9 @@ public float getMaxZoomRatio() { Camera.Parameters params = CameraInstance.getInstance().getParams(); if (params == null || !params.isZoomSupported()) return 1.0f; List ratios = params.getZoomRatios(); - return ratios.get(params.getMaxZoom()) / 100.0f; + if (ratios == null || ratios.isEmpty()) return 1.0f; + int maxIndex = Math.min(params.getMaxZoom(), ratios.size() - 1); + return ratios.get(maxIndex) / 100.0f; } @Override @@ -167,14 +169,20 @@ public void setZoomRatio(float ratio) { Camera.Parameters params = CameraInstance.getInstance().getParams(); if (params == null || !params.isZoomSupported()) return; + if (!Float.isFinite(ratio) || ratio <= 0f) { + Log.e(LOG_TAG, "Camera1: invalid zoom ratio: " + ratio); + return; + } + List zoomRatios = params.getZoomRatios(); + if (zoomRatios == null || zoomRatios.isEmpty()) return; int targetHundredths = Math.round(ratio * 100.0f); // Find the closest discrete zoom index for the requested ratio. int bestIndex = 0; - int bestDiff = Integer.MAX_VALUE; + long bestDiff = Long.MAX_VALUE; for (int i = 0; i < zoomRatios.size(); i++) { - int diff = Math.abs(zoomRatios.get(i) - targetHundredths); + long diff = Math.abs((long) zoomRatios.get(i) - targetHundredths); if (diff < bestDiff) { bestDiff = diff; bestIndex = i; From df2b1b60bde3c7ea47fdd4ec14cd30a9af262e8a Mon Sep 17 00:00:00 2001 From: wy Date: Sun, 1 Mar 2026 00:58:05 +0800 Subject: [PATCH 6/6] refactor: extract getParams/getZoomParams helpers in Camera1Provider Eliminate repeated CameraInstance.getInstance().getParams() calls (5 sites) and the duplicated zoom-support guard (params == null || !isZoomSupported) by introducing two private helpers: - getParams(): shorthand wrapper, no behavioral change - getZoomParams(): returns params only when camera is open and zoom is supported; used by isZoomSupported, getMaxZoomRatio, setZoomRatio Pure structural cleanup, zero logic change. --- .../org/wysaid/camera/Camera1Provider.java | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/library/src/main/java/org/wysaid/camera/Camera1Provider.java b/library/src/main/java/org/wysaid/camera/Camera1Provider.java index cd87bc27..386f859c 100644 --- a/library/src/main/java/org/wysaid/camera/Camera1Provider.java +++ b/library/src/main/java/org/wysaid/camera/Camera1Provider.java @@ -105,7 +105,7 @@ public void focusAtPoint(float x, float y, float radius, AutoFocusCallback callb @Override public boolean setFlashMode(FlashMode mode) { - Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = getParams(); if (params == null) return false; String camera1Mode = ICameraProvider.flashModeToCamera1(mode); @@ -126,7 +126,7 @@ public boolean setFlashMode(FlashMode mode) { @Override public FlashMode getFlashMode() { - Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = getParams(); if (params == null) return null; return ICameraProvider.camera1ToFlashMode(params.getFlashMode()); } @@ -141,12 +141,30 @@ public void setFocusMode(String focusMode) { CameraInstance.getInstance().setFocusMode(focusMode); } + // ========== Private helpers ========== + + /** + * Shorthand for {@link CameraInstance#getParams()}. + * Returns {@code null} when the camera is closed. + */ + private Camera.Parameters getParams() { + return CameraInstance.getInstance().getParams(); + } + + /** + * Returns camera parameters only when the camera is open and zoom is supported; + * returns {@code null} otherwise. Use this as the single guard in all zoom methods. + */ + private Camera.Parameters getZoomParams() { + Camera.Parameters params = getParams(); + return (params != null && params.isZoomSupported()) ? params : null; + } + // ========== Zoom ========== @Override public boolean isZoomSupported() { - Camera.Parameters params = CameraInstance.getInstance().getParams(); - return params != null && params.isZoomSupported(); + return getZoomParams() != null; } @Override @@ -156,8 +174,8 @@ public float getMinZoomRatio() { @Override public float getMaxZoomRatio() { - Camera.Parameters params = CameraInstance.getInstance().getParams(); - if (params == null || !params.isZoomSupported()) return 1.0f; + Camera.Parameters params = getZoomParams(); + if (params == null) return 1.0f; List ratios = params.getZoomRatios(); if (ratios == null || ratios.isEmpty()) return 1.0f; int maxIndex = Math.min(params.getMaxZoom(), ratios.size() - 1); @@ -166,8 +184,8 @@ public float getMaxZoomRatio() { @Override public void setZoomRatio(float ratio) { - Camera.Parameters params = CameraInstance.getInstance().getParams(); - if (params == null || !params.isZoomSupported()) return; + Camera.Parameters params = getZoomParams(); + if (params == null) return; if (!Float.isFinite(ratio) || ratio <= 0f) { Log.e(LOG_TAG, "Camera1: invalid zoom ratio: " + ratio); @@ -248,7 +266,7 @@ public void takePicture(ShutterCallback shutterCallback, PictureDataCallback pic int jpegRotation = computeJpegRotation(camera1Facing); // Set rotation - Camera.Parameters params = CameraInstance.getInstance().getParams(); + Camera.Parameters params = getParams(); if (params != null) { try { params.setRotation(jpegRotation);