Bug 919410 (lockscreen-canvas)

[LockScreen] Re-implement LockScreen in Canvas to solve the glitches issue

VERIFIED FIXED

Status

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: gweng, Assigned: gweng)

Tracking

(Blocks: 1 bug)

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+)

Details

(Whiteboard: [FT:System-Platform])

Attachments

(5 attachments, 2 obsolete attachments)

Created attachment 808431 [details]
On device #1

STR:

1. Lock the phone by pressing the power button
2. The lockscreen shows
3. Pull one of the handles to left/right to unlock

Excepted:

The slider will be full of the blue color.

Actual Result:

There're one or two glitches among the slider, and they will be invisible after few seconds even when user's finger is still at the same position.

Notes:

1. It's only obvious on real devices. On FirefoxNightly for Mac & Linux, the glitches gone or become very fuzzy.
2. Attachments are the screenshots.
(Assignee)

Comment 1

5 years ago
Created attachment 808432 [details]
On device #2
(Assignee)

Comment 2

5 years ago
Created attachment 808434 [details]
On desktop
(Assignee)

Updated

5 years ago
Summary: Fix the glitches among the sliders when user is pulling the handle → [LockScreen] Fix the glitches among the sliders when user is pulling the handle
(Assignee)

Comment 3

5 years ago
Well, the old hack tricks work here: using a scaling factor to scale the center area to overlap on the possible glitches eases the headache. However, there're still some sub-pixel lines show near the center if user pull the handle close to it. I shall investigate what exactly happens here.
(Assignee)

Comment 4

5 years ago
I've pushed the WIP patch to GitHub, here is the link to the branch:

https://github.com/snowmantw/gaia/tree/issue919410

But the problem wasn't solved completely. There're still some thin lines will appear while the handle is bouncing.
I will check the slider components' screen position first.
(Assignee)

Updated

5 years ago
Assignee: nobody → gweng
(Assignee)

Comment 6

5 years ago
A debug version to show the detail of the elements:

https://github.com/snowmantw/gaia/tree/issue919410
(Assignee)

Comment 7

5 years ago
Jerry: or I put the original slider beyond the half-opacity area and seperate the elements in it, then we can get rid off the debug sliders?
Flags: needinfo?(hshih)
(Assignee)

Comment 8

