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

Dispose camera when navigated to another page #330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RaajeevChandran
Copy link

Description

Possible fix for #86

This pull request contains changes that disposes the camera when navigated to a new page and re-initialize the camera when popped back to the camera page.

Checklist

Before creating any Pull Request, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).

  • 📕 I read the Contributing page.
  • 🤝 I match the actual coding style.
  • ✅ I ran flutter analyze without any issues.

Breaking Change

  • 🛠 My feature contain breaking change.

If your feature break something, please detail it

@apalala-dev
Copy link
Contributor

Thanks for submitting your PR!

How can we see the behaviour difference?

I tried with the subroute_camera.dart example in the example project and don't see any difference on Android.

@RaajeevChandran
Copy link
Author

@apalala-dev You're right. The example code in the subroute_camera.dart works as intended because the pages are being replaced with Navigator.pushReplacementNamed. But if you try using Navigator.pushNamed to push the pages, the camera continues to run in the background. The same behavior could be seen when using other navigation packages like go_router and auto_router.

Here is a preview of the example code in subroute_camera.dart when Navigator.pushNamed is used,

From.Example.code.mp4

A preview from this PR

The camera is disposed when a new page is pushed and re-initialized when popping back to the camera page

From.This.PR.mp4

@apalala-dev
Copy link
Contributor

I just tried it and you're right, it disposes correctly the camera.

However, it also implies that when you get the focus back, the camera configuration is reset.
E.g.:

  1. Configure CamerAwesome to start without flash in 4/3
  2. Go to the camera page, switch to 16/9 and enable flash in the Camera UI
  3. Go to an other page (losing focus)
  4. Return to the camera page (regaining focus) : the initial configuration (1) replaces the one updated in the UI (2).

I am not sure how it could be solved without adding a lot of complexity. I believe it would need:

  • listeners to each configuration option that can change
  • save the configuration in a variable on each change
  • dispose the listeners when not needed anymore
  • use either the configurations options saved in variables or, if null, the initial configuration passed by parameter

Each time we would want to add an option, this logic would have to be respected (which might not be easy for a new contributor).

An other point to consider is that the package focus_detector seem to not be maintained anymore and other forks have been published to pub.dev.

@RaajeevChandran
Copy link
Author

While I agree with your interpretation, I see this bug/issue as a foundational pre-requisite to anyone incorporating this package in their project. Presuming that Navigator.pushNamed has more likelihood of usage than Navigator.pushReplacementNamed, the camera continuing to run in the background is an obvious red flag.

@DannyFilo
Copy link

Hello folks. Is there any news on manual dispose() method ? I don't use Navigator.pushReplacementNamed at all and I'd like to navigate between different modules in my app manually disposing Cameraawesome.

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.

3 participants