1008349 - [B2G][Camera][Zoom][Open C]zoom level in preview is higher than the zoom level of the picture taken.

Status: RESOLVED FIXED

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

Description

[Ross Kuhlman ([e-mail])]

Attachment: ZoomComparison.jpg

The zoom level in preview is higher than the zoom level of the picture taken. when fully zoomed in, the image on the preview screen is only a small portion of the image taken.

Repro Steps:
1) Update a Open_C to BuildID: 20140509000203
2) Launch camera
3) zoom in as far as possible (pay attention to what is visible onscreen)
4) take a picture
5) observe the picture

Actual:
The image shown in preview is zoomed further in than the image stored in gallery

Expected:
The image shown in preview accurately represents what the picture will look like

v1.4 Environmental Variables:
Device: Open_C v1.4
BuildID: 20140509000203
Gaia: f19735d288d9bf1c6ee0c0ecc7941421365037c7
Gecko: 33aff135dc42
Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL

Notes:

Repro frequency: 100%
See attached: screenshot

Comment 1

[Natalya Kot [:nkot]]

this also reproduces on Open_C master build

Open_C master m-c
BuildID: 20140509040202
Gaia: 15ac34804eb8b3c9b2582d7cf754c57e23182df6
Gecko: cf89b5d018f8
Version: 32.0a1
P821A10V1.0.0B06_LOG_DL

'qawanted' please confirm if it repros on Buri

Comment 2

[Chris Lord [:cwiiis]]

I don't see this in the ZTE 1.3 build, should this be marked as a regression?

Comment 3

[Justin D'Arcangelo [:justindarc]]

Zoom is a new feature as of 1.4 which is why you don't see it in 1.3.

Comment 4

[Chris Lord [:cwiiis]]

ah, I completely misunderstood this bug, sorry.

Comment 5

[Joshua Mitchell (Inactive)]

This does NOT reproduce on 1.4 Buri
1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140508000201
Gaia: 4ce973ef0732b0d52cb043210db598aa176b2ce9
Gecko: 16ab7f6b18f8
Version: 30.0
Firmware Version: v1.2-device.cfg


This also does NOT reproduce on the 2.0 Buri
2 Environmental Variables:
Device: Buri 2 MOZ
BuildID: 20140508040203
Gaia: 1e0574b8f6b8a2a8d9d468878ce2b4c283fc9a84
Gecko: 2a03b34c8953
Version: 32.0a1
Firmware Version: v1.2-device.cf

Comment 6

[Jason Smith [:jsmith]]

This sounds like wrong behavior in a new 1.4 feature.

Comment 7

[Hema Koka [:hema]]

Justin, can you please investigate?

Thanks
Hema

Comment 8

[Justin D'Arcangelo [:justindarc]]

This is an easy problem to fix, the problem is the fix for it will break Hamachi, Nexus 4 and Helix (maybe others).

The issue is that on Hamachi, Nexus 4 and Helix, the preview doesn't appear to reflect the current zoom level. Because of this, we scale the preview using CSS. However, on Flame/Open C, the preview *DOES* appear to correctly reflect the current zoom level. Therefore, when we scale the preview using CSS on top of the already-zoomed preview, we end up zooming the preview twice.

I've set Bug 983930 as blocking this bug. All signs seem to point to this being a camera driver issue with Hamachi, Nexus 4 and Helix. If this turns out to be the case, we'll just apply the patch to remove our CSS scaling and let the camera driver deal with zooming the preview (the way it *should* be done anyway). If we still need to use the CSS scaling as a workaround for Hamachi, Nexus 4 and Helix, we can possibly put it behind a build-time configurable flag.

Comment 9

[Hema Koka [:hema]]

(In reply to Justin D'Arcangelo [:justindarc] from comment #8)
> This is an easy problem to fix, the problem is the fix for it will break
> Hamachi, Nexus 4 and Helix (maybe others).
> 
> The issue is that on Hamachi, Nexus 4 and Helix, the preview doesn't appear
> to reflect the current zoom level. Because of this, we scale the preview
> using CSS. However, on Flame/Open C, the preview *DOES* appear to correctly
> reflect the current zoom level. Therefore, when we scale the preview using
> CSS on top of the already-zoomed preview, we end up zooming the preview
> twice.
> 
> I've set Bug 983930 as blocking this bug. All signs seem to point to this
> being a camera driver issue with Hamachi, Nexus 4 and Helix. If this turns
> out to be the case, we'll just apply the patch to remove our CSS scaling and
> let the camera driver deal with zooming the preview (the way it *should* be
> done anyway). If we still need to use the CSS scaling as a workaround for
> Hamachi, Nexus 4 and Helix, we can possibly put it behind a build-time
> configurable flag.

Mike, Can you please confirm if it is camera driver issue with Hamachi/nexus4?

Comment 10

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Hema Koka [:hema] from comment #9)
>
> Mike, Can you please confirm if it is camera driver issue with
> Hamachi/nexus4?

Justin and I have observed the weird preview non-zooming behaviour on both Hamachi and the Nexus 4, yes.

Comment 11

[Justin D'Arcangelo [:justindarc]]

I think the only way around this issue is to have some sort of build-time configuration flag or inspect the device name on app startup to determine if we need to apply preview scale adjustment for zoom. It seems that Flame is the only device at the moment that is actually zooming the preview as expected. Even though several other devices (helix, nexus4, hamachi) do not scale the preview correctly on their own, I think we should treat them as the exception rather than the norm. Since Flame is our reference device, we should probably consider this behavior as being correct even though it is outnumbered by other devices behaving differently.

Unless anyone has an objection, I'm going to submit a patch that disables the preview scale adjustment by default unless specified otherwise in a build-time setting. Also, since we already have an ugly conditional hack in for Nexus4 at runtime, I can go ahead and flip this flag on Nexus4 devices as well so they won't need any special builds to zoom properly.

Comment 12

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Comment 13

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Comment 14

[Diego Marcos [:dmarcos]]

Comment on attachment 8426553 [details] [review]
pull-request (master)

Conditional r+ to filing a new bug to extract all the device specific checks to it's own module. The device specific values have to be factored out to a configuration/profile file.

Comment 15

[Justin D'Arcangelo [:justindarc]]

Follow-up bug to extract the device detection code out and use device profiles:

Bug 1014330 - [Camera] Add support for device profiles for configuration

Comment 16

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8426673 [details] [review]
pull-request (master)

Carrying R+ from other PR (they are the same patch)

Comment 17

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/05e0c07a3a559f00f0fb3862ac4bf215e66424ff

Comment 18

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v1.4)

Carrying over R+ from patch on master.

Comment 19

[Justin D'Arcangelo [:justindarc]]

Landed in v1.4:

https://github.com/mozilla-b2g/gaia/commit/16f7ef17feb08562aa6a3cfd6d561b2fcca652c0