5 years ago
(In reply to Greg Weng [:snowmantw] from comment #7)
> Jerry: or I put the original slider beyond the half-opacity area and
> seperate the elements in it, then we can get rid off the debug sliders?

I've make a new debug version as I said and pushed to the branch above.
blocking-b2g: --- → koi+
Greg will provide bandage fix for v1.2 in bug 921360, set this bug to -'ing.
blocking-b2g: koi+ → -
(Assignee)

Comment 10

5 years ago
I'll remove some 'rem' is CSS and fix them with static pixel width, to make sure the whole sliding component can be the same at every device. Rest parts of the lockscreen can remain the 'rem' as the unit.
In git commit d34bdb0aada2f847ba6f3e9fa8457b4dbee128e2.

I print some position info for slider.

In unagi device, the rectangle and the right half circle are at:
upper left(159.599976,400.000000), lower right(190.399975,460.000000)
upper left(189.000000,400.000000), lower right(219.000000,460.000000)

In leo device, , the rectangle and the right half circle are at:
upper left(160.000000,400.000000), lower right(194.000000,460.000000)
upper left(192.000000,400.000000), lower right(222.000000,460.000000)

The component seems to be overlapped with a little pixel width and 
you will see some bright line at the overlapped area when you pull the slider bar.
Flags: needinfo?(hshih)
(Assignee)

Comment 12

5 years ago
I'm trying to use canvas to solve this problem completely. So far it works perfectly.

The jsFiddle:

http://jsfiddle.net/jxjY7/

Updated

5 years ago
See Also: → bug 919899
(Assignee)

Comment 13

5 years ago
Created attachment 832063 [details]
WIP patch

I keep legacy code to make sure there're nothing would be broken due to my patch. These code would be removed in the progress of following refactoring.

Some different points (some of them would be fixed soon):

1. The gradient color happens on sliding, and not according to the fixed timing but the distance user dragged.

2. The arrows now slide incorrectly, because they are affected by the legacy parts. I would try to pull them out and use new method to implement it.

3. The glitches had gone and the performance is still acceptable. Thanks God.

It also should work on larger devices (tested on FirefoxNightly).

BTW, I keep the commits because this is just a WIP patch. I'll squash them all when this patch can be landed.

---

Future works

1. Fix bugs
2. Move it into a standalone app at Bug 898348
3. Integrate it with incoming call mode
4. Bug 937442 -  Implement LockWindow to manage LockScreen
Attachment #832063 - Flags: feedback?(timdream)
(Assignee)

Updated

5 years ago
Alias: lockscreen-canvas
Summary: [LockScreen] Fix the glitches among the sliders when user is pulling the handle → [LockScreen] Re-implement LockScreen in Canvas to solve the glitches issue
(Assignee)

Comment 14

5 years ago
Change the title because we need to focus on the Canvas implementation.
(Assignee)

Comment 15

5 years ago
Created attachment 832778 [details] [review]
Patch

This is the Canvas version of the LockScreen. It eliminates the glitches problem.

A particular change is the unlock handle now can only be triggered by touching on the circle, which was described in the original spec at the Bug 887802.

I also improved the auto-expanding of the handle. It now uses acceleration instead of automatically pull-to-end.

The last change is the opacity now changing according to the length of sliding, not a fixed time, although their effects are almost the same. This involved some technique difficulties so far I can't solve. If it's possible, UX may review this too see if we need to fire another bug to use the old way, or the new stlye is okay.
Attachment #832063 - Attachment is obsolete: true
Attachment #832063 - Flags: feedback?(timdream)
Attachment #832778 - Flags: review?(timdream)
(Assignee)

Comment 16

5 years ago
I'm now do some adjustments about pixel ratio. So the patch will be updated with some WIP commits. I'll squash them again once the issue is resolved.
blocking-b2g: - → 1.3+
Whiteboard: [FT:System-Platform]
Comment on attachment 832778 [details] [review]
Patch

Discussed offline
Attachment #832778 - Flags: review?(timdream)
(Assignee)

Comment 18

5 years ago
Solved several pixel mapping issues today. The only one left is the image issue.
(Assignee)

Comment 19

5 years ago
Created attachment 8335809 [details] [review]
Patch

New patch. Refactored as follow:

1. Move unlocking code to new, independent unlocking strategy named LockScreenSlide.
2. The LockScreen would need to composite the strategy to unlock itself
3. There would be a intentionRouter sent by LockScreen to the unlocker, to let it can communicate with the LockScreen
4. In this way, it's possible to change or add new unlocker without modify the LockScreen itself. The LockScreen just need to be initialized with the new strategy
5. Other functions are still inside the LockScreen, because they're irrelevant to the unlocker.
Attachment #832778 - Attachment is obsolete: true
Attachment #8335809 - Flags: review?(timdream)
(Assignee)

Comment 20

5 years ago
Comment on attachment 8335809 [details] [review]
Patch

Well rebase on the newest master would make it doesn't appear even turn on the setting. I need to check it first.
Attachment #8335809 - Flags: review?(timdream)
Fabrice raised an issue with this 'bandaging' on the other bug https://bugzilla.mozilla.org/show_bug.cgi?id=898348#c18

I'd like to know the answer to that too. I'd like to know if a fix for the underlying problem was considered and if there was a discussion with the Graphics team. It might be faster/easier/cheaper to fix that.
While canvas is cool, there is no longer a DOM tree to manipulate, which defeats the point of a HTML based phone.
(Assignee)

Comment 23

5 years ago
(In reply to Nikhil Marathe [:nsm] (use needinfo?) from comment #22)
> While canvas is cool, there is no longer a DOM tree to manipulate, which
> defeats the point of a HTML based phone.

IMO if you really implement an animation, you should use Canvas, SVG and even WebGL. What happens on us is the similar question. It's not a shame to use this techniques to implement animations, but using DOMs and CSSs may be (remember those "3D" animations before WebGL?)
(Assignee)

Comment 24

5 years ago
Comment on attachment 8335809 [details] [review]
Patch

Solved the problem and sent it again.
Attachment #8335809 - Flags: review?(timdream)
(In reply to Nikhil Marathe [:nsm] (use needinfo?) from comment #22)
> While canvas is cool, there is no longer a DOM tree to manipulate, which
> defeats the point of a HTML based phone.

I don't know. Maybe 10 years from now we will stop writing JS/DOM/CSS and all websites will be run with compiled asm.js and WebGL. But I know converting our lock screen handle to canvas does not accelerate nor slow the trend, while solving our problems. :)
Update: I was caught up by 3rd-party keyboard wrapping up for v1.2 so this will have to wait for a few more day for review. Sorry about that.
Comment on attachment 8335809 [details] [review]
Patch

Everything is good except tests and API between LockScreenHandle and LockScreen.
Attachment #8335809 - Flags: review?(timdream) → feedback+
Comment on attachment 8335809 [details] [review]
Patch

We would like to have some UI and perceptional performance review on the implementation.
Attachment #8335809 - Flags: ui-review?(firefoxos-ux-bugzilla)

Comment 30

5 years ago
Comment on attachment 8335809 [details] [review]
Patch

Assigning UX review from general UX to Jaime and adding a note to make sure we all review this in our first pre-FC UX-QA Quality pass at 4 PM PST today.
Attachment #8335809 - Flags: ui-review?(firefoxos-ux-bugzilla) → ui-review?(jachen)
(Assignee)

Comment 31

5 years ago
And since I don't know if I disable the two icons from the clicking event, whether the ScreenReader would still work or not, I filed another bug to discuss this: Bug 943237
(Assignee)

Comment 32

5 years ago
Comment on attachment 8335809 [details] [review]
Patch

Add lockscreen's tests and change flag to review.
Attachment #8335809 - Flags: feedback+ → review?(timdream)
Attachment #8335809 - Flags: review?(timdream) → review+
(Assignee)

Comment 34

5 years ago
master: https://github.com/mozilla-b2g/gaia/commit/845371db449590d0ff73055ec3eb790a4467755b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 35

5 years ago
Comment on attachment 8335809 [details] [review]
Patch

We reviewed this today and the lockscreen does not work with incoming calls. Rob will post a vidyo shortly.
Attachment #8335809 - Flags: ui-review?(jachen) → ui-review-

Updated

5 years ago
Flags: needinfo?(rmacdonald)
In our review today, we discovered a number of issues with the incoming call lock screen as well as the lock screen. I will be filing bugs and meeting with Greg and Steve in the Taipei office tomorrow.

The video that Jaime references for the incoming call unlock bug is referenced in Bug 945715 comment 1.
Flags: needinfo?(rmacdonald)
Depends on: 946712

Comment 37

5 years ago
Hi, Greg,

Thanks for your help! I verified this bug.

* Test Build:
 - Gaia:     c952e2756c03eceb4de6a3eba15651741a62f9e8
 - Gecko:    http://hg.mozilla.org/mozilla-central/rev/df82be9d89a5
 - BuildID   20131210040206
 - Version   29.0a1

Attaching the verification result. FYI.
Status: RESOLVED → VERIFIED

Comment 38

5 years ago
Created attachment 8345739 [details]
Verify glitches issue.mp4
You need to log in before you can comment on or make changes to this bug.