974119 - [Camera][Madai] 1.4 Viewfinder size and alignment

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, enhancement)

Description

[Diego Marcos [:dmarcos]]

Attachment: Dual_Shutter_Aspect_Ratio.png

The size and alignment of the viewfinder changes depending on the selected aspect ratio.

Comment 1

[Jyothiprasad]

For Madai camera app, there is an option of 1:1 aspect ration for Preview size. But there is no image size setting that support 1:1 aspect ratio.
So, to show the preview in 1:1 aspect ratio as per the UX spec, the preview in viewfinder must be scaled to fit the 1:1 ratio.

Comment 2

[Wilson Page [:wilsonpage]]

jyothiprasad: The new implementation I'm working on will expose **all** pictureSizes that Gecko exposes. We Then aim to match the chosen pictureSize aspect ratio with a previewSize. If your device exposes a 1:1 pictureSize, it will be selectable after this patch has landed.

Comment 3

[Wilson Page [:wilsonpage]]

Attachment: pull-request (camera-new-features)

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8385237 [details] [review]
pull-request (camera-new-features)

r- because:

- it appears to disable an entire file full of tests

- it does not include any tests of the preview size selection code or of the viewport sizing and positioning code

- the pickPreviewSize() algorithm is inadequate because it does not consider the size of the preview stream, only its aspect ratio

- the updatePreview() algorithm (which is the whole point of this bug) is too opaque to understand.

Also, given that there is still not any code that uses capabilities.pictureSizes, I don't believe that the patch will actually allow the user to select a picture size and see the preview displayed in the ways illustrated in the attached image.

Given the big overlap between this PR and the one for 974337, I suggest that you combine the two into a single patch on 974337 and leave this bug open

Comment 5

[Justin D'Arcangelo [:justindarc]]

Attachment: UNIFIED PATCH: pull-request (camera-new-features)

Comment 6

[Justin D'Arcangelo [:justindarc]]

Attachment: UNIFIED PATCH: pull-request (camera-new-features)

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8386481 [details] [review]
UNIFIED PATCH: pull-request (camera-new-features)

Landed in 980599. Clearing review request and marking resolved.

Comment 8

[Hema Koka [:hema]]

I don't see explicit ux+ -- Flagging ux for reviews (amy/tif: if there are minor spec deviations, please file bugs so we can fix them -- reopen this bug if there are major issues) -- this feature has landed on camera-new-features branch only

Comment 9

[Tiffanie Shakespeare [:tif]]

Pulling the patch listed in the attachments - from the interaction side it seems fine. Viewfinder changes based on camera resolution chosen and looking at the photos in gallery, they seem to match up.

I'll let Amy comment on the visual side of things.

Comment 10

[Justin D'Arcangelo [:justindarc]]

The patch attached to this bug never landed, but instead a unified patch was landed for:

https://bugzilla.mozilla.org/show_bug.cgi?id=980599

The UX review should probably be for 980599 instead.

Comment 11

[Amy Lee [:amylee] UX]

(In reply to Hema Koka [:hema] from comment #8)
> I don't see explicit ux+ -- Flagging ux for reviews (amy/tif: if there are
> minor spec deviations, please file bugs so we can fix them -- reopen this
> bug if there are major issues) -- this feature has landed on
> camera-new-features branch only

Comment 12

[Yeojin Chung [:YeojinC]]

New test case needs to be added. There is no existing test case.

Comment 13

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added to moztrap:

https://moztrap.mozilla.org/manage/case/14395/