808470 - [Gallery] scanning makes thumbnails jumpy and breaks zooming

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect, P3)

Description

[Nicolas B. Pierron [:nbp]]

STR:
1- Add 600 MB of photo on the sdcard.
2- make the sdcard readable (unplug the usb cable)
3- Open the gallery application.
-- … *While* the application is loading/indexing the new photos …
4- Pick one precise[1] photo.
-- [1] if you can, because the application constantly put another picture in front of it which makes it almost impossible to pick one photo, especially at the top when I experienced this issue.
5- Zoom on it.
-- The zoom level[2] is reset every-time a new picture is indexed/loaded.
6- If the zoom level is kept, swipe on a side
-- As the zoom level is reset the swipe is interpreted as changing picture[3].

Expectation:
May be a loading screen reporting the evolution of the indexation of the sdcard's photos, blocking the gallery application during the indexation.

Comment 1

[Alex Keybl [:akeybl]]

Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".

Comment 2

[David Flanagan [:djf]]

Nicholas,

Based on your description, I think the issues are:

 - the thumbnail display is too jumpy while scanning
 - you can't zoom while scanning

In your description I think you're saying that you can still successfully pan from image to image while the scan is in progress, correct?

I'm going to update the summary line

Comment 3

[Nicolas B. Pierron [:nbp]]

(In reply to David Flanagan [:djf] from comment #2)
>  - the thumbnail display is too jumpy while scanning

Pictures are added anywhere in the list of thumbnail, which makes it jumpy.

>  - you can't zoom while scanning

You can, but it won't stay in position, it will come back to the no-zoom level and moving the image once you are in a zoom view, will cause to change image after the gallery enforce the 0-zoom level.

> In your description I think you're saying that you can still successfully
> pan from image to image while the scan is in progress, correct?

Yes.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This is a major perceived-perf issue too, because it makes scanning feel O(n) instead of O(#-slots-in-one-screen).

Comment 5

[David Flanagan [:djf]]

I've done some performance testing.

When the gallery is first started, if there are many new images, the scan takes about .25s per image.  This includes all the time for DeviceStorage enumeration, extracting the preview image from the original, creating a thumbnail for it, and saving everything to indexeddb.

It could be that peformance work would make that faster (batching database writes maybe), but usually, the user isn't going to take more than 40 pictures without launching Gallery, so it will be unusual to have to scan for more than 10s.  Once I solve this jumpiness bug, I think this will feel fast enough.

Comment 6

[Nicolas B. Pierron [:nbp]]

(In reply to David Flanagan [:djf] from comment #5)
> […], but usually, the user isn't going to take more than 40
> pictures without launching Gallery, […]

What is this assumption based on?  I think my experience and this bug are the proof that somebody can *copy* hundreds of photos on his phone.  This is one thing I have done with all my phones, copy pictures from the old device to the new one (since 2003), and this is a bad first experience.

Comment 7

[David Flanagan [:djf]]

Nicolas,

I'm not saying I'm going to fix the bug and make scanning smoother.  I'm just saying that we don't have to spend a lot of time trying to make scanning faster. I think we're going close to as fast as we can with the CPU and memory resources we have anyway. Our typical user will not own an old smartphone with hundreds of pictures.

If you put hundreds of photos on your phone, you should expect to have to wait while they are being indexed.  But when this bug is fixed, the gallery will be usable while that scanning is going on, so I'm not going to worry about the scan speed.  That's all I meant in comment 5.

Comment 8

[David Flanagan [:djf]]

Currently MediaDB, during its scan process, waits 100ms after finding and processing each new file before reporting it, so that if it finds more new files in that time it can batch them into a single event.

This works well on desktop, but 100ms proves to be too short on unagi.

Increasing this to 500ms gives large batches on the unagi. I could limit the batch size, so that it always sent an event if it had 12 new files pending.

With some tuning of the wait interval and the batch size, I ought to be able to get something that feels responsive but is less jumpy.

The issue with jumpiness even when a single photo is displayed seems to be caused by these lines at the end of the fileCreated() callback:

  if (currentView === fullscreenView) {
    showFile(currentFileIndex);
  }

I'm not sure why I've got that there.  The currentFileIndex value may have just changed, but the displayed photo should not, so I need to find a way to avoid the call, or modify showFile() so it doesn't do anything if the requested images is the currently displayed one.

Comment 11

[David Flanagan [:djf]]

Attachment: link to patch on github

Dominic, 

Since this patch involves changes to MediaDB, I think you're the best one to review it. 

Since the music app rebuilds its entire UI when scanning finds new files, you might want to set the batch timeout to be large in the Music app to prevent unnecessarily frequent UI updates when many new songs are added.

Comment 12

[David Flanagan [:djf]]

The patch I've attached here turns out to be closely related to the patch I'm going to attach to 819182. They both add features to mediadb.js, so I'm going to combine them into one.

So I'm cancelling the review request here.  This bug will be closed when the patch on 819182 lands.

Comment 13

[David Flanagan [:djf]]

The attachment on #819182 landed in github pull request 7008, and resolves this bug.