capawesome-team / capacitor-mlkit

⚑️ ML Kit plugins for Capacitor. Supports Android and iOS.
https://capawesome.io/plugins/mlkit/
Apache License 2.0
147 stars 46 forks source link

bug: flat orientation on ios #115

Closed CmdrDats closed 9 months ago

CmdrDats commented 11 months ago

Plugin(s)

Version

5.3.0

Platform(s)

Current behavior

(PR incoming)

For the flat orientation bug, the x/y gets flipped as if it's in landscape - Ideally it should pick out the current window orientation instead of the device orientation. I can't see a way to do that without juggling with the main UI thread?

Further, the scale for the coordinates is unknown, so it's difficult to figure out how to scale for the webview, so having a screen size would be useful

Expected behavior

coordinates that are consistent with the window orientation, and a way to scale the barcode coordinates to match the webview.

Reproduction

N/A

Steps to reproduce

SVG polygon overlay on a div with barcode coordinates from scans.

Other information

I have created a fork and cutting a PR, but creating this issue to go with it.

Capacitor doctor

πŸ’Š Capacitor Doctor πŸ’Š

Latest Dependencies:

@capacitor/cli: 5.6.0 @capacitor/core: 5.6.0 @capacitor/android: 5.6.0 @capacitor/ios: 5.6.0

Installed Dependencies:

@capacitor/cli: 5.5.1 @capacitor/core: 5.5.1 @capacitor/android: 5.5.1 @capacitor/ios: 5.5.1

[success] iOS looking great! πŸ‘Œ [success] Android looking great! πŸ‘Œ

Before submitting

robingenz commented 11 months ago

Thank you for your request. Did you take a look at the demo app? It shows you how to scale the coordinates, see https://github.com/robingenz/capacitor-mlkit-plugin-demo/blob/main/src/app/modules/barcode-scanning/barcode-scanning-modal.component.ts#L151:L175

CmdrDats commented 11 months ago

Ah! I looked around the code a bit, but didn't spot this - I remember trying to run it, but failing somehow (probably locally).. that could have saved me some time πŸ™ˆ, thanks! I didn't look hard enough at the js side because I didn't trust it as the underlying source of truth 🀣 oh well, I learnt a bunch in the process.

So it's just the device orientation bit - I'm not overly convinced by this implementation (it alters the way things work for landscape users) - do you know of a way to get the actual window orientation?

CmdrDats commented 11 months ago

I've updated the issue title and PR to remove the screen size bit

robingenz commented 11 months ago

Alright, i will take a look.

Melynt3 commented 9 months ago

Any update on this one? o/

robingenz commented 9 months ago

Not yet, i will try to take a look at it next week.

robingenz commented 9 months ago

@CmdrDats I would like to test your pr. What exactly do I have to do to reproduce the issue?

Ideally it should pick out the current window orientation instead of the device orientation.

What do I have to do so that the window orientation is different from the device orientation? Do you have a screen recording?

CmdrDats commented 9 months ago

@robingenz it's a bit tricky to get a screen recording (given that I need to record my iphone itself), but I can try and see what I can do.

However, I use the capacitor screen locking https://capacitorjs.com/docs/guides/screen-orientation to lock it into portrait mode:

window.screen.orientation.lock('portrait');

and then without the PR, when the device goes from portrait to flat the camera coordinates change as if in landscape (because flat = landscape in the old method).

The PR pulls through the window orientation (which gets locked to portrait by capacitor, in my case) and uses that instead of the device orientation, so there is no 'flat' orientation to get mixed up with, it will be the actual interface orientation.

A bit more of a pain, since it's not quite as straightforward to get to as the (technically incorrect) UIDevice.current.orientation - the BarcodeScannerViewDelegate does that work, so I decided to bubble it through, though maybe there's a simpler way?

robingenz commented 9 months ago

Thank you! I will take a look and try to reproduce it.