Skip to content

Conversation

@filipkilibarda
Copy link
Contributor

@filipkilibarda filipkilibarda commented Apr 26, 2020

Workaround for previously reported screen blinking issue #583 #517

System

  • Ubuntu 18.04
  • Xorg
  • Gnome Shell 3.28.4

The bug

Basically if you hover your mouse over one of the widgets in the screenshot GUI, wait for the tooltip to pop up, then immediately move your mouse off the widget, the whole screen blinks once.

Unfortunately I can't screen record it because it doesn't reproduce if a screen recorder is running. Only option would be to record my screen with my phone.

What does this PR improve?

  • Stops the blinking
  • Allows the user to switch workspaces while in the screenshot GUI. Prior to this PR the screenshot GUI would effectively freeze because the window manager would never be able to bring the screenshot GUI back into focus after switching from/to workspaces.

Coments

I don't know why Qt::BypassWindowManagerHint was specified in the first place, so I don't know what kinds of unintentional side effects removing this could have.

@filipkilibarda filipkilibarda marked this pull request as ready for review April 26, 2020 20:13
@borgmanJeremy
Copy link
Collaborator

I want to look into the side effects of removing Qt::BypassWindowManagerHint Once we document that I'll merge this in.

@jfly
Copy link
Contributor

jfly commented Oct 30, 2020

I'd like to see something like this get merged. I played around with it a bit and it has some really nice properties for me as a Xmonad X11 user:

I did some git spelunking and here's the history of the current code:

  • c4d9210 is the first commit where something like this showed up, but it used Qt::X11BypassWindowManagerHint, which is just an alias for Qt::BypassWindowManagerHint
  • 0f30529 removed it (yay!)
  • 11b0e2d 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 a9b0c21 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?

jfly added a commit to jfly/flameshot that referenced this pull request Oct 30, 2020
@jfly
Copy link
Contributor

jfly commented Oct 30, 2020

(Btw, not sure if this is the right place to ask, but @borgmanJeremy is there a slack workspace for flameshot you'd be willing to invite @FatBoyXPC and I to?)

@borgmanJeremy
Copy link
Collaborator

@jfly Absolutely! I should probably link to it in the README somewhere, its a public slack: https://flameshotworkspace.slack.com

I need to actually test this branch. It was submitted before I really took over so I haven't sat down and tested it. Does it basically just allow alt-tab functionality without adding a border?

@jfly
Copy link
Contributor

jfly commented Oct 30, 2020

Does it basically just allow alt-tab functionality without adding a border?

The behavior for me (again, xmonad on x11) with the Qt::BypassWindowManagerHint removed is that I end up with a window floating on top of everything on my screen. If I switch workspaces, I see the workspace and can interact with it as normal, no flameshot in the way (I like that!). I don't really have something like alt-tab, so it's hard for me to answer. The flameshot window still appears on top of most everything (notifications still appear on top of it), but I can use my window manager to converts the window to a regular tiling window and see other stuff.

What's still a bit confusing is that if I switch focus to another window, the flameshot window is still on top of it. But that's just how floating windows work in xmonad (floating stuff floats on top of tiled stuff). What's really really nice about this change though is that if I interact with the flameshot window again (by clicking on it) it gets focus again and keyboard shortcuts (such as esc) work, which is why I'd say this could be considered a fix for #1072.

Unrelated, the slack link you sent shows me an option to sign in, but I don't see an option to sign up?

@FatBoyXPC
Copy link
Contributor

Yeah I think we need a special invite link, rather than the link to the workspace. Alternatively, we could be invited via email address, which for me, is my username here with a gmail address (trying to avoid the spam here).

@borgmanJeremy
Copy link
Collaborator

Here is the invite link: https://join.slack.com/t/flameshotworkspace/shared_invite/zt-in7gdipi-99oWqKeGKK1DRDOUaMSjvA

@borgmanJeremy borgmanJeremy self-assigned this Nov 30, 2020
@borgmanJeremy
Copy link
Collaborator

@jfly Have you looked at this in more detail to see what happens on other DE's?

@jfly
Copy link
Contributor

jfly commented Jan 12, 2021

I have not! Will try to find some time to do that tonight.

jfly added a commit to jfly/flameshot that referenced this pull request Feb 10, 2021
@jfly
Copy link
Contributor

jfly commented Feb 10, 2021

@borgmanJeremy sorry for the massive delay here. 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.

@borgmanJeremy
Copy link
Collaborator

@jfly Ok great thanks for getting back to me. I'll resolve the conflicts and get this merged shortly so I can include it in the 0.9 release candidate.

jfly added a commit to jfly/flameshot that referenced this pull request Feb 11, 2021
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 77c509e.

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
flameshot-org#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 flameshot-org#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
> flameshot-org#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:
>
> - c4d9210 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)
> - 0f30529 removed it (yay!)
> - 11b0e2d 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 a9b0c21 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?
> flameshot-org#731 (comment)
> 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.
jfly added a commit to jfly/flameshot that referenced this pull request Feb 11, 2021
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 77c509e.

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
flameshot-org#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 flameshot-org#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
> flameshot-org#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:
>
> - c4d9210 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)
> - 0f30529 removed it (yay!)
> - 11b0e2d 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 a9b0c21 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?
> flameshot-org#731 (comment)
> 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.
jfly added a commit to jfly/flameshot that referenced this pull request Feb 11, 2021
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 77c509e.

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
flameshot-org#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 flameshot-org#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
> flameshot-org#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:
>
> - c4d9210 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)
> - 0f30529 removed it (yay!)
> - 11b0e2d 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 a9b0c21 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?
> flameshot-org#731 (comment)
> 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.
borgmanJeremy pushed a commit that referenced this pull request Feb 11, 2021
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 77c509e.

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
#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 #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
> #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:
>
> - c4d9210 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)
> - 0f30529 removed it (yay!)
> - 11b0e2d 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 a9b0c21 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?
> #731 (comment)
> 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.
panpuchkov pushed a commit to namecheap/flameshot that referenced this pull request Feb 15, 2021
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 77c509e.

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
flameshot-org#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 flameshot-org#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
> flameshot-org#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:
>
> - c4d9210 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)
> - 0f30529 removed it (yay!)
> - 11b0e2d 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 a9b0c21 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?
> flameshot-org#731 (comment)
> 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.

(cherry picked from commit f482596)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants