Skip to content

Fix Active Area Calculation#37

Open
alexdremov wants to merge 8 commits into
mainfrom
active-area
Open

Fix Active Area Calculation#37
alexdremov wants to merge 8 commits into
mainfrom
active-area

Conversation

@alexdremov

@alexdremov alexdremov commented May 7, 2026

Copy link
Copy Markdown
Owner

Fixes #36

Copilot AI review requested due to automatic review settings May 7, 2026 17:56
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

Code Coverage Report

Overall Project 23.19% -0.17% 🍏
Files changed 0% 🍏

File Coverage
OnyxCanvasView.kt 0% -7.14% 🍏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts how the drawing “active area” (limit + exclusion rects) is calculated for Onyx/EPD devices, aiming to better match the raw hardware coordinate space (e.g., when device DPI/app-optimization scaling is enabled).

Changes:

  • Added a helper to scale logical screen rectangles into “hardware pixel” rectangles using real display metrics.
  • Updated setExclusionRects() to use global visible bounds and pass scaled (hardware-mapped) limit/exclusion rects into TouchHelper.
  • Updated touch helper setup to reuse setExclusionRects() rather than re-applying limit rect logic separately.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/src/main/java/com/alexdremov/notate/ui/OnyxCanvasView.kt Outdated
Comment thread app/src/main/java/com/alexdremov/notate/ui/OnyxCanvasView.kt Outdated
Comment thread app/src/main/java/com/alexdremov/notate/ui/OnyxCanvasView.kt Outdated
@alexdremov alexdremov added the build-apk Build release APK for PR label May 7, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

APK Build Successful!

An admin triggered a release build for this PR. You can download the generated APK here: Download APK

@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown

APK Build Successful!

An admin triggered a release build for this PR. You can download the generated APK here: Download APK

@dezwit

dezwit commented May 8, 2026

Copy link
Copy Markdown

@alexdremov

Hi! I tested this PR on Onyx BOOX Note Air 1 and unfortunately it doesn't resolve the issue on my device.

Why it still fails on Note Air 1

The scaleRectForEpd() function calculates the scale as getRealMetrics().widthPixels / displayMetrics.widthPixels. On Note Air 1 with the per-app "Screen DPI" (App Optimization) set to 350, getRealMetrics() returns the same scaled value as displayMetrics (~641), so the scale factor is 1.0 and nothing changes.

What Onyx does at 350 DPI: widthPixels = hardwareWidth / density = 1404 / 2.1875 ≈ 641. The hardware screen is still 1404px wide, but the TouchHelper receives a limit rect of 641px — covering only the left ~46% of the screen.

Proposed fix

Add a fallback that detects this mode by comparing densityDpi (configured DPI) vs xdpi (actual physical DPI of the screen). When the configured DPI significantly exceeds the physical DPI, it means Onyx is doing resolution scaling and density is the correct scale factor.

private fun scaleRectForEpd(rect: Rect): Rect {
    val wm = context.getSystemService(android.content.Context.WINDOW_SERVICE) as android.view.WindowManager
    val realMetrics = android.util.DisplayMetrics()
    @Suppress("DEPRECATION")
    wm.defaultDisplay.getRealMetrics(realMetrics)
    val lm = context.resources.displayMetrics

    var scaleX = realMetrics.widthPixels.toFloat() / lm.widthPixels
    var scaleY = realMetrics.heightPixels.toFloat() / lm.heightPixels

    if (scaleX < 1.05f && lm.xdpi > 0f && lm.densityDpi > lm.xdpi * 1.1f) {
        scaleX = lm.density
        scaleY = lm.density
    }

    if (scaleX <= 1.0f && scaleY <= 1.0f) return rect

    return Rect(
        kotlin.math.floor(rect.left * scaleX.toDouble()).toInt(),
        kotlin.math.floor(rect.top * scaleY.toDouble()).toInt(),
        kotlin.math.ceil(rect.right * scaleX.toDouble()).toInt(),
        kotlin.math.ceil(rect.bottom * scaleY.toDouble()).toInt(),
    )
}

Logic:

  • Primary (your approach): realMetrics / displayMetrics — works when they differ ✅
  • Fallback (new): when densityDpi > xdpi * 1.1, use density as scale factor
    • Note Air 1 @ 350 DPI: 350 > 227 * 1.1 = 250 → scale = 2.1875 → limit.right = 641 * 2.1875 ≈ 1402 ✅
    • Standard device: densityDpi ≈ xdpi → condition false → no change ✅

I also changed setExclusionRects() to use getGlobalVisibleRect() (consistent with exclusion rects from toolbar) and refactored setupTouchHelper() to go through setExclusionRects() as a single path — same as your PR.

Tested and working on Note Air 1. Full diff: dezwit@6274d44

@alexdremov

Copy link
Copy Markdown
Owner Author

@copilot update this PR with improvemnts proposed in #37 (comment) and dezwit@6274d44

The 1.1 difference fix is a bit weird, so I wonder if there's a cleaner approach.

Copilot AI and others added 2 commits May 8, 2026 13:42

Copilot AI commented May 8, 2026

Copy link
Copy Markdown
Contributor

@copilot update this PR with improvemnts proposed in #37 (comment) and dezwit@6274d44...

Implemented in f160355 (plus 792a426). I applied the requested improvements and replaced the deprecated defaultDisplay/getRealMetrics path with physical display mode–based scaling (DisplayManager + Display.mode.physicalWidth/physicalHeight), so the 1.1-threshold workaround is no longer needed. Exclusion/limit rect mapping still uses outward rounding and getGlobalVisibleRect().

No UI-visible changes in this update (logic-only), so there’s no UI screenshot to attach.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED --add-opens=java.base/java.time=ALL-UNNAMED -Xmx4096m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

APK Build Successful!

An admin triggered a release build for this PR. You can download the generated APK here: Download APK

@alexdremov

Copy link
Copy Markdown
Owner Author

APK Build Successful!

An admin triggered a release build for this PR. You can download the generated APK here: Download APK

Thanks for the debugging, @dezwit! A subtle bug indeed. I (copilot) have incorporated fixes into this PR. Please try this APK. I have tested that this does not introduce any problems on my device.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/workflows/release.yml:46

  • The Decode Keystore step will attempt to base64-decode even when KEYSTORE_BASE64 isn’t available (e.g., fork PRs), which typically fails the job. If you keep PR builds, make this step conditional on the secret being present and skip signing/build an alternative artifact when it isn’t.
    # Decode the Keystore from Secrets
    - name: Decode Keystore
      env:
        KEYSTORE_BASE64: ${{ secrets.KEYSTORE_BASE64 }}
      run: |
        echo "$KEYSTORE_BASE64" | base64 --decode > release.keystore

        # Create keystore.properties
        echo "storeFile=../release.keystore" > keystore.properties
        echo "keyAlias=${{ secrets.KEY_ALIAS }}" >> keystore.properties
        echo "storePassword=${{ secrets.STORE_PASSWORD }}" >> keystore.properties
        echo "keyPassword=${{ secrets.KEY_PASSWORD }}" >> keystore.properties

Comment thread .github/workflows/release.yml Outdated
Comment thread .github/workflows/release.yml
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

APK Build Successful!

An admin triggered a release build for this PR. You can download the generated APK here: Download APK

@dezwit

dezwit commented May 8, 2026

Copy link
Copy Markdown

Unfortunately, this version of the app doesn't solve the problem for me. I'll try to find a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build-apk Build release APK for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Right-side strokes not visible until viewport interaction

4 participants