Merge lp:~uriboni/camera-app/reset-zoom-ui into lp:camera-app

[Ugo Riboni]

Correctly reset the zoom UI when the actual zoom level of the camera resets due to switching cameras or recording modes

Correctly reset the zoom UI when the actual zoom level of the camera resets due to switching cameras or recording modes

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:600
http://jenkins.qa.ubuntu.com/job/camera-app-ci/455/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/151
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/151/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/151
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/309/console
    ABORTED: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/968/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/809
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25237

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/455/rebuild

review: Needs Fixing (continuous-integration)

[Florian Boucault (fboucault)]

We have a binding one way.

         Binding { target: camera; property: "currentZoom"; value: zoomControl.value }

We ideally would want one the other way no? This could probably be achieved without binding loop issues with something like:

Connections {
   target: camera
   onCurrentZoomChanged: zoomControl.value = camera.currentZoom
}

review: Needs Information

[lp:~uriboni/camera-app/reset-zoom-ui]

601. By Ugo Riboni on 2015-11-19

Implement the two-way binding in a more general way

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:600
http://jenkins.qa.ubuntu.com/job/camera-app-ci/458/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/154
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/154/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/154
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/312
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/973
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/814
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25260

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/458/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:600
http://jenkins.qa.ubuntu.com/job/camera-app-ci/461/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/157
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/157
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/315
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/976
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/817
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25277

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/461/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:601
http://jenkins.qa.ubuntu.com/job/camera-app-ci/464/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/160
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/160/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/160
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/318
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/979
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/820
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25299

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/464/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:601
http://jenkins.qa.ubuntu.com/job/camera-app-ci/469/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/165
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/165
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/165/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/165
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/324
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/985
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/828
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25394

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/469/rebuild

review: Needs Fixing (continuous-integration)

[Ugo Riboni (uriboni)]

> We have a binding one way.
>
> Binding { target: camera; property: "currentZoom"; value:
> zoomControl.value }
>
> We ideally would want one the other way no? This could probably be achieved
> without binding loop issues with something like:
>
> Connections {
> target: camera
> onCurrentZoomChanged: zoomControl.value = camera.currentZoom
> }

This actually causes lots of binding loop warnings when zooming, as the value changes:

ViewFinderOverlay.qml:743:13: QML Binding: Binding loop detected for property "value"

I don't think they are a problem, but it still not nice to have them. Should I revert to my previous implementation ?

[lp:~uriboni/camera-app/reset-zoom-ui]

602. By Ugo Riboni on 2015-11-24

Revert previous commit as it was causing binding loop warnings when zooming

603. By Ugo Riboni on 2015-11-24

Reset the zoom UI to the actual level when coming back from suspend

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:603
http://jenkins.qa.ubuntu.com/job/camera-app-ci/474/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-amd64-ci/170
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/170
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-armhf-ci/170/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-vivid-i386-ci/170
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-vivid-touch/332
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-touch/993
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-vivid-armhf/837
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25477

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/474/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

=== modified file 'ViewFinderOverlay.qml'
--- ViewFinderOverlay.qml	2015-10-22 12:21:14 +0000
+++ ViewFinderOverlay.qml	2015-11-24 17:48:24 +0000
@@ -571,18 +571,25 @@
         function completeSwitch() {
             viewFinderSwitcherAnimation.restart();
             camera.switchInProgress = false;
+            zoomControl.value = camera.currentZoom;
         }
 
         function changeRecordMode() {
             if (camera.captureMode == Camera.CaptureVideo) camera.videoRecorder.stop()
             camera.captureMode = (camera.captureMode == Camera.CaptureVideo) ? Camera.CaptureStillImage : Camera.CaptureVideo
+            zoomControl.value = camera.currentZoom
+        }
+
+        Connections {
+            target: Qt.application
+            onActiveChanged: if (active) zoomControl.value = camera.currentZoom
         }
 
         Timer {
             id: shootingTimer
             repeat: true
             triggeredOnStart: true
-            
+
             property int remainingSecs: 0
 
             onTriggered: {

=== modified file 'tests/autopilot/camera_app/emulators/main_window.py'
--- tests/autopilot/camera_app/emulators/main_window.py	2015-11-20 15:01:02 +0000
+++ tests/autopilot/camera_app/emulators/main_window.py	2015-11-24 17:48:24 +0000
@@ -7,7 +7,7 @@
 
 from camera_app.emulators.panel import Panel
 from autopilot.matchers import Eventually
-from testtools.matchers import Equals
+from testtools.matchers import Equals, NotEquals
 
 
 class MainWindow(object):
@@ -163,3 +163,22 @@
             tx, ty, (tx + view_switcher.width // 2), ty, rate=1)
         viewfinder = self.get_viewfinder()
         testCase.assertThat(viewfinder.inView, Eventually(Equals(True)))
+
+    def switch_cameras(self):
+        # Swap cameras and wait for camera to settle
+        shoot_button = self.get_exposure_button()
+        swap_camera_button = self.get_swap_camera_button()
+        self.app.pointing_device.move_to_object(swap_camera_button)
+        self.app.pointing_device.click()
+        shoot_button.enabled.wait_for(True)
+
+    def switch_recording_mode(self):
+        record_control = self.get_record_control()
+
+        # Wait for the camera overlay to be loaded
+        record_control.enabled.wait_for(True)
+        record_control.width.wait_for(NotEquals(0))
+        record_control.height.wait_for(NotEquals(0))
+
+        self.app.pointing_device.move_to_object(record_control)
+        self.app.pointing_device.click()

=== modified file 'tests/autopilot/camera_app/tests/test_capture.py'
--- tests/autopilot/camera_app/tests/test_capture.py	2015-10-05 13:14:12 +0000
+++ tests/autopilot/camera_app/tests/test_capture.py	2015-11-24 17:48:24 +0000
@@ -90,13 +90,11 @@
 
         """
         # Get all the elements
-        record_control = self.main_window.get_record_control()
         stop_watch = self.main_window.get_stop_watch()
         exposure_button = self.main_window.get_exposure_button()
 
         # Click the record button to toggle photo/video mode
-        self.pointing_device.move_to_object(record_control)
-        self.pointing_device.click()
+        self.main_window.switch_recording_mode()
 
         # Before recording the stop watch should read zero recording time
         # and not be visible anyway.
@@ -137,8 +135,7 @@
         # Now stop the video and go back to picture mode and check if
         # everything resets itself to previous states
         self.pointing_device.click()
-        self.pointing_device.move_to_object(record_control)
-        self.pointing_device.click()
+        self.main_window.switch_recording_mode()
 
         self.assertThat(stop_watch.opacity, Eventually(Equals(0.0)))
 
@@ -268,7 +265,7 @@
     """Test recording videos at a set resolution and switching cameras"""
     def test_video_resolution_setting_switching_cameras(self):
         # switch to video recording and empty video folder
-        self.switch_to_video_recording()
+        self.main_window.switch_recording_mode()
         self.delete_all_videos()
 
         # select the first resolution for the current camera
@@ -277,14 +274,14 @@
         self.set_video_resolution(initial_resolution)
 
         # switch cameras and select the last resolution for the current camera
-        self.switch_cameras()
+        self.main_window.switch_cameras()
         resolutions = self.get_available_video_resolutions()
         expected_resolution = resolutions[-1]
         self.assertThat(expected_resolution, NotEquals(initial_resolution))
         self.set_video_resolution(expected_resolution)
 
         # switch back to the initial camera and record a video
-        self.switch_cameras()
+        self.main_window.switch_cameras()
         self.record_video(2)
         video_file = self.get_first_video()
         height = self.read_video_height(video_file)
@@ -292,17 +289,9 @@
             expected_resolution)
         self.assertThat(height, Equals(expected_height))
 
-    def switch_cameras(self):
-        # Swap cameras and wait for camera to settle
-        shoot_button = self.main_window.get_exposure_button()
-        swap_camera_button = self.main_window.get_swap_camera_button()
-        self.pointing_device.move_to_object(swap_camera_button)
-        self.pointing_device.click()
-        self.assertThat(shoot_button.enabled, Eventually(Equals(True)))
-
     """Test recording videos at various resolutions"""
     def test_video_resolution_setting(self):
-        self.switch_to_video_recording()
+        self.main_window.switch_recording_mode()
         resolutions = self.get_available_video_resolutions()
 
         for resolution_label in resolutions:
@@ -316,16 +305,6 @@
             self.assertThat(height, Equals(expected_height))
             self.dismiss_first_photo_hint()
 
-    def switch_to_video_recording(self):
-        record_control = self.main_window.get_record_control()
-        # Wait for the camera overlay to be loaded
-        self.assertThat(record_control.enabled, Eventually(Equals(True)))
-        self.assertThat(record_control.width, Eventually(NotEquals(0)))
-        self.assertThat(record_control.height, Eventually(NotEquals(0)))
-
-        self.pointing_device.move_to_object(record_control)
-        self.pointing_device.click()
-
     def get_available_video_resolutions(self):
         # open bottom edge
         bottom_edge = self.main_window.get_bottom_edge()

=== modified file 'tests/autopilot/camera_app/tests/test_zoom.py'
--- tests/autopilot/camera_app/tests/test_zoom.py	2015-04-29 15:56:44 +0000
+++ tests/autopilot/camera_app/tests/test_zoom.py	2015-11-24 17:48:24 +0000
@@ -68,3 +68,50 @@
 
         self.pointing_device.drag(tx, ty, (tx - zoom_control.width), ty)
         self.assertThat(zoom_control.value, Eventually(Equals(1.0)))
+
+    """Tests zoom is reset to minimum on camera switch"""
+    def test_zoom_reset_on_camera_change(self):
+        zoom_control = self.main_window.get_zoom_control()
+        zoom_slider = self.main_window.get_zoom_slider()
+
+        self.activate_zoom()
+        x, y, w, h = zoom_slider.globalRect
+        tx = x + (w // 2)
+        ty = y + (h // 2)
+        self.pointing_device.drag(tx, ty, (tx + zoom_control.width), ty)
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.maximumValue)))
+
+        self.main_window.switch_cameras()
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.minimumValue)))
+
+        self.activate_zoom()
+        self.pointing_device.drag(tx, ty, (tx + zoom_control.width), ty)
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.maximumValue)))
+
+        self.main_window.switch_cameras()
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.minimumValue)))
+
+    """Tests zoom is reset to minimum on recording mode switch"""
+    def test_zoom_reset_on_recording_mode_change(self):
+        zoom_control = self.main_window.get_zoom_control()
+        zoom_slider = self.main_window.get_zoom_slider()
+
+        self.activate_zoom()
+        x, y, w, h = zoom_slider.globalRect
+        tx = x + (w // 2)
+        ty = y + (h // 2)
+        self.pointing_device.drag(tx, ty, (tx + zoom_control.width), ty)
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.maximumValue)))
+
+        self.main_window.switch_recording_mode()
+        self.assertThat(
+            zoom_control.value, Eventually(Equals(zoom_control.minimumValue)))
+
+        # Ideally we should test the same thing when switching back to photo
+        # mode, however due to http://pad.lv/1191088 zooming when recording
+        # video is disabled, so adding that test is pointless until fixed.