Closed
Bug 1022880
Opened 10 years ago
Closed 10 years ago
[Camera] Add focus state color to the continuous auto focus ring
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dmarcos, Assigned: dmarcos)
References
Details
Attachments
(1 file)
The focus ring should be green or red depending on the focus state: failed or focused
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dmarcos
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8441069 -
Flags: review?(jdarcangelo)
Assignee | ||
Comment 2•10 years ago
|
||
The continuous auto focus ring also displays red or green color depending if the focus succeeds of fails.
It also changes the focus ring animation as requested by Amy
Assignee | ||
Updated•10 years ago
|
Attachment #8441069 -
Flags: ui-review?(amlee)
Comment 3•10 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #2)
> The continuous auto focus ring also displays red or green color depending if
> the focus succeeds of fails.
>
> It also changes the focus ring animation as requested by Amy
Hi Diego,
Just a few things:
1. The rotation should be clock-wise
2. The animation feels rigid and slow. When you tap to focus, the ring appears and hangs briefly before it rotates. The animation should feel quick and responsive.
I had worked on the AF animation previously with Justin and we had got the animation to feel pretty good. I would keep to that original animation and just remove the snapback from the rotation. Justin, do you know where I can find that bug/patch?
Flags: needinfo?(jdarcangelo)
Assignee | ||
Comment 4•10 years ago
|
||
The animation is exactly the same that the one Justin implemented but rotating counter clockwise and with no snapback. The original animation always felt sluggish to me. The total duration of the animation is shorter in this patch than in Justin's original implementation.
Focusing is not an instantaneous process and the time variates depending on the scene. We cannot just play an animation of fixed duration. For continuous auto focus it hangs for a little because it's only triggered when the camera finishes focusing. As soon as the camera start focusing we display the ring. We still don't know if the camera will focus or not and when it will finish. Once the camera focuses we play the rotation and we change the color to red or green. It's important to display the ring as soon the camera starts focusing to indicate the user that she has to hold the camera still.
In the case of touch to focus we need to immediately display the ring to provide immediate feedback that the touch has triggered the focus process. The animation again only plays when the camera finishes focusing.
Flags: needinfo?(amlee)
Assignee | ||
Comment 5•10 years ago
|
||
I changed the animation to always play before focusing. The rotation still feels choppy.
Comment 6•10 years ago
|
||
(In reply to Diego Marcos [:dmarcos] from comment #4)
> The animation is exactly the same that the one Justin implemented but
> rotating counter clockwise and with no snapback. The original animation
> always felt sluggish to me. The total duration of the animation is shorter
> in this patch than in Justin's original implementation.
>
> Focusing is not an instantaneous process and the time variates depending on
> the scene. We cannot just play an animation of fixed duration. For
> continuous auto focus it hangs for a little because it's only triggered when
> the camera finishes focusing. As soon as the camera start focusing we
> display the ring. We still don't know if the camera will focus or not and
> when it will finish. Once the camera focuses we play the rotation and we
> change the color to red or green. It's important to display the ring as soon
> the camera starts focusing to indicate the user that she has to hold the
> camera still.
>
> In the case of touch to focus we need to immediately display the ring to
> provide immediate feedback that the touch has triggered the focus process.
> The animation again only plays when the camera finishes focusing.
Hi Diego
Can you make the white focus ring 40% bigger so when it rotates it should shink down to the focus state. Also pleaes make the rotation clock-wise. Is there another bug to implement the same animation for AF? I would like to see both CF and AF in action if possible. Thanks!
Flags: needinfo?(amlee)
Comment 7•10 years ago
|
||
Comment on attachment 8441069 [details]
Pull Request
Need to update tests to reflect changes made by this patch (failed on Travis/Try).
Attachment #8441069 -
Flags: review?(jdarcangelo) → review-
Flags: needinfo?(jdarcangelo)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8441069 [details]
Pull Request
I fixed the unit tests and addressed your comments
Attachment #8441069 -
Flags: review- → review?(jdarcangelo)
Comment 9•10 years ago
|
||
Comment on attachment 8441069 [details]
Pull Request
Looks good. Tests are passing on Gaia-Try.
Attachment #8441069 -
Flags: review?(jdarcangelo) → review+
Comment 10•10 years ago
|
||
Hi,
I just loaded the build and have some feedback for AF and CF:
1. The indicator currently grows and shrinks. It shouldn't grow. It should appear and shrink to focus.
2. There is still a snap back affect happening. Please remove. The indicator should appear and rotate 45 degrees.
Thanks!
Assignee | ||
Comment 11•10 years ago
|
||
The animation is exactly the same that Justin already implemented in the past. I just copied the code back that Amy had approved. Anyways I agree that it can be improved and I'm happy to work on that with Amy. I created a follow up bug 1030380 just to polish the animation. This bug was about adding the focus state color to the continuous focus ring so I'm going to close it to keep us focused.
Assignee | ||
Comment 12•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•10 years ago
|
||
This patch needs to be uplifted to 2.0. It landed a few weeks ago on master but post 2.0 branching.
blocking-b2g: --- → 2.0?
Assignee | ||
Updated•10 years ago
|
blocking-b2g: 2.0? → ---
Flags: needinfo?(hkoka)
Comment 14•10 years ago
|
||
Only critical issues are being uplifted to 2.0 at this point
Flags: needinfo?(hkoka)
You need to log in
before you can comment on or make changes to this bug.
Description
•