Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add fuctions( getIntrinsics and getImageResolution ) to ARKitController #217

Merged
merged 14 commits into from
Jun 1, 2024

Conversation

Oct7
Copy link
Contributor

@Oct7 Oct7 commented Dec 1, 2023

Hello

I've added functions that get camera intrinsics or image resolution for getting some raw datas. I believe this addition could be beneficial for the community. Please find my pull request attached for your review.

Best regards,
Oct7

Copy link
Owner

@olexale olexale left a comment

Choose a reason for hiding this comment

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

First of all, thank you so much for the PR! I don't have much time to work on this project, so I'm happy to see that the community members like you drive it forward!

The PR looks good, but please reformat the code with the default Dart formatting.

As this project lacks integration tests, would please create a page for the example project that demonstrates how to use this new functionality? I use these samples for smoke testing before pushing any new version to make sure that in the new version everything works as expected.

ios/Classes/FlutterArkitView+Handlers.swift Show resolved Hide resolved
lib/src/utils/json_converters.dart Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Oct7 Oct7 left a comment

Choose a reason for hiding this comment

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

Hi there! 🥳

I'm happy to report that I've updated the code based on your feedback. Soon, I'll add new options to the captureHighResolutionFrame(or capturedImage) function in ARKit. For more details, check out the ARKit Documentation.

I've noticed that the imageResolution option previously added doesn't quite match the default snapshot resolution from the controller.snapshot. I'm addressing this for better compatibility. Also, I'll update new examples for getImageResolution, getIntrinsics, and captureHighResolutionFrame(or capturedImage) when I update captureHighResolutionFrame function.

Thanks for your support. Looking forward to sharing these updates!

@olexale
Copy link
Owner

olexale commented Dec 6, 2023

Hey! 👋
Thank you for your updates! Would you please address the last comment from my initial review and provide a sample for the added functionality? It would help others to understand how to use it + will allow us to double-check that nothing is broken when we release a new version.
Thanks in advance!

@Oct7
Copy link
Contributor Author

Oct7 commented May 29, 2024

I've been so busy with my main job that I've postponed this matter for a long time. I apologize for this. I've added the example files in the way you originally used. Please check them.
thank you :)

@olexale
Copy link
Owner

olexale commented Jun 1, 2024

Hello @Oct7,
Thanks again for your contribution! I've made a few changes to your PR. You may expect a new version on pub.dev soon.

@olexale olexale merged commit 1fca9be into olexale:master Jun 1, 2024
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.

2 participants