unity-mir

Merge lp:~gerboland/unity-mir/fix-screenshots into lp:unity-mir

  1. fix-screenshots
  2. Merge into trunk
Proposed by Gerry Boland
Status: Merged
Approved by: Michał Sawicz
Approved revision: 34
Merged at revision: 37
Proposed branch: lp:~gerboland/unity-mir/fix-screenshots
Merge into: lp:unity-mir
Diff against target: 231 lines (+58/-25)
8 files modified
src/modules/Unity/ApplicationManager/application.cpp (+0/-6)
src/modules/Unity/ApplicationManager/application.h (+0/-3)
src/modules/Unity/ApplicationManager/application_list_model.cpp (+2/-2)
src/modules/Unity/ApplicationManager/application_list_model.h (+1/-1)
src/modules/Unity/ApplicationManager/application_manager.cpp (+16/-5)
src/modules/Unity/ApplicationManager/application_manager.h (+2/-0)
src/modules/Unity/ApplicationManager/applicationscreenshotprovider.cpp (+35/-7)
src/modules/Unity/ApplicationManager/applicationscreenshotprovider.h (+2/-1)
To merge this branch: bzr merge lp:~gerboland/unity-mir/fix-screenshots
Reviewer Review Type Date Requested Status
Michał Sawicz Approve
PS Jenkins bot (community) continuous-integration Approve
Albert Astals Cid Pending
Review via email: mp+179307@code.launchpad.net

Commit message

Remove unnecessary Application id, use app_id instead. Fix screenshots to remove white padding pixels. Application only focused when it creates its surface.

Description of the change

Remove unnecessary Application id, use app_id instead. This is needed for lenses to obtain app screenshots.

Fix screenshots to remove white padding pixels, which are due to current implementation detail where all qtubuntu applications are fullscreen, and just draw under the panel.

Application only focused when it creates its surface. Helps shell timings for animations.

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote :

Looks good, i understand m_panelHeight is just a "hack" for the moment, right?

Revision history for this message
Gerry Boland (gerboland) wrote :

> Looks good, i understand m_panelHeight is just a "hack" for the moment, right?

Yep. When platform-api/qtubuntu create surfaces more correctly, this can go away.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:32
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/1/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/1/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/1/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-mir-ci/1/rebuild

review: Needs Fixing (continuous-integration)
lp:~gerboland/unity-mir/fix-screenshots updated
33. By Gerry Boland

Merge trunk and fix commit

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:33
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/3/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/3/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/3

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-mir-ci/3/rebuild

review: Approve (continuous-integration)
lp:~gerboland/unity-mir/fix-screenshots updated
34. By Gerry Boland

Merge trunk

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:34
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/10

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-mir-ci/10/rebuild

review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

To test, please use the Unity8 in lp:~gerboland/unity8/u8m-animation-fixes

Revision history for this message
Michał Sawicz (saviq) wrote :

Yup, works.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/modules/Unity/ApplicationManager/application.cpp'
--- src/modules/Unity/ApplicationManager/application.cpp 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/application.cpp 2013-08-15 15:37:10 +0000
@@ -37,7 +37,6 @@
37 DASSERT(desktopData != NULL);37 DASSERT(desktopData != NULL);
38 DLOG("Application::Application (this=%p, desktopData=%p, pid=%lld, stage=%d, state=%d",38 DLOG("Application::Application (this=%p, desktopData=%p, pid=%lld, stage=%d, state=%d",
39 this, desktopData, pid, static_cast<int>(stage), static_cast<int>(state));39 this, desktopData, pid, static_cast<int>(stage), static_cast<int>(state));
40 m_id = qrand();
41}40}
4241
43Application::~Application()42Application::~Application()
@@ -46,11 +45,6 @@
46 delete m_desktopData;45 delete m_desktopData;
47}46}
4847
49int Application::id() const
50{
51 return m_id;
52}
53
54QString Application::desktopFile() const48QString Application::desktopFile() const
55{49{
56 return m_desktopData->file();50 return m_desktopData->file();
5751
=== modified file 'src/modules/Unity/ApplicationManager/application.h'
--- src/modules/Unity/ApplicationManager/application.h 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/application.h 2013-08-15 15:37:10 +0000
@@ -35,7 +35,6 @@
35 Q_OBJECT35 Q_OBJECT
36 Q_ENUMS(Stage)36 Q_ENUMS(Stage)
37 Q_ENUMS(State)37 Q_ENUMS(State)
38 Q_PROPERTY(int id READ id CONSTANT)
39 Q_PROPERTY(QString desktopFile READ desktopFile CONSTANT)38 Q_PROPERTY(QString desktopFile READ desktopFile CONSTANT)
40 Q_PROPERTY(QString appId READ appId CONSTANT)39 Q_PROPERTY(QString appId READ appId CONSTANT)
41 Q_PROPERTY(QString name READ name CONSTANT)40 Q_PROPERTY(QString name READ name CONSTANT)
@@ -54,7 +53,6 @@
54 Application(DesktopFileReader* desktopData, qint64 pid, Stage stage, State state);53 Application(DesktopFileReader* desktopData, qint64 pid, Stage stage, State state);
55 ~Application();54 ~Application();
5655
57 int id() const;
58 QString desktopFile() const;56 QString desktopFile() const;
59 QString appId() const;57 QString appId() const;
60 QString name() const;58 QString name() const;
@@ -81,7 +79,6 @@
81 void setSession(const std::shared_ptr<mir::shell::ApplicationSession>& session);79 void setSession(const std::shared_ptr<mir::shell::ApplicationSession>& session);
82 void setSessionName(const QString& name);80 void setSessionName(const QString& name);
8381
84 int m_id;
85 DesktopFileReader* m_desktopData;82 DesktopFileReader* m_desktopData;
86 qint64 m_pid;83 qint64 m_pid;
87 Stage m_stage;84 Stage m_stage;
8885
=== modified file 'src/modules/Unity/ApplicationManager/application_list_model.cpp'
--- src/modules/Unity/ApplicationManager/application_list_model.cpp 2013-08-15 12:40:41 +0000
+++ src/modules/Unity/ApplicationManager/application_list_model.cpp 2013-08-15 15:37:10 +0000
@@ -122,10 +122,10 @@
122 return nullptr;122 return nullptr;
123}123}
124124
125Application* ApplicationListModel::getApplicationWithId(const int id)125Application* ApplicationListModel::getApplicationWithAppId(const QString &appId)
126{126{
127 for (Application *app : m_applications) {127 for (Application *app : m_applications) {
128 if (app->m_id == id) {128 if (app->appId() == appId) {
129 return app;129 return app;
130 }130 }
131 }131 }
132132
=== modified file 'src/modules/Unity/ApplicationManager/application_list_model.h'
--- src/modules/Unity/ApplicationManager/application_list_model.h 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/application_list_model.h 2013-08-15 15:37:10 +0000
@@ -51,7 +51,7 @@
5151
52 Application* getApplicationWithSession(const std::shared_ptr<mir::shell::Session> &session);52 Application* getApplicationWithSession(const std::shared_ptr<mir::shell::Session> &session);
53 Application* getApplicationWithSession(const mir::shell::Session *session);53 Application* getApplicationWithSession(const mir::shell::Session *session);
54 Application* getApplicationWithId(const int id);54 Application* getApplicationWithAppId(const QString &appId);
55 Application* getApplicationWithPid(const int pid);55 Application* getApplicationWithPid(const int pid);
56 Application* getLastExecutedApplication();56 Application* getLastExecutedApplication();
5757
5858
=== modified file 'src/modules/Unity/ApplicationManager/application_manager.cpp'
--- src/modules/Unity/ApplicationManager/application_manager.cpp 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/application_manager.cpp 2013-08-15 15:37:10 +0000
@@ -283,11 +283,11 @@
283{283{
284 DLOG("ApplicationManager::sessionFocused (this=%p, application=%s)", this, session->name().c_str());284 DLOG("ApplicationManager::sessionFocused (this=%p, application=%s)", this, session->name().c_str());
285 Application* application = m_mainStageApplications->getApplicationWithSession(session);285 Application* application = m_mainStageApplications->getApplicationWithSession(session);
286 if (application && application != m_mainStageFocusedApplication) {286
287 m_mainStageFocusedApplication = application;287 // Don't give application focus until it has created it's surface, when it is set as state "Running"
288 m_mainStageFocusedApplication->setFocus(true);288 if (application && application->state() != Application::Starting
289 Q_EMIT mainStageFocusedApplicationChanged();289 && application != m_mainStageFocusedApplication) {
290 m_dbusWindowStack->FocusedWindowChanged(0, application->name(), application->stage());290 setFocused(application);
291 }291 }
292}292}
293293
@@ -313,5 +313,16 @@
313 application->setState(Application::Running);313 application->setState(Application::Running);
314314
315 m_dbusWindowStack->WindowCreated(0, application->name());315 m_dbusWindowStack->WindowCreated(0, application->name());
316
317 // only when Session creates a Surface will we actually mark it focused
318 setFocused(application);
316 }319 }
317}320}
321
322void ApplicationManager::setFocused(Application *application)
323{
324 m_mainStageFocusedApplication = application;
325 m_mainStageFocusedApplication->setFocus(true);
326 Q_EMIT mainStageFocusedApplicationChanged();
327 m_dbusWindowStack->FocusedWindowChanged(0, application->name(), application->stage());
328}
318329
=== modified file 'src/modules/Unity/ApplicationManager/application_manager.h'
--- src/modules/Unity/ApplicationManager/application_manager.h 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/application_manager.h 2013-08-15 15:37:10 +0000
@@ -101,6 +101,8 @@
101 void sessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);101 void sessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);
102102
103private:103private:
104 void setFocused(Application *application);
105
104 ApplicationListModel* m_mainStageApplications;106 ApplicationListModel* m_mainStageApplications;
105 Application* m_mainStageFocusedApplication; // remove as Mir has API for this107 Application* m_mainStageFocusedApplication; // remove as Mir has API for this
106 ShellServerConfiguration* m_mirServer;108 ShellServerConfiguration* m_mirServer;
107109
=== modified file 'src/modules/Unity/ApplicationManager/applicationscreenshotprovider.cpp'
--- src/modules/Unity/ApplicationManager/applicationscreenshotprovider.cpp 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/applicationscreenshotprovider.cpp 2013-08-15 15:37:10 +0000
@@ -26,10 +26,29 @@
26// mir26// mir
27#include "mirserver/mir/shell/application_session.h"27#include "mirserver/mir/shell/application_session.h"
2828
29// fallback grid unit used if GRID_UNIT_PX is not in the environment.
30const int defaultGridUnitPx = 8;
31
29ApplicationScreenshotProvider::ApplicationScreenshotProvider(ApplicationManager *appManager)32ApplicationScreenshotProvider::ApplicationScreenshotProvider(ApplicationManager *appManager)
30 : QQuickImageProvider(QQuickImageProvider::Image)33 : QQuickImageProvider(QQuickImageProvider::Image)
31 , m_appManager(appManager)34 , m_appManager(appManager)
35 , m_panelHeight(54)
32{36{
37 // See below to explain why this is needed for now.
38 int gridUnitPx = defaultGridUnitPx;
39
40 QByteArray gridUnitString = qgetenv("GRID_UNIT_PX");
41 if (!gridUnitString.isEmpty()) {
42 bool ok;
43 int value = gridUnitString.toInt(&ok);
44 if (ok) {
45 gridUnitPx = value;
46 }
47 }
48
49 int densityPixelPx = qFloor( (float)gridUnitPx / defaultGridUnitPx );
50
51 m_panelHeight = 3 * gridUnitPx + 2 * densityPixelPx;
33}52}
3453
35QQmlImageProviderBase::Flags ApplicationScreenshotProvider::flags() const54QQmlImageProviderBase::Flags ApplicationScreenshotProvider::flags() const
@@ -38,14 +57,14 @@
38 return QQmlImageProviderBase::ForceAsynchronousImageLoading;57 return QQmlImageProviderBase::ForceAsynchronousImageLoading;
39}58}
4059
41QImage ApplicationScreenshotProvider::requestImage(const QString & id, QSize * size,60QImage ApplicationScreenshotProvider::requestImage(const QString &appId, QSize * size,
42 const QSize & requestedSize)61 const QSize &requestedSize)
43{62{
44 DLOG("ApplicationScreenshotProvider::requestPixmap (this=%p, id=%s)", this, id.toLatin1().constData());63 DLOG("ApplicationScreenshotProvider::requestPixmap (this=%p, id=%s)", this, appId.toLatin1().constData());
4564
46 Application* app = m_appManager->mainStageApplications()->getApplicationWithId(id.toInt());65 Application* app = m_appManager->mainStageApplications()->getApplicationWithAppId(appId);
47 if (app == NULL) {66 if (app == NULL) {
48 LOG("ApplicationScreenshotProvider - app with id %d not found", id.toInt());67 LOG("ApplicationScreenshotProvider - app with appId %s not found", appId.toLatin1().constData());
49 return QImage();68 return QImage();
50 }69 }
5170
@@ -54,6 +73,15 @@
54 return QImage();73 return QImage();
55 }74 }
5675
76 /* Workaround for bug https://bugs.launchpad.net/qtubuntu/+bug/1209216 - currently all qtubuntu
77 * based applications are allocated a fullscreen Mir surface, but draw in a geometry excluding
78 * the panel's rectangle. Mir snapshots thus have a white rectangle which the panel hides.
79 * So need to clip this rectangle from the snapshot. */
80 int yOffset = 0;
81 if (!app->fullscreen()) {
82 yOffset = m_panelHeight;
83 }
84
57 QMutex mutex;85 QMutex mutex;
58 QWaitCondition wait;86 QWaitCondition wait;
59 mutex.lock();87 mutex.lock();
@@ -66,9 +94,9 @@
66 DLOG("ApplicationScreenshotProvider - Mir snapshot ready with size %d x %d",94 DLOG("ApplicationScreenshotProvider - Mir snapshot ready with size %d x %d",
67 snapshot.size.height.as_int(), snapshot.size.width.as_int());95 snapshot.size.height.as_int(), snapshot.size.width.as_int());
6896
69 image = QImage( (const uchar*)snapshot.pixels,97 image = QImage( (const uchar*)snapshot.pixels, // since we mirror, no need to offset starting position
70 snapshot.size.width.as_int(),98 snapshot.size.width.as_int(),
71 snapshot.size.height.as_int(),99 snapshot.size.height.as_int() - yOffset,
72 QImage::Format_ARGB32_Premultiplied).mirrored();100 QImage::Format_ARGB32_Premultiplied).mirrored();
73 wait.wakeOne();101 wait.wakeOne();
74 });102 });
75103
=== modified file 'src/modules/Unity/ApplicationManager/applicationscreenshotprovider.h'
--- src/modules/Unity/ApplicationManager/applicationscreenshotprovider.h 2013-08-09 13:16:13 +0000
+++ src/modules/Unity/ApplicationManager/applicationscreenshotprovider.h 2013-08-15 15:37:10 +0000
@@ -26,10 +26,11 @@
26 explicit ApplicationScreenshotProvider(ApplicationManager *appManager);26 explicit ApplicationScreenshotProvider(ApplicationManager *appManager);
2727
28 Flags flags() const override;28 Flags flags() const override;
29 QImage requestImage(const QString & id, QSize * size, const QSize & requestedSize) override;29 QImage requestImage(const QString &appId, QSize *size, const QSize &requestedSize) override;
3030
31private:31private:
32 ApplicationManager* m_appManager;32 ApplicationManager* m_appManager;
33 int m_panelHeight;
33};34};
3435
35#endif // APPLICATIONSCREENSHOTPROVIDER_H36#endif // APPLICATIONSCREENSHOTPROVIDER_H

Subscribers

People subscribed via source and target branches