Unity 8

Merge lp:~dandrader/unity8/multiMonitorScreenshot into lp:unity8

multiMonitorScreenshot
Merge into trunk

Proposed by Daniel d'Andrada on 2016-04-26

Status:

Merged

Approved by:

Lukáš Tinkl on 2016-04-29

Approved revision:

2343

Merged at revision:

2376

Proposed branch:

lp:~dandrader/unity8/multiMonitorScreenshot

Merge into:

lp:unity8

Diff against target:

693 lines (+105/-282)

19 files modified

debian/unity8-private.install (+1/-1)
plugins/CMakeLists.txt (+1/-1)
plugins/ScreenGrabber/ScreenGrabber.qmltypes (+0/-17)
plugins/ScreenshotDirectory/CMakeLists.txt (+5/-9)
plugins/ScreenshotDirectory/ScreenshotDirectory.cpp (+18/-69)
plugins/ScreenshotDirectory/ScreenshotDirectory.h (+10/-20)
plugins/ScreenshotDirectory/plugin.cpp (+5/-7)
plugins/ScreenshotDirectory/plugin.h (+5/-7)
plugins/ScreenshotDirectory/qmldir (+2/-3)
qml/Components/ItemGrabber.qml (+27/-17)
qml/Components/NotificationAudio.qml (+7/-3)
qml/Shell.qml (+11/-5)
qml/Stages/AbstractStage.qml (+1/-0)
qml/Stages/DesktopStage.qml (+6/-0)
src/ShellApplication.cpp (+6/-0)
tests/plugins/CMakeLists.txt (+0/-1)
tests/plugins/ScreenGrabber/CMakeLists.txt (+0/-14)
tests/plugins/ScreenGrabber/ScreenGrabberTest.cpp (+0/-77)
tests/plugins/ScreenGrabber/grabber.qml (+0/-31)

To merge this branch:

bzr merge lp:~dandrader/unity8/multiMonitorScreenshot

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Lukáš Tinkl (community)		2016-04-26	Approve on 2016-04-29
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-04-27
Review via email: mp+292961@code.launchpad.net

Commit Message

Refactor screenshotting code and make it work in multimonitor scenarios

+ Workaround bug in QWindow regrading "visible" property when moving a window between screens.
+ Implement snapshotting the focused window with Alt+PrtScn

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes.
Tested on a laptop connected to an external monitor. Can't test on a phone connected to a monitor as my Slimport dongle seens to be busted.

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable.

* If you changed the UI, has there been a design review?
Not applicable.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-26:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1068/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-26:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1071/rebuild

review: Needs Fixing (continuous-integration)

Gerry Boland (gerboland) wrote on 2016-04-27:

+++ qml/Components/ItemGrabber.qml
+ root.parent.grabToImage
Breaking encapsulation?

Lukáš Tinkl (lukas-kde) wrote on 2016-04-27:

Since you removed the previous plugin's CPP test, could you add a new one that checks whether a screenshot is taken when you press

a) both Volume keys
b) the dedicated PrtScr key

Thanks, otherwise the code looks alright to me (minus the issue pointed out by Gerry above)

Daniel d'Andrada (dandrader) wrote on 2016-04-27:

On 27/04/2016 04:54, Gerry Boland wrote:
> +++ qml/Components/ItemGrabber.qml
> + root.parent.grabToImage
> Breaking encapsulation?

I didn't put a "property Item target" parameter because ItemGrabber not
only grabs an image and saves it, it also shows an effect (the white
flash) over its parent it even has an "anchors.fill: parent" so it's
meant to capture its parent.

I don't mind much adding a target parameter and have Shell do the
anchoring but just feels like a flexibility that doesn't make sense (no
use case).

Documented this component to make intent and design clear.

Lukáš Tinkl (lukas-kde) wrote on 2016-04-27:

Tested this with an external screen, works great. Still would like to see some test :)

review: Approve

Daniel d'Andrada (dandrader) wrote on 2016-04-27:

On 27/04/2016 05:30, Lukáš Tinkl wrote:
> Since you removed the previous plugin's CPP test, could you add a new one that checks whether a screenshot is taken when you press
>
> a) both Volume keys
> b) the dedicated PrtScr key
>

You're asking for tests different from the ones I removed.

This is what I removed:

- testGrabScreenshot(): tests that a screenshot is saved to disk.

The screenshot-taking part is 100% done by Qt. I don't see any sense in
testing that. What's left is checking if
ScreenshotDirectory::makeFileName() (which is the remaining C++ code
from the old ScreenGrabber) generates valid file names.
I see little value in such test as it's in the same abstraction level as
the code it's testing, which also just a couple of lines long.

- testRotatedScreenshot(): tests that a screenshot taken when Shell is
rotated by OrientedShell is still upright.

Doesn't make sense as there's no code for that anymore since screenshots
are now being taken of the Shell component (at Item level) instead of
the QQuickWindow (at window level).

The new tests you're asking for are merely testing signal-slot connections:

a)
"""
onScreenshotTriggered: itemGrabber.capture();
"""

b)
"""
onTriggered: capture()
"""

In my opinition they would make for pretty useless tests. Useful tests
check emerging behavior or code interactions that are not obvious by
just looking at the code. They save the developer for unwittingly
breaking code or behavior. They shouldn't be a one-to-one mapping of the
code being tested.

I don't see anything test-worthy in ItemGrabber or ScreenshotDirectory.
There's little code there and it's all pretty simple and isolated.

Unless you mean high-level functional tests that press the volume keys
and check if a file lands on the correct dir, which lies out of scope of
this change (it exercises *way* more than the code I'm changing). And,
besides, I have my issues with high-level functional tests, but that's a
whole different topic.

Lukáš Tinkl (lukas-kde) wrote on 2016-04-27:

It wouldn't be much work:

QStandardPaths::setTestModeEnabled(true);
QString filename = ScreenshotDirectory::makeFileName();
QFileSystemWatcher::addPath(filename);

then check output of:
QFileSystemWatcher::fileChanged(filename);

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-27:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1076/rebuild

review: Needs Fixing (continuous-integration)

Nick Dedekind (nick-dedekind) wrote on 2016-04-28:

> On 27/04/2016 04:54, Gerry Boland wrote:
> > +++ qml/Components/ItemGrabber.qml
> > + root.parent.grabToImage
> > Breaking encapsulation?
>
> I didn't put a "property Item target" parameter because ItemGrabber not
> only grabs an image and saves it, it also shows an effect (the white
> flash) over its parent it even has an "anchors.fill: parent" so it's
> meant to capture its parent.
>
> I don't mind much adding a target parameter and have Shell do the
> anchoring but just feels like a flexibility that doesn't make sense (no
> use case).
>
> Documented this component to make intent and design clear.

I believe there is a use case where we would screenshot the focused surface on desktop.

Nick Dedekind (nick-dedekind) wrote on 2016-04-28:

> > On 27/04/2016 04:54, Gerry Boland wrote:
> > > +++ qml/Components/ItemGrabber.qml
> > > + root.parent.grabToImage
> > > Breaking encapsulation?
> >
> > I didn't put a "property Item target" parameter because ItemGrabber not
> > only grabs an image and saves it, it also shows an effect (the white
> > flash) over its parent it even has an "anchors.fill: parent" so it's
> > meant to capture its parent.
> >
> > I don't mind much adding a target parameter and have Shell do the
> > anchoring but just feels like a flexibility that doesn't make sense (no
> > use case).
> >
> > Documented this component to make intent and design clear.
>
> I believe there is a use case where we would screenshot the focused surface on
> desktop.

Alt-PrtSc
I don't think we a ItemGrabber on every surface...

Lukáš Tinkl (lukas-kde) wrote on 2016-04-28:

See here: https://code.launchpad.net/~lukas-kde/unity8/screenshotWindow/+merge/292376

lp:~dandrader/unity8/multiMonitorScreenshot updated on 2016-04-28

2342. By Daniel d'Andrada on 2016-04-28

Refactor screenshotting code and make it work in multimonitor scenarios

+ Implement snapshotting the focused window with Alt+PrtScn
+ Workaround bug in QWindow regarding visibility when moving between screens

Daniel d'Andrada (dandrader) wrote on 2016-04-28:

> > > On 27/04/2016 04:54, Gerry Boland wrote:
> > > > +++ qml/Components/ItemGrabber.qml
> > > > + root.parent.grabToImage
> > > > Breaking encapsulation?
> > >
> > > I didn't put a "property Item target" parameter because ItemGrabber not
> > > only grabs an image and saves it, it also shows an effect (the white
> > > flash) over its parent it even has an "anchors.fill: parent" so it's
> > > meant to capture its parent.
> > >
> > > I don't mind much adding a target parameter and have Shell do the
> > > anchoring but just feels like a flexibility that doesn't make sense (no
> > > use case).
> > >
> > > Documented this component to make intent and design clear.
> >
> > I believe there is a use case where we would screenshot the focused surface
> on
> > desktop.
>
> Alt-PrtSc
> I don't think we a ItemGrabber on every surface...

Right. Implemented Alt+PrtSc support. Thanks!

Daniel d'Andrada (dandrader) wrote on 2016-04-28:

Right. Implemented Alt+PrtSc support. Thanks!

lp:~dandrader/unity8/multiMonitorScreenshot updated on 2016-04-29

2343. By Daniel d'Andrada on 2016-04-29: s/Ctrl/Alt

Lukáš Tinkl (lukas-kde) wrote on 2016-04-29:

All good, tested on mako (both internal and external screen), no issues spotted.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Daniel d'Andrada

Mohammad Rakibul Hasan

Unity Team

 === modified file 'debian/unity8-private.install'
 --- debian/unity8-private.install	2016-04-04 13:38:56 +0000
 +++ debian/unity8-private.install	2016-04-29 12:42:40 +0000
@@ -8,7 +8,7 @@
  usr/lib/*/unity8/qml/IntegratedLightDM
  usr/lib/*/unity8/qml/Lights
  usr/lib/*/unity8/qml/Powerd
--usr/lib/*/unity8/qml/ScreenGrabber
++usr/lib/*/unity8/qml/ScreenshotDirectory
  usr/lib/*/unity8/qml/SessionBroadcast
  usr/lib/*/unity8/qml/Ubuntu
  usr/lib/*/unity8/qml/Unity
 === modified file 'plugins/CMakeLists.txt'
 --- plugins/CMakeLists.txt	2016-04-04 13:37:49 +0000
 +++ plugins/CMakeLists.txt	2016-04-29 12:42:40 +0000
@@ -19,8 +19,8 @@
  add_subdirectory(Lights)
  add_subdirectory(Dash)
  add_subdirectory(Powerd)
++add_subdirectory(ScreenshotDirectory)
  add_subdirectory(SessionBroadcast)
--add_subdirectory(ScreenGrabber)
  add_subdirectory(Ubuntu)
  add_subdirectory(UInput)
  add_subdirectory(Unity)
 === removed file 'plugins/ScreenGrabber/ScreenGrabber.qmltypes'
 --- plugins/ScreenGrabber/ScreenGrabber.qmltypes	2015-02-13 09:01:16 +0000
 +++ plugins/ScreenGrabber/ScreenGrabber.qmltypes	1970-01-01 00:00:00 +0000
@@ -1,17 +0,0 @@
--import QtQuick.tooling 1.1
--
--// This file describes the plugin-supplied types contained in the library.
--// It is used for QML tooling purposes only.
--//
--// This file was auto-generated by:
--// 'qmlplugindump -notrelocatable ScreenGrabber 0.1 plugins'
--
--Module {
--    Component {
--        name: "ScreenGrabber"
--        prototype: "QObject"
--        exports: ["ScreenGrabber/ScreenGrabber 0.1"]
--        exportMetaObjectRevisions: [0]
--        Method { name: "captureAndSave" }
--    }
--}
 === renamed directory 'plugins/ScreenGrabber' => 'plugins/ScreenshotDirectory'
 === modified file 'plugins/ScreenshotDirectory/CMakeLists.txt'
 --- plugins/ScreenGrabber/CMakeLists.txt	2014-09-27 02:29:36 +0000
 +++ plugins/ScreenshotDirectory/CMakeLists.txt	2016-04-29 12:42:40 +0000
@@ -1,13 +1,9 @@
--include_directories(
--  ${CMAKE_CURRENT_BINARY_DIR}
--)
--
--set(SCREENGRABBERSOURCES
--  screengrabber.cpp
++set(PLUGIN_SOURCES
++  ScreenshotDirectory.cpp
    plugin.cpp
+ )
--add_library(ScreenGrabber-qml MODULE ${SCREENGRABBERSOURCES})
--qt5_use_modules(ScreenGrabber-qml Qml Gui Quick Concurrent)
++add_library(ScreenshotDirectory-qml MODULE ${PLUGIN_SOURCES})
++qt5_use_modules(ScreenshotDirectory-qml Qml Gui Quick)
--add_unity8_plugin(ScreenGrabber 0.1 ScreenGrabber TARGETS ScreenGrabber-qml)
++add_unity8_plugin(ScreenshotDirectory 0.1 ScreenshotDirectory TARGETS ScreenshotDirectory-qml)
 === renamed file 'plugins/ScreenGrabber/screengrabber.cpp' => 'plugins/ScreenshotDirectory/ScreenshotDirectory.cpp'
 --- plugins/ScreenGrabber/screengrabber.cpp	2015-11-30 13:27:20 +0000
 +++ plugins/ScreenshotDirectory/ScreenshotDirectory.cpp	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014-2015 Canonical, Ltd.
++ * Copyright (C) 2014-2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -14,36 +14,18 @@
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
   */
--#include "screengrabber.h"
++#include "ScreenshotDirectory.h"
  #include <QDir>
  #include <QDateTime>
  #include <QStandardPaths>
  #include <QTemporaryDir>
--#include <QtGui/QImage>
--#include <QtGui/QGuiApplication>
--#include <QtQuick/QQuickWindow>
  #include <QDebug>
--QString saveScreenshot(const QImage &screenshot, const QString &filename, const QString &format, int quality)
--{
--    if (screenshot.save(filename, format.toLatin1().data(), quality)) {
--        return filename;
--    } else {
--        qWarning() << "ScreenGrabber: failed to save snapshot!";
--    }
--    return QString();
--}
--
--ScreenGrabber::ScreenGrabber(QObject *parent)
++ScreenshotDirectory::ScreenshotDirectory(QObject *parent)
      : QObject(parent)
+ {
--    QObject::connect(&m_watcher,
--                     &QFutureWatcher<QString>::finished,
--                     this,
--                     &ScreenGrabber::onScreenshotSaved);
--
      QDir screenshotsDir;
      if (qEnvironmentVariableIsSet("UNITY_TESTING")) {
          QTemporaryDir tDir;
@@ -55,60 +37,27 @@
      screenshotsDir.mkpath(QStringLiteral("Screenshots"));
      screenshotsDir.cd(QStringLiteral("Screenshots"));
      if (screenshotsDir.exists()) {
--        fileNamePrefix = screenshotsDir.absolutePath();
--        fileNamePrefix.append("/screenshot");
++        m_fileNamePrefix = screenshotsDir.absolutePath();
++        m_fileNamePrefix.append("/screenshot");
      } else {
--        qWarning() << "ScreenGrabber: failed to create directory at: " << screenshotsDir.absolutePath();
--    }
--}
--
--void ScreenGrabber::captureAndSave(int angle)
--{
--    if (fileNamePrefix.isEmpty())
--    {
--        qWarning() << "ScreenShotter: no directory to save screenshot";
--        return;
--    }
--
--    const QWindowList windows = QGuiApplication::topLevelWindows();
--    if (windows.empty())
--    {
--        qWarning() << "ScreenShotter: no top level windows found!";
--        return;
--    }
--
--    QQuickWindow *main_window = qobject_cast<QQuickWindow *>(windows[0]);
--    if (!main_window)
--    {
--        qWarning() << "ScreenShotter: can only take screenshots of QQuickWindows";
--        return;
--    }
--
--    const QImage screenshot = main_window->grabWindow().transformed(QTransform().rotate(angle));
--    const QString filename = makeFileName();
--    qDebug() << "Saving screenshot to" << filename;
--    QFuture<QString> saveFuture(QtConcurrent::run(saveScreenshot, screenshot, filename, getFormat(), screenshotQuality));
--    m_watcher.setFuture(saveFuture);
--}
--
--void ScreenGrabber::onScreenshotSaved()
--{
--    const QString filename = m_watcher.future().result();
--    if (!filename.isEmpty()) {
--        Q_EMIT screenshotSaved(filename);
--    }
--}
--
--QString ScreenGrabber::makeFileName() const
--{
--    QString fileName(fileNamePrefix);
++        qWarning() << "ScreenshotDirectory: failed to create directory at:" << screenshotsDir.absolutePath();
++    }
++}
++
++QString ScreenshotDirectory::makeFileName() const
++{
++    if (m_fileNamePrefix.isEmpty()) {
++        return QString();
++    }
++
++    QString fileName(m_fileNamePrefix);
      fileName.append(QDateTime::currentDateTime().toString(QStringLiteral("yyyyMMdd_hhmmsszzz")));
      fileName.append(".");
--    fileName.append(getFormat());
++    fileName.append(format());
      return fileName;
+ }
--QString ScreenGrabber::getFormat() const
++QString ScreenshotDirectory::format() const
+ {
      //TODO: This should be configurable (perhaps through gsettings?)
      return QStringLiteral("png");
 === renamed file 'plugins/ScreenGrabber/screengrabber.h' => 'plugins/ScreenshotDirectory/ScreenshotDirectory.h'
 --- plugins/ScreenGrabber/screengrabber.h	2015-11-18 18:38:36 +0000
 +++ plugins/ScreenshotDirectory/ScreenshotDirectory.h	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014-2015 Canonical, Ltd.
++ * Copyright (C) 2014-2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -14,36 +14,26 @@
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
   */
--#ifndef SNAPSHOTTER_H
--#define SNAPSHOTTER_H
++#ifndef SCREENSHOTDIRECTORY_H
++#define SCREENSHOTDIRECTORY_H
  #include <QObject>
  #include <QString>
--#include <QtConcurrent>
--class ScreenGrabber: public QObject
++class ScreenshotDirectory: public QObject
+ {
      Q_OBJECT
  public:
--    explicit ScreenGrabber(QObject *parent = 0);
--    ~ScreenGrabber() = default;
++    explicit ScreenshotDirectory(QObject *parent = 0);
++    ~ScreenshotDirectory() = default;
  public Q_SLOTS:
--    void captureAndSave(int angle = 0);
--
--Q_SIGNALS:
--    void screenshotSaved(const QString &filename);
--
--private Q_SLOTS:
--    void onScreenshotSaved();
++    QString makeFileName() const;
  private:
--    QString makeFileName() const;
--    QString getFormat() const;
--    QString fileNamePrefix;
--    int screenshotQuality = -1; // default quality for the format
--    QFutureWatcher<QString> m_watcher;
++    QString format() const;
++    QString m_fileNamePrefix;
  };
--#endif
++#endif // SCREENSHOTDIRECTORY_H
 === modified file 'plugins/ScreenshotDirectory/plugin.cpp'
 --- plugins/ScreenGrabber/plugin.cpp	2014-09-27 02:16:11 +0000
 +++ plugins/ScreenshotDirectory/plugin.cpp	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014 Canonical, Ltd.
++ * Copyright (C) 2014,2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -12,17 +12,15 @@
+  *
   * You should have received a copy of the GNU General Public License
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- *
-- * Authors: Alberto Aguirre <alberto.aguirre@canonical.com>
   */
  #include "plugin.h"
--#include "screengrabber.h"
++#include "ScreenshotDirectory.h"
  #include <QtQml/qqml.h>
--void ScreenGrabberPlugin::registerTypes(const char *uri)
++void ScreenshotDirectoryPlugin::registerTypes(const char *uri)
+ {
--    Q_ASSERT(uri == QLatin1String("ScreenGrabber"));
--    qmlRegisterType<ScreenGrabber>(uri, 0, 1, "ScreenGrabber");
++    Q_ASSERT(uri == QLatin1String("ScreenshotDirectory"));
++    qmlRegisterType<ScreenshotDirectory>(uri, 0, 1, "ScreenshotDirectory");
+ }
 === modified file 'plugins/ScreenshotDirectory/plugin.h'
 --- plugins/ScreenGrabber/plugin.h	2015-04-30 09:31:51 +0000
 +++ plugins/ScreenshotDirectory/plugin.h	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014 Canonical, Ltd.
++ * Copyright (C) 2014,2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -12,16 +12,14 @@
+  *
   * You should have received a copy of the GNU General Public License
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- *
-- * Authors: Alberto Aguirre <alberto.aguirre@canonical.com>
   */
--#ifndef SCREENGRABBER_PLUGIN_H
--#define SCREENGRABBER_PLUGIN_H
++#ifndef SCREENSHOTDIRECTORY_PLUGIN_H
++#define SCREENSHOTDIRECTORY_PLUGIN_H
  #include <QtQml/QQmlExtensionPlugin>
--class ScreenGrabberPlugin : public QQmlExtensionPlugin
++class ScreenshotDirectoryPlugin : public QQmlExtensionPlugin
+ {
      Q_OBJECT
      Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QQmlExtensionInterface")
@@ -30,4 +28,4 @@
      void registerTypes(const char *uri) override;
  };
--#endif // SCREENGRABBER_PLUGIN_H
++#endif // SCREENSHOTDIRECTORY_PLUGIN_H
 === modified file 'plugins/ScreenshotDirectory/qmldir'
 --- plugins/ScreenGrabber/qmldir	2014-09-27 02:16:11 +0000
 +++ plugins/ScreenshotDirectory/qmldir	2016-04-29 12:42:40 +0000
@@ -1,3 +1,2 @@
--module ScreenGrabber
--plugin ScreenGrabber-qml
--typeinfo ScreenGrabber.qmltypes
++module ScreenshotDirectory
++plugin ScreenshotDirectory-qml
 === renamed file 'qml/Components/ScreenGrabber.qml' => 'qml/Components/ItemGrabber.qml'
 --- qml/Components/ScreenGrabber.qml	2015-10-26 15:54:13 +0000
 +++ qml/Components/ItemGrabber.qml	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014-2015 Canonical, Ltd.
++ * Copyright (C) 2014-2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -15,36 +15,31 @@
   */
  import QtQuick 2.4
--import ScreenGrabber 0.1
--import GlobalShortcut 1.0
++import ScreenshotDirectory 0.1
++/*
++    Captures an image of the given item and saves it in a screenshots directory.
++    It also displays a flash visual effect and camera shutter sound, as feedback
++    to the user to hint that a screenshot was taken.
++ */
  Rectangle {
      id: root
      visible: false
      color: "white"
--    anchors.fill: parent
      opacity: 0.0
--    // to be set from outside
--    property int rotationAngle: 0
--
--    ScreenGrabber {
--        id: screenGrabber
++    ScreenshotDirectory {
++        id: screenshotDirectory
          objectName: "screenGrabber"
+     }
--    GlobalShortcut {
--        id: screenshotShortcut
--        shortcut: Qt.Key_Print
--        onTriggered: capture()
--    }
--
      NotificationAudio {
          id: shutterSound
          source: "/system/media/audio/ui/camera_click.ogg"
+     }
--    function capture() {
++    function capture(item) {
++        d.target = item;
          visible = true;
          shutterSound.stop();
          shutterSound.play();
@@ -62,13 +57,28 @@
+         }
+     }
++    QtObject {
++        id: d
++        property Item target
++    }
++
      NumberAnimation on opacity {
          id: fadeOut
          from: 1.0
          to: 0.0
          onStopped: {
              if (visible) {
--                screenGrabber.captureAndSave(root.rotationAngle);
++                d.target.grabToImage(function(result)
++                    {
++                        var fileName = screenshotDirectory.makeFileName();
++                        if (fileName.length === 0) {
++                            console.warn("ItemGrabber: No fileName to save image to");
++                        } else {
++                            console.log("ItemGrabber: Saving image to " + fileName);
++                            result.saveToFile(fileName);
++                        }
++                    });
++
                  visible = false;
+             }
+         }
 === modified file 'qml/Components/NotificationAudio.qml'
 --- qml/Components/NotificationAudio.qml	2015-10-29 14:19:34 +0000
 +++ qml/Components/NotificationAudio.qml	2016-04-29 12:42:40 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2015 Canonical, Ltd.
++ * Copyright (C) 2015-2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -22,10 +22,14 @@
      readonly property var playbackState: priv.audio ? priv.audio.playbackState : 0
      function play() {
--        priv.audio.play();
++        if (priv.audio) {
++            priv.audio.play();
++        }
+     }
      function stop() {
--        priv.audio.stop();
++        if (priv.audio) {
++            priv.audio.stop();
++        }
+     }
      QtObject {
 === modified file 'qml/Shell.qml'
 --- qml/Shell.qml	2016-04-21 13:47:29 +0000
 +++ qml/Shell.qml	2016-04-29 12:42:40 +0000
@@ -193,7 +193,7 @@
          onPowerKeyLongPressed: dialogs.showPowerDialog();
          onVolumeDownTriggered: volumeControl.volumeDown();
          onVolumeUpTriggered: volumeControl.volumeUp();
--        onScreenshotTriggered: screenGrabber.capture();
++        onScreenshotTriggered: itemGrabber.capture(shell);
+     }
      GlobalShortcut {
@@ -706,16 +706,22 @@
          onShowHome: showHome()
+     }
--    ScreenGrabber {
--        id: screenGrabber
--        rotationAngle: -shell.orientationAngle
++    ItemGrabber {
++        id: itemGrabber
++        anchors.fill: parent
          z: dialogs.z + 10
++        GlobalShortcut { shortcut: Qt.Key_Print; onTriggered: itemGrabber.capture(shell) }
++        Connections {
++            target: applicationsDisplayLoader.item
++            ignoreUnknownSignals: true
++            onItemSnapshotRequested: itemGrabber.capture(item)
++        }
+     }
      Cursor {
          id: cursor
          visible: shell.hasMouse
--        z: screenGrabber.z + 1
++        z: itemGrabber.z + 1
          onPushedLeftBoundary: {
              if (buttons === Qt.NoButton) {
 === modified file 'qml/Stages/AbstractStage.qml'
 --- qml/Stages/AbstractStage.qml	2016-04-04 13:37:49 +0000
 +++ qml/Stages/AbstractStage.qml	2016-04-29 12:42:40 +0000
@@ -60,6 +60,7 @@
                                        | Qt.InvertedLandscapeOrientation
      signal stageAboutToBeUnloaded
++    signal itemSnapshotRequested(Item item)
      // Shared code for use in stage implementations
      GSettings {
 === modified file 'qml/Stages/DesktopStage.qml'
 --- qml/Stages/DesktopStage.qml	2016-04-13 14:13:36 +0000
 +++ qml/Stages/DesktopStage.qml	2016-04-29 12:42:40 +0000
@@ -94,6 +94,12 @@
          active: priv.focusedAppDelegate !== null
+     }
++    GlobalShortcut {
++        shortcut: Qt.AltModifier|Qt.Key_Print
++        onTriggered: root.itemSnapshotRequested(priv.focusedAppDelegate)
++        active: priv.focusedAppDelegate !== null
++    }
++
      Connections {
          target: root.topLevelSurfaceList
          onCountChanged: {
 === modified file 'src/ShellApplication.cpp'
 --- src/ShellApplication.cpp	2015-12-16 13:58:39 +0000
 +++ src/ShellApplication.cpp	2016-04-29 12:42:40 +0000
@@ -168,6 +168,9 @@
          // Changing the QScreen where a QWindow is drawn makes it also lose focus (besides having
          // its backing QPlatformWindow recreated). So lets refocus it.
          m_shellView->requestActivate();
++        // QWindow::destroy() is called when it changes between screens. We have to manually make it visible again
++        // <dandrader> This bug is supposedly fixed in Qt 5.5.1, although I can still reproduce it there. :-/
++        m_shellView->setVisible(true);
          m_secondaryWindow = new SecondaryWindow(m_qmlEngine);
          m_secondaryWindow->setScreen(screens().at(0));
@@ -194,5 +197,8 @@
          // Changing the QScreen where a QWindow is drawn makes it also lose focus (besides having
          // its backing QPlatformWindow recreated). So lets refocus it.
          m_shellView->requestActivate();
++        // QWindow::destroy() is called when it changes between screens. We have to manually make it visible again
++        // <dandrader> This bug is supposedly fixed in Qt 5.5.1, although I can still reproduce it there. :-/
++        m_shellView->setVisible(true);
+     }
+ }
 === modified file 'tests/plugins/CMakeLists.txt'
 --- tests/plugins/CMakeLists.txt	2015-09-02 09:30:32 +0000
 +++ tests/plugins/CMakeLists.txt	2016-04-29 12:42:40 +0000
@@ -3,7 +3,6 @@
  add_subdirectory(Greeter)
  add_subdirectory(IntegratedLightDM)
  add_subdirectory(Dash)
--add_subdirectory(ScreenGrabber)
  add_subdirectory(Ubuntu)
  add_subdirectory(Unity)
  add_subdirectory(Utils)
 === removed directory 'tests/plugins/ScreenGrabber'
 === removed file 'tests/plugins/ScreenGrabber/CMakeLists.txt'
 --- tests/plugins/ScreenGrabber/CMakeLists.txt	2015-11-24 12:25:30 +0000
 +++ tests/plugins/ScreenGrabber/CMakeLists.txt	1970-01-01 00:00:00 +0000
@@ -1,14 +0,0 @@
--include_directories(
--    ${CMAKE_SOURCE_DIR}/plugins/ScreenGrabber
--    ${CMAKE_CURRENT_BINARY_DIR}
--    )
--
--add_definitions(
--    -DCURRENT_SOURCE_DIR="${CMAKE_CURRENT_SOURCE_DIR}"
--    )
--add_definitions(-DBUILT_PLUGINS_DIR="${CMAKE_BINARY_DIR}/plugins")
--
--add_executable(ScreenGrabberTestExec ScreenGrabberTest.cpp ${CMAKE_SOURCE_DIR}/plugins/ScreenGrabber/screengrabber.cpp)
--qt5_use_modules(ScreenGrabberTestExec Test Quick Concurrent)
--target_link_libraries(ScreenGrabberTestExec ${Qt5Quick_LIBRARIES} ScreenGrabber-qml)
--add_unity8_uitest(ScreenGrabber ScreenGrabberTestExec DEPENDS ScreenGrabber-qml)
 === removed file 'tests/plugins/ScreenGrabber/ScreenGrabberTest.cpp'
 --- tests/plugins/ScreenGrabber/ScreenGrabberTest.cpp	2015-11-18 18:38:36 +0000
 +++ tests/plugins/ScreenGrabber/ScreenGrabberTest.cpp	1970-01-01 00:00:00 +0000
@@ -1,77 +0,0 @@
--/*
-- * Copyright (C) 2015 Canonical, Ltd.
-- *
-- * This program is free software; you can redistribute it and/or modify
-- * it under the terms of the GNU General Public License as published by
-- * the Free Software Foundation; version 3.
-- *
-- * This program is distributed in the hope that it will be useful,
-- * but WITHOUT ANY WARRANTY; without even the implied warranty of
-- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-- * GNU General Public License for more details.
-- *
-- * You should have received a copy of the GNU General Public License
-- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- */
--
--#include "screengrabber.h"
--
--#include <QtTest>
--#include <QtQuick/QQuickView>
--#include <QtQuick/QQuickItem>
--#include <QQmlEngine>
--
--class ScreenGrabberTest: public QObject
--{
--    Q_OBJECT
--
--private Q_SLOTS:
--
--    void initTestCase()
--    {
--        m_view = new QQuickView();
--        m_view->engine()->addImportPath(BUILT_PLUGINS_DIR);
--        m_view->setSource(QUrl::fromLocalFile(CURRENT_SOURCE_DIR "/grabber.qml"));
--        m_grabber = dynamic_cast<ScreenGrabber*>(m_view->rootObject()->property("grabber").value<QObject*>());
--        QVERIFY(m_grabber);
--        m_view->show();
--        QTest::qWaitForWindowExposed(m_view);
--    }
--
--    void cleanupTestCase()
--    {
--        delete m_view;
--        m_view = nullptr;
--        QTRY_COMPARE(QThreadPool::globalInstance()->activeThreadCount(), 0);
--    }
--
--    void testGrabScreenshot()
--    {
--        QSignalSpy grabberSpy(m_grabber, &ScreenGrabber::screenshotSaved);
--        m_grabber->captureAndSave();
--        QTRY_VERIFY(grabberSpy.count() == 1);
--        const QVariantList args = grabberSpy.takeFirst();
--        QVERIFY(args.count() == 1); // verify we got a non-empty filename where the screenshot has been saved
--        QVERIFY(!args.first().toString().isEmpty());
--    }
--
--    void testRotatedScreenshot()
--    {
--        QSignalSpy grabberSpy(m_grabber, &ScreenGrabber::screenshotSaved);
--        m_grabber->captureAndSave(90); // rotate by 90°
--        QTRY_VERIFY(grabberSpy.count() == 1);
--        const QVariantList args = grabberSpy.takeFirst();
--        const QString filename = args.first().toString();
--        QVERIFY(!filename.isEmpty());
--        QImage img(filename);
--        QVERIFY(img.height() > img.width()); // verify that the image got rotated by 90° (height > width)
--    }
--
--private:
--    QQuickView *m_view;
--    ScreenGrabber *m_grabber = nullptr;
--};
--
--QTEST_MAIN(ScreenGrabberTest)
--
--#include "ScreenGrabberTest.moc"
 === removed file 'tests/plugins/ScreenGrabber/grabber.qml'
 --- tests/plugins/ScreenGrabber/grabber.qml	2015-10-12 12:55:34 +0000
 +++ tests/plugins/ScreenGrabber/grabber.qml	1970-01-01 00:00:00 +0000
@@ -1,31 +0,0 @@
--/*
-- * Copyright 2015 Canonical Ltd.
-- *
-- * This program is free software; you can redistribute it and/or modify
-- * it under the terms of the GNU General Public License as published by
-- * the Free Software Foundation; version 3.
-- *
-- * This program is distributed in the hope that it will be useful,
-- * but WITHOUT ANY WARRANTY; without even the implied warranty of
-- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-- * GNU General Public License for more details.
-- *
-- * You should have received a copy of the GNU General Public License
-- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- */
--
--import QtQuick 2.4
--import ScreenGrabber 0.1
--
--Rectangle {
--    property var grabber: screenGrabber
--
--    width: 101 // width intentionally bigger than height to test rotation
--    height: 100
--    color: "green"
--
--    ScreenGrabber {
--        id: screenGrabber
--        objectName: "screenGrabber"
--    }
--}