Skip to content

Conversation

@phlpsong
Copy link
Contributor

Summary

Add barcodeFrameSize prop to CameraView, make code scan box size customizable.

This PR may fix some issues like #380 #552 #485

How did you test this change?

Run example on Android/iOS and change the prop barcodeFrameSize.

@phlpsong phlpsong force-pushed the barcodeFrameSize_prop branch from 124c3c0 to 855b90b Compare February 24, 2025 14:08
@yalanyali
Copy link

For iOS, would this work for subsequent camera sessions as well? As in, after the first successful session, when the user quits the screen and comes back, would the barcodeFrameSize get set again using the initial prop and update the scannerFrameSize -> rectOfInterest? Did you get a chance to test this? Thank you for the contribution! @phlpsong

@phlpsong
Copy link
Contributor Author

@yalanyali Thanks for your comment. I think it works fine in your described case, but there is a issue that the rectOfInterest not change when update scanner box size in same screen(iOS). I already pushed a new commit to fix this.

@scarlac scarlac merged commit 2cd45d9 into teslamotors:master Mar 31, 2025
4 checks passed
@scarlac
Copy link
Collaborator

scarlac commented Mar 31, 2025

Looks good on the surface. I'm unsure about the math but we shall see.
Thanks @phlpsong !

@WoLewicki
Copy link
Contributor

Is there a reason you did not add the new architecture counterpart for this prop @phlpsong ? I can see you hardcoded the values in the paper interface and delegate, which is just a copy of an autogenerated one.

Maybe it is not documented well enough, I'll add the missing things to #690 since without it, new arch does not compile.

@phlpsong
Copy link
Contributor Author

phlpsong commented Apr 1, 2025

@WoLewicki Thank you for figuring this out, I think I'm not aware of the new architecture part. And which hardcode values you mean in interface and delegate? I checked but could not tell this part, the frame size has default value but not hardcoded.

It's great if you could optimize this part of doc, it will help others understand this.

@WoLewicki
Copy link
Contributor

And which hardcode values you mean in interface and delegate?

I meant you added the implementation to CKCameraManagerDelegate.java and CKCameraManagerInterface.java.

Another thing I see is that you meant those props to be Floats yeah? But here https://github.com/teslamotors/react-native-camera-kit/pull/701/files#diff-1d6c401dbe70abbe21602508bc1618804705ed5f4be6315f5b82a85b3b3e24c8R126 you cast them to int, probably an oversight?

@phlpsong
Copy link
Contributor Author

phlpsong commented Apr 1, 2025

OK, I thought we should implement the delegate and interface, I just checked the file header it's auto gen right?
Then I think I should not touch these two files.

No, I'm not declare the size to be float, in CKCameraManager I did get the width and height value like,

val width = frameSize.getInt("width")
val height = frameSize.getInt("height")

The reason of cast them to int is val frameMaxWidth = barcodeFrameSize.width * context.resources.displayMetrics.density, the density value is float.

@WoLewicki
Copy link
Contributor

No, I'm not declare the size to be float

Here: https://github.com/teslamotors/react-native-camera-kit/pull/701/files#diff-5011dc3d3aefd7b67661ffa11130868519526e31f62f630ebf6ddc2d7f714e09R243 I can see that you cast them to CGFloat values. I added necessary changes in the files, you can also check if they don't destroy any of the behaviors you would expect.

@phlpsong
Copy link
Contributor Author

phlpsong commented Apr 1, 2025

I think the main reason is followed previous defined size type, and need int value to change frame rect in Android and CGFloat for CGRect.

I checked, looks good! Thank u for figuring this out.

@scarlac
Copy link
Collaborator

scarlac commented Apr 1, 2025

I merged @WoLewicki 's related fixes as well as his original proposal. You can install from master until we make a release (I'd like to test myself before we bump the version officially).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants