From f482596c037129d906aec6fcb609c88527f54922 Mon Sep 17 00:00:00 2001 From: Jeremy Fleischman Date: Thu, 11 Feb 2021 11:33:23 -0800 Subject: [PATCH] Add back missing call to `resize` The call to `resize` is pretty important... without it, nothing shows up on the screen when I run flameshot (presumably because it's 0 pixels by 0 pixels large). This was accidentally removed when resolving merge conflicts in 77c509e7989b924d31d54e78c6f8e7a7e68968b9. While I was in here, I also opted to delete some comments. I personally am never a fan of commenting out code: if we need something, that's what Git is for! And, just in case GitHub disappears, I thought it would be nice to record my research in a Git commit about why I think removing `Qt::BypassWindowManagerHint` is ok. Comments copied from https://github.com/flameshot-org/flameshot/pull/731): > I played around with it [removing `Qt::BypassWindowManagerHint`] a bit > and it has some really nice properties for me as a Xmonad X11 user: > > - It resolves https://github.com/flameshot-org/flameshot/issues/784 > - It also makes it possible for me to clearly see when flameshot has > focus, and give it focus, which I'd argue is a workaround/fix for > https://github.com/flameshot-org/flameshot/issues/1072, and also has > some nice properties as @filipkilibarda mentioned: "Allows the user to > switch workspaces while in the screenshot GUI..." > > I did some git spelunking and here's the history of the current code: > > - c4d9210c35025e328df57a10364c0da5570b1c15 is the first commit where > something like this showed up, but it used > `Qt::X11BypassWindowManagerHint`, which is just [an alias for > `Qt::BypassWindowManagerHint`](https://github.com/qt/qtbase/blob/b75d60abd2012d78387ec0751e205aef970a024b/src/corelib/global/qnamespace.h#L247) > - 0f30529c77476e90ec49cc9abbf5bb6cdc422f6f removed it (yay!) > - 11b0e2db4b4c0aadae9077622e9e2e72bb5d737f added in > `Qt::BypassWindowManagerHint` with somewhat cryptic message: "Capture > window showing when mouse events are holded" message. I'm not sure what > that means. > - Later, this commit a9b0c213048259f20b95c0fb6599f3c318fa3234 added a > `#ifdef Q_OS_WIN` branch that made it so this > `Qt::BypassWindowManagerHint` only happens on Linux, not Windows. > > So, since flameshot doesn't currently target OSX, I think this change > only affects Linux. @borgmanJeremy if I did some investigation into how > this behaves with other window managers (and maybe wayland?) would you > be open to merging it up? > https://github.com/flameshot-org/flameshot/pull/731#issuecomment-719767364 > for more information. Later, I investigated how things behave on Linux with a non-tiling window manager: > I just installed xfce and tried this out, and as far as I can tell, > things work great! During a screenshot, I can alt-tab to other windows > and they end up on top of the ongoing screenshot, but I can click back > on the screenshot to continue editing the screenshot (strangely, I > cannot alt-tab back to the window). Keyboard shortcuts work as expected, > and this feels like an improvement in every way, IMO. --- src/widgets/capture/capturewidget.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/widgets/capture/capturewidget.cpp b/src/widgets/capture/capturewidget.cpp index a43aedaf..93d3950c 100644 --- a/src/widgets/capture/capturewidget.cpp +++ b/src/widgets/capture/capturewidget.cpp @@ -136,10 +136,9 @@ CaptureWidget::CaptureWidget(const uint id, move(currentScreen->geometry().x(), currentScreen->geometry().y()); resize(currentScreen->size()); #else - // Disable Qt::BypassWindowManagerHint. Workaround for #583 #517 - setWindowFlags(/* Qt::BypassWindowManagerHint */ - Qt::WindowStaysOnTopHint | Qt::FramelessWindowHint | + setWindowFlags(Qt::WindowStaysOnTopHint | Qt::FramelessWindowHint | Qt::Tool); + resize(pixmap().size()); #endif } // Create buttons