Changeset View
Standalone View
src/SpectacleCore.cpp
Show First 20 Lines • Show All 69 Lines • ▼ Show 20 Line(s) | 66 | #endif | |||
---|---|---|---|---|---|
70 | 70 | | |||
71 | if (!mImageGrabber) { | 71 | if (!mImageGrabber) { | ||
72 | mImageGrabber = new DummyImageGrabber; | 72 | mImageGrabber = new DummyImageGrabber; | ||
73 | } | 73 | } | ||
74 | 74 | | |||
75 | setGrabMode(grabMode); | 75 | setGrabMode(grabMode); | ||
76 | mImageGrabber->setCapturePointer(guiConfig.readEntry("includePointer", true)); | 76 | mImageGrabber->setCapturePointer(guiConfig.readEntry("includePointer", true)); | ||
77 | mImageGrabber->setCaptureDecorations(guiConfig.readEntry("includeDecorations", true)); | 77 | mImageGrabber->setCaptureDecorations(guiConfig.readEntry("includeDecorations", true)); | ||
78 | mImageGrabber->setSaveMode(ImageGrabber::SaveMode::File); | ||||
78 | 79 | | |||
79 | if ((!(mImageGrabber->onClickGrabSupported())) && (delayMsec < 0)) { | 80 | if ((!(mImageGrabber->onClickGrabSupported())) && (delayMsec < 0)) { | ||
80 | delayMsec = 0; | 81 | delayMsec = 0; | ||
81 | } | 82 | } | ||
82 | 83 | | |||
83 | connect(mExportManager, &ExportManager::errorMessage, this, &SpectacleCore::showErrorMessage); | 84 | connect(mExportManager, &ExportManager::errorMessage, this, &SpectacleCore::showErrorMessage); | ||
84 | connect(this, &SpectacleCore::errorMessage, this, &SpectacleCore::showErrorMessage); | 85 | connect(this, &SpectacleCore::errorMessage, this, &SpectacleCore::showErrorMessage); | ||
85 | connect(mImageGrabber, &ImageGrabber::pixmapChanged, this, &SpectacleCore::screenshotUpdated); | 86 | connect(mImageGrabber, &ImageGrabber::pixmapChanged, this, &SpectacleCore::screenshotUpdated); | ||
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Line(s) | 140 | { | |||
140 | qApp->setQuitOnLastWindowClosed(true); | 141 | qApp->setQuitOnLastWindowClosed(true); | ||
141 | if (!(mStartMode == GuiMode)) { | 142 | if (!(mStartMode == GuiMode)) { | ||
142 | mStartMode = GuiMode; | 143 | mStartMode = GuiMode; | ||
143 | return initGui(); | 144 | return initGui(); | ||
144 | } | 145 | } | ||
145 | } | 146 | } | ||
146 | 147 | | |||
147 | void SpectacleCore::takeNewScreenshot(const ImageGrabber::GrabMode &mode, | 148 | void SpectacleCore::takeNewScreenshot(const ImageGrabber::GrabMode &mode, | ||
148 | const int &timeout, const bool &includePointer, const bool &includeDecorations) | 149 | const int &timeout, const bool &includePointer, | ||
150 | const bool &includeDecorations, const bool &shouldCopy, const bool &shouldSave) | ||||
149 | { | 151 | { | ||
150 | setGrabMode(mode); | 152 | setGrabMode(mode); | ||
151 | mImageGrabber->setCapturePointer(includePointer); | 153 | mImageGrabber->setCapturePointer(includePointer); | ||
152 | mImageGrabber->setCaptureDecorations(includeDecorations); | 154 | mImageGrabber->setCaptureDecorations(includeDecorations); | ||
155 | if (shouldSave) { | ||||
156 | mImageGrabber->setSaveMode(ImageGrabber::SaveMode::File); | ||||
cfeck: Missing space. Please review all code changes for correct white space, or use a code formatting… | |||||
157 | } else { | ||||
158 | mImageGrabber->setSaveMode(ImageGrabber::SaveMode::Clipboard); | ||||
159 | } | ||||
153 | 160 | | |||
154 | if (timeout < 0) { | 161 | if (timeout < 0) { | ||
155 | mImageGrabber->doOnClickGrab(); | 162 | mImageGrabber->doOnClickGrab(); | ||
156 | return; | 163 | return; | ||
157 | } | 164 | } | ||
158 | 165 | | |||
159 | // when compositing is enabled, we need to give it enough time for the window | 166 | // when compositing is enabled, we need to give it enough time for the window | ||
160 | // to disappear and all the effects are complete before we take the shot. there's | 167 | // to disappear and all the effects are complete before we take the shot. there's | ||
161 | // no way of knowing how long the disappearing effects take, but as per default | 168 | // no way of knowing how long the disappearing effects take, but as per default | ||
162 | // settings (and unless the user has set an extremely slow effect), 200 | 169 | // settings (and unless the user has set an extremely slow effect), 200 | ||
163 | // milliseconds is a good amount of wait time. | 170 | // milliseconds is a good amount of wait time. | ||
164 | 171 | | |||
165 | const int msec = KWindowSystem::compositingActive() ? 200 : 50; | 172 | const int msec = KWindowSystem::compositingActive() ? 200 : 50; | ||
166 | QTimer::singleShot(timeout + msec, mImageGrabber, &ImageGrabber::doImageGrab); | 173 | QTimer::singleShot(timeout + msec, mImageGrabber, &ImageGrabber::doImageGrab); | ||
167 | } | 174 | } | ||
168 | 175 | | |||
169 | void SpectacleCore::showErrorMessage(const QString &errString) | 176 | void SpectacleCore::showErrorMessage(const QString &errString) | ||
170 | { | 177 | { | ||
what happens if you press the shortcut with an existing spectacle open? Also do you have a danger of making multiple connections if a user presses it twice? davidedmundson: what happens if you press the shortcut with an existing spectacle open?
Also do you have a… | |||||
171 | qDebug() << "ERROR: " << errString; | 178 | qDebug() << "ERROR: " << errString; | ||
172 | 179 | | |||
173 | if (mStartMode == GuiMode) { | 180 | if (mStartMode == GuiMode) { | ||
174 | KMessageBox::error(0, errString); | 181 | KMessageBox::error(0, errString); | ||
175 | } | 182 | } | ||
176 | } | 183 | } | ||
177 | 184 | | |||
178 | void SpectacleCore::screenshotUpdated(const QPixmap &pixmap) | 185 | void SpectacleCore::screenshotUpdated(const QPixmap &pixmap) | ||
179 | { | 186 | { | ||
Those connects look wrong. Why would you call them everytime you take a screenshot via DBus? Spectacle can already be running and accept DBus calls, it's not true in general that it only starts when the hotkey is pressed and exits immediately. rkflx: Those `connects` look wrong. Why would you call them everytime you take a screenshot via DBus? | |||||
I didn't think of this. We can use Qt::UniqueConnection to prevent duplicate connections or else multiple connections will lead to multiple calls to slots. What we want to know is if user wants to save the image or copy it to clipboard which is indicated by shouldSave variable, then only we connect appropriate signals and slots. kapillamba4: I didn't think of this. We can use Qt::UniqueConnection to prevent duplicate connections or… | |||||
kapillamba4: I thought qt prevents multiple duplicate connections by default | |||||
In general it works like this:
If you need to use different connections depending on what the user is doing, your code design might be wrong. From a high level, it should look like this: takeScreenshot(options) { if(option1) action1; if(option2 && specialInternalState) action2; } rkflx: In general it works like this:
- At application startup, wire together signals and slots with… | |||||
I tried this in the first place but the codebase is such that it will require alot of changes if we don't use connects inside the function. kapillamba4: I tried this in the first place but the codebase is such that it will require alot of changes… | |||||
Workflow before: setOptions takeScreenshot (always) doSave Workflow after: setOptions takeScreenshot if(save) doSave if(copy) doCopy The structure of the code should reflect that workflow. It might need adaptations beyond takeNewScreenshot, of course. I'm sure with patience you'll solve this eventually ;) rkflx: Workflow before:
setOptions
takeScreenshot
(always)
doSave
Workflow after… | |||||
180 | mExportManager->setPixmap(pixmap); | 187 | mExportManager->setPixmap(pixmap); | ||
188 | if (mImageGrabber->saveMode() == ImageGrabber::SaveMode::Clipboard) { | ||||
189 | mExportManager->doCopyToClipboard(); | ||||
190 | QTimer::singleShot(250, this, &SpectacleCore::allDone); | ||||
191 | } | ||||
181 | 192 | | |||
193 | if (mImageGrabber->saveMode() == ImageGrabber::SaveMode::File) { | ||||
182 | switch (mStartMode) { | 194 | switch (mStartMode) { | ||
183 | case BackgroundMode: | 195 | case BackgroundMode: | ||
184 | case DBusMode: | 196 | case DBusMode: { | ||
185 | { | | |||
186 | if (mNotify) { | 197 | if (mNotify) { | ||
187 | connect(mExportManager, &ExportManager::imageSaved, this, &SpectacleCore::doNotify); | 198 | connect(mExportManager, &ExportManager::imageSaved, this, &SpectacleCore::doNotify); | ||
188 | } | 199 | } | ||
189 | 200 | | |||
190 | QUrl savePath = (mStartMode == BackgroundMode && mFileNameUrl.isValid() && mFileNameUrl.isLocalFile()) ? | 201 | QUrl savePath = (mStartMode == BackgroundMode && mFileNameUrl.isValid() && mFileNameUrl.isLocalFile()) ? | ||
191 | mFileNameUrl : QUrl(); | 202 | mFileNameUrl : QUrl(); | ||
192 | mExportManager->doSave(savePath); | 203 | mExportManager->doSave(savePath); | ||
193 | 204 | | |||
194 | // if we notify, we emit allDone only if the user either dismissed the notification or pressed | 205 | // if we notify, we emit allDone only if the user either dismissed the notification or pressed | ||
195 | // the "Open" button, otherwise the app closes before it can react to it. | 206 | // the "Open" button, otherwise the app closes before it can react to it. | ||
196 | if (!mNotify) { | 207 | if (!mNotify) { | ||
197 | emit allDone(); | 208 | emit allDone(); | ||
198 | } | 209 | } | ||
199 | } | 210 | } | ||
200 | break; | 211 | break; | ||
201 | case GuiMode: | 212 | case GuiMode: | ||
202 | mMainWindow->setScreenshotAndShow(pixmap); | 213 | mMainWindow->setScreenshotAndShow(pixmap); | ||
203 | } | 214 | } | ||
204 | } | 215 | } | ||
216 | } | ||||
205 | 217 | | |||
206 | void SpectacleCore::screenshotFailed() | 218 | void SpectacleCore::screenshotFailed() | ||
207 | { | 219 | { | ||
208 | switch (mStartMode) { | 220 | switch (mStartMode) { | ||
209 | case BackgroundMode: | 221 | case BackgroundMode: | ||
210 | showErrorMessage(i18n("Screenshot capture canceled or failed")); | 222 | showErrorMessage(i18n("Screenshot capture canceled or failed")); | ||
211 | emit allDone(); | 223 | emit allDone(); | ||
212 | return; | 224 | return; | ||
213 | case DBusMode: | 225 | case DBusMode: | ||
214 | emit grabFailed(); | 226 | emit grabFailed(); | ||
215 | emit allDone(); | 227 | emit allDone(); | ||
216 | return; | 228 | return; | ||
217 | case GuiMode: | 229 | case GuiMode: | ||
218 | mMainWindow->show(); | 230 | mMainWindow->show(); | ||
219 | } | 231 | } | ||
220 | } | 232 | } | ||
221 | 233 | | |||
222 | void SpectacleCore::doNotify(const QUrl &savedAt) | 234 | void SpectacleCore::doNotify(const QUrl &savedAt) | ||
223 | { | 235 | { | ||
224 | KNotification *notify = new KNotification(QStringLiteral("newScreenshotSaved")); | 236 | KNotification *notify = new KNotification(QStringLiteral("newScreenshotSaved")); | ||
225 | 237 | | |||
226 | switch(mImageGrabber->grabMode()) { | 238 | switch (mImageGrabber->grabMode()) { | ||
227 | case ImageGrabber::GrabMode::FullScreen: | 239 | case ImageGrabber::GrabMode::FullScreen: | ||
228 | notify->setTitle(i18nc("The entire screen area was captured, heading", "Full Screen Captured")); | 240 | notify->setTitle(i18nc("The entire screen area was captured, heading", "Full Screen Captured")); | ||
229 | break; | 241 | break; | ||
230 | case ImageGrabber::GrabMode::CurrentScreen: | 242 | case ImageGrabber::GrabMode::CurrentScreen: | ||
231 | notify->setTitle(i18nc("The current screen was captured, heading", "Current Screen Captured")); | 243 | notify->setTitle(i18nc("The current screen was captured, heading", "Current Screen Captured")); | ||
232 | break; | 244 | break; | ||
233 | case ImageGrabber::GrabMode::ActiveWindow: | 245 | case ImageGrabber::GrabMode::ActiveWindow: | ||
234 | notify->setTitle(i18nc("The active window was captured, heading", "Active Window Captured")); | 246 | notify->setTitle(i18nc("The active window was captured, heading", "Active Window Captured")); | ||
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Line(s) | |||||
290 | 302 | | |||
291 | // Private | 303 | // Private | ||
292 | 304 | | |||
293 | void SpectacleCore::initGui() | 305 | void SpectacleCore::initGui() | ||
294 | { | 306 | { | ||
295 | if (!isGuiInited) { | 307 | if (!isGuiInited) { | ||
296 | mMainWindow = new KSMainWindow(mImageGrabber->onClickGrabSupported()); | 308 | mMainWindow = new KSMainWindow(mImageGrabber->onClickGrabSupported()); | ||
297 | 309 | | |||
298 | connect(mMainWindow, &KSMainWindow::newScreenshotRequest, this, &SpectacleCore::takeNewScreenshot); | 310 | connect(mMainWindow, &KSMainWindow::newScreenshotRequest, | ||
311 | [this](const ImageGrabber::GrabMode & mode, const int &timeout, const bool & includePointer, const bool & includeDecorations) { | ||||
312 | takeNewScreenshot(mode, timeout, includePointer, includeDecorations); | ||||
313 | }); | ||||
299 | connect(mMainWindow, &KSMainWindow::dragAndDropRequest, this, &SpectacleCore::doStartDragAndDrop); | 314 | connect(mMainWindow, &KSMainWindow::dragAndDropRequest, this, &SpectacleCore::doStartDragAndDrop); | ||
300 | 315 | | |||
301 | isGuiInited = true; | 316 | isGuiInited = true; | ||
302 | QMetaObject::invokeMethod(mImageGrabber, "doImageGrab", Qt::QueuedConnection); | 317 | QMetaObject::invokeMethod(mImageGrabber, "doImageGrab", Qt::QueuedConnection); | ||
303 | } | 318 | } | ||
304 | } | 319 | } |
Missing space. Please review all code changes for correct white space, or use a code formatting tool.