888882 - [Gallery] Not able to zoom an image taken from camera, when launched gallery through camera app

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title: When launched gallery through camera app , Not able to zoom images .
2. Precondition: 
3. Tester's Action:
        [User needs to switch between camera and gallery app]
        1. Capture a image from camera
        2. Goto the gallery button at the bottom left corner
	3. Zoom the image
        4. Not able to zoom in/out
        5. But images can be zoomed ,if you open image in camera filmstrip .
4. Detailed Symptom (ENG.): Zooming can be done through camera filmstrip , but not through gallery buttom at bottom left corner
5. Expected : Zooming feature is broken when user tried to zoom image through gallery button at the bottom of camera app.
6. Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: 0fccb0113eb635e1c1592e72ea0492d5a827d842

Comment 1

[(Inactive after June) George Duan [:gduan] [:喬智]]

cannot locate 0fccb0113 version , and it seems fine in build 20130630230208 on Inari.

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

Unable to reproduce it with Leo and Inari in gaia master and v1-train.

BTW, Leo, where do you find the revision 0fccb0113eb635e1c1592e72ea0492d5a827d842??

We all can't find it in github. If that's a temp version, would you mind to rebase to latest one and test it again??

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

BTW, is this your private patch?? If yes, please try a pure gaia, cloning from github master, to test again...

Comment 4

[Leo]

Hi ,

we are not able to reproduce this defect on v1 train.

Thanks,

Comment 5

[Leo]

Please find the below STR for this issue:

1. Capture a image from camera which gives the image size more than 2MB(width*height)
2. Goto the gallery button at the bottom left corner
3. Zoom the image
4. Follow steps 1 to 3 again
4. Not able to zoom in/out

Here is the analysis for this issue:

In metada_scripts.js file 
if (iw * ih > 2 * 1024 * 1024)
scanningBigImages = true;

we are making scanningBigImages = true; and we are allowing the zoom functionality when ever scanningBigImages=true case.

So if the image size is more than 2MB this issue is reproduciable because of thinking scanningBigImages=true.

So adding the below condition will fix the problem.

if (iw * ih > 2 * 1024 * 1024)
{
  if(currentOverlay === 'scanning')
    scanningBigImages = true;
}

In V1train/Master the image width*height is less than 2MB so this condition fails and works properly.

Comment 6

[Leo]

(In reply to Leo from comment #5)
> Please find the below STR for this issue:
> 
> 1. Capture a image from camera which gives the image size more than
> 2MB(width*height)
> 2. Goto the gallery button at the bottom left corner
> 3. Zoom the image
> 4. Follow steps 1 to 3 again
> 4. Not able to zoom in/out
> 
> Here is the analysis for this issue:
> 
> In metada_scripts.js file 
> if (iw * ih > 2 * 1024 * 1024)
> scanningBigImages = true;
> 
> we are making scanningBigImages = true; and we are not allowing the zoom
> functionality when ever scanningBigImages=true case.
> 
> So if the image size is more than 2MB this issue is reproduciable because of
> scanningBigImages=true.
> 
> So adding the below condition will fix the problem.
> 
> if (iw * ih > 2 * 1024 * 1024)
> {
>   if(currentOverlay === 'scanning')
>     scanningBigImages = true;
> }
> 
> In V1train/Master the image width*height is less than 2MB so this condition
> fails and works properly.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

Great Leo, I can confirm this bug. It really occurs at image large than 2MB which need to do some modification. That's a good finding. Thanks. I will take this bug.

As to the patch, I don't think it is just a bug here. It looks like that scanning overlay is not visible. According to the origin implementation, the scanningBigImages turns to false when the image is scanned. But it didn't at this case. I need to check more codes about it to give you the correct replying.

Comment 8

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: Patch for this bug

The origin implementation only sets scanningBigImages to false when start-up, storage card swapping. But once scanningBigImages is true, it is always true if there is no event like above.

scanningBigImages is using for blocking zooming in while scanning big image. So, I add the code to set scanningBigImages to false when photodb.oncreated is dispatched. That is the end of metadata parsing and thumbnail generation.

Comment 9

[Dale Harvey (:daleharvey)]

Comment on attachment 770608 [details]
Patch for this bug

This patch will break part of the functionality of scanningBigImages, I am pretty sure that when we encounter a large image during scanning I think we want to make sure this is false until the scan has ended, onCreated gets called in between scanning. 

I also think we should move the logic back into mediadb a bit more, its very confusing to have one file toggling true and another setting false.

Does that sounds good?

Comment 10

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi Dale, 

Thanks for your information about onCreated. That's my fault. I didn't notice it.

I agreed that it is very confusing to have one file toggling true and another setting false. But, I don't think it is good to move scanningBigImages to mediadb. Because, the scanningBigImages is related to metadata parsing not scanning, and mediadb tries to separate the metadata parsing out of it.

Maybe, I can collect related staffs into MetadataParser.js where knows the start and end of parsing and creating previews of big images. How about this one??

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Hi Dale, 

I think I am wrong again. It is not possible to move all scanningBigImages staffs to MetadataParser.js. The reason are:

1. MetadataParser.js only knows when the big image is parsing. It doesn't know the whole scanning process ends. But the 
2. Although MetadataParser.js is loaded by lazyloader, it can get photodb instance but it is not good to depends on photodb.

So, it is not a good idea to move to MetadataParser.js.


This bug only occurs when gallery app is opened and scanned, and a large photo(>2MB) is taken by camera app. This is because scanningBigImages becomes to true when mediadb needs MetadataParser.js to create thumbnail and preview. If mediadb dispatches scanend event, scanningBigImages becomes false. But it is already scanned and gallery is using manual scan mode.

I think the solution of this bug may be four:
1. keep scanningBigImages mechanism, and find a place to update scanningBigImages to false, like the patch but needs more fixes.
2. not to set scanningBigImages to true when mediadb is not scanning, like leo's version.
3. let mediadb dispatch scanend event when a single file is created.
4. add a flag about parsing big files to mediadb and use a callback function for MetadataParser.js to change this flag to true.

I will trying the solution 4, as mentioned by you to move logics to mediadb.

Comment 12

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: moves logics to mediadb

This version moves logics of scanningBigImages to mediadb and introduces a property named parsingBigFiles. The settings true and false are in mediadb.js. But the logic to determine if a file is big is done at MetadataParser.js.

Comment 13

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: not to set scanningBigImages to true when mediadb is not scanning

This patch is made with leo's version. It does not set the scanningBigImages flag when mediadb is not scanning.

Comment 14

[Dale Harvey (:daleharvey)]

Clearing needinfo since theres a patch, will review today, cheers

Comment 15

[Dale Harvey (:daleharvey)]

Comment on attachment 771936 [details]
moves logics to mediadb

Tested this and looks to be working good, thanks.

Comment 16

[Dale Harvey (:daleharvey)]

Merged in: https://github.com/mozilla-b2g/gaia/commit/c4b0c0a5d928bcbf76546ec63692c7a4bb5181c7

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted c4b0c0a5d928bcbf76546ec63692c7a4bb5181c7 to:
v1-train: 755441bb244ff522e6a84ea8a27616669ba98369

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: 755441bb244ff522e6a84ea8a27616669ba98369