Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clicking on a tile with the mouse doesn't trigger a redraw event #253

Open
keepitsane opened this issue Mar 2, 2021 · 29 comments
Open

Clicking on a tile with the mouse doesn't trigger a redraw event #253

keepitsane opened this issue Mar 2, 2021 · 29 comments
Labels
bug Something isn't working

Comments

@keepitsane
Copy link
Contributor

Lets say I have a tile open on workspace 1, but I currently have a workspace on a separate monitor focused. Clicking on the tile on workspace 1 with the mouse will automatically change Nog's focus to workspace 1. However, Nog doesn't redraw the tiles if a separate tile on workspace 1 had been closed will not focused.

Not sure if this entirely makes sense if not let me know I can trying recording a video of what I mean.

@ramirezmike
Copy link
Collaborator

I think I know what you mean, it feels like a missing workspace refresh. It's not as noticeable when working with one screen.

Similar scenarios include:

  1. multiple monitors, fullscreen an app on one workspace and then move the workspace to another monitor. The remaining, non-fullscreened apps will still be visible on the original monitor.

  2. moving an application to a workspace on another monitor

  3. closing a program on a visible, but unfocused workspace. This can happen either like... when a program crashes or if you close it via task manager or if you just move your mouse and click the X on the program (if it has that kind of thing)

In all the above, what I see is like... the workspace leaves all other apps in the same position/size and doesn't fill in the space until I focus the workspace. Is that what you mean?

If so, shouldn't take too much to fix. The "workaround" is to just focus the workspace that needs a refresh and it should resolve itself; if not, then this might be a bigger issue than I suspect.

@TimUntersberger
Copy link
Owner

@ramirezmike I think fixing this should be really simple right? I haven't worked on this part of the code base in a long time, but if I remember correctly we just have to change a few if statements right?

@TimUntersberger
Copy link
Owner

I think the below line has to be changed. I will look at this next week when I have time.

// window is not already managed and the event isn't `Show`

@ramirezmike
Copy link
Collaborator

I think it's a simple fix but I'm not 100% sure what needs to be fixed. My interpretation of the issue is that it's related to switching focus between monitors/handling when programs on unfocused monitors change (like if an app closes on its own). If that is the issue, the fix would just be to add in something that refreshes all visible workspaces, not just the focused one.

again, I could be misinterpreting the issue, although, if so, what I said above is also an issue and should be fixed to make nog feel more in control when on multiple monitors. I might have some time this week to look into that at least.

ramirezmike added a commit that referenced this issue Mar 13, 2021
- added lines to refresh all grids, not just ones on the focused monitor
- added missing updates of the grid file when moving workspaces
@ramirezmike
Copy link
Collaborator

hey @keepitsane, I made some changes here that might resolve the issue you're seeing if it's what I think it is? I want to change one thing still, but if you get a chance and can check that out and let me know if it resolves your issue. if not, I might need a video or another explanation to understand exactly what you're saying.

@keepitsane
Copy link
Contributor Author

@ramirezmike The new build fixes the issue perfectly! I'll go ahead and leave the issue open though until you add that one other change you were talking about.

Thanks once again!

ramirezmike added a commit that referenced this issue Mar 13, 2021
- added lines to refresh all grids, not just ones on the focused monitor
- added missing updates of the grid file when moving workspaces
- set rendering to position all tiles correctly when one tile is
fullscreened so that all tiles move correctly when moving a grid that
currently has a fullscreened tile. Previously, changes were only applied
to the tile that is fullscreened which caused tiles to remain displaying
when moving workspaces across monitors
@ramirezmike
Copy link
Collaborator

I made one more additional change to address this

multiple monitors, fullscreen an app on one workspace and then move the workspace to another monitor. The remaining, non-fullscreened apps will still be visible on the original monitor.

This one was interesting to me. The problem was that I had originally written the fullscreen code to calculate and apply changes only to the fullscreened application if the workspace being drawn has an application fullscreened. When you move a workspace that is currently fullscreening an application to another monitor it calculates the new size/position for the fullscreened application but doesn't change anything on the "hidden" apps for that workspace. Then it goes through each application and shows them, which shows the "hidden" apps where they were: on the old monitor.

I fixed this by making it always calculate all applications' size and positions on a workspace and then it just renders the fullscreen app on top of all of them.

Anyway, @keepitsane if you'd like to check out the latest patch just to double check everything is running smooth and then I'll probably merge this. Really appreciated this issue because I was able to identify and fix a couple things along the way that I've been noticing but not understanding the source of haha.

@ramirezmike
Copy link
Collaborator

Actually, I might undo that latest fullscreen change because I am noticing that it introduces a "flicker" in some cases. That scenario only really happens when moving a workspace to another monitor while fullscreened and I'd rather add some specific code for that than having this affect other scenarios.

@TimUntersberger
Copy link
Owner

Actually, I might undo that latest fullscreen change because I am noticing that it introduces a "flicker" in some cases

Are you talking about a flicker like when you resize a window?

The remaining, non-fullscreened apps will still be visible on the original monitor.

What if we actually "hide" (whether minimizing or some winapi magic) the windows that aren't shown on a workspace? The only downside I can see is that a crash of nog renders the windows useless if we decide to do some winapi magic to hide the windows.

@ramirezmike
Copy link
Collaborator

ramirezmike commented Mar 14, 2021

What if we actually "hide" (whether minimizing or some winapi magic) the windows that aren't shown on a workspace?

That can work but the downside is that it introduces edge cases that we'd have to add more changes to deal with like... if you have an app fullscreened and you quit that app, now we'd have to add a show call on other windows. or if you move the fullscreened window to another workspace. There might be others, I just worry about adding a fix for a very edge case issue that creates more prominent edge case issues.

I might try just altering the move_workspace_to_monitor code to check if the workspace is fullscreened, unfullscreen it, move the workspace, draw, and then re-fullscreen and draw again. It might be slightly inefficient but it should only affect the one place where this edge case happens and likely won't be noticable since it's moving everything anyway.

Are you talking about a flicker like when you resize a window?

I can't really tell if that's just an existing issue that I ignore or assume is just related to resizing.. that already happens, right? Is that something we should address in another issue?

edit: I'll add the flicker I was seeing was from having an app fullscreened on a workspace and moving to the same workspace or even just interacting with the app sometimes would make the apps behind it briefly change their layering to be in front.

also the other thing I'll add is that removing the latest tweak I did does revert the code back to a state where it is kinda hiding everything that isn't fullscreened. I at least know that works well enough since it's been that way for some time.

@TimUntersberger
Copy link
Owner

I can't really tell if that's just an existing issue that I ignore or assume is just related to resizing.. that already happens, right? Is that something we should address in another issue?

After thinking about it some more I think it is not the same thing.

Is that something we should address in another issue?

I don't think this needs to be addressed yet, because it is very minor and fixing the problem isn't very easy.

@keepitsane
Copy link
Contributor Author

@ramirezmike sorry for the late response, but here are my findings so far.

multiple monitors, fullscreen an app on one workspace and then move the workspace to another monitor. The remaining, non-fullscreened apps will still be visible on the original monitor.

This is actually not a use case that I normally encounter, but I decided to try to test it out. If my understanding is correct, you have two apps open on workspace 1 on monitor 1 and of the two app A is fullscreened. Then you are moving the entire workspace 1 to monitor 2 (not the individual apps, but the entire workspace). Whatever app is fullscreened moves to monitor 2, but on monitor 1 the non-fullscreened app B gets stuck there. Let me know if my understanding of the issue is correct.

So to test I opened a youtube video and opened another window so they are side by side. I then made the video fullscreen and I encountered this bug:
image

Seems like the window that was open next to doesn't hide itself or even disappear. I don't think I have ever had this happen before the redraw build.

Clicking on the window causes the following to happen:
image

It keeps both apps open side by side and makes the video fullscreen in just the width of the tile rather then the entire width of the monitor. (Sidenote I honestly kind of like the layout in the second image where fullscreen content can be restricted to the width of the tile it is operating in. Maybe this can added as a feature?) I would imagine though most people when they want fullscreen they intend the video to go above any other windows even if other windows managed by Nog exist on that workspace. The only way I can get the video to be fullscreen properly is if it is the only managed window on that workspace.

I have tested a few other fullscreen websites and applications and they all resulted in the same issue.

That being said I am not noticing any flicker or issue when trying to move the entire workspace to a new monitor with a fullscreen app. All the windows that were open get moved properly, but when it moves to a new monitor it looks like the second image. Meaning fullscreen content doesn't take the full width of the monitor and go above any other existing tiles.

@ramirezmike
Copy link
Collaborator

I'm gonna play with it a bit more today before I make any changes just to make sure I know what I'm fixing. @keepitsane Do you feel like that specific issue, where the 2nd app appears above the fullscreened 1st app, only happens after moving workspaces around? Or have you seen it occur without having moved a workspace? I thought it happened to me once but I couldn't remember if it was after moving a workspace.

Sidenote I honestly kind of like the layout in the second image where fullscreen content can be restricted to the width of the tile it is operating in. Maybe this can added as a feature?

I kinda thought about something similar. Iin the pinning issue #255 I kinda described that as the "secondary" pin mode... another thought I had was to have multiple workspaces/appbars on one monitor... like, almost like how nog behaves with multiple monitors but on one monitor. But, that seems tricky to implement in a user friendly way and I'm not sure if it's a good idea yet.

@keepitsane
Copy link
Contributor Author

Do you feel like that specific issue, where the 2nd app appears above the fullscreened 1st app, only happens after moving workspaces around? Or have you seen it occur without having moved a workspace?

@ramirezmike It happens both before moving workspaces and even after. The moving workspaces seems to irrelevant to this bug.

I also found that in this redraw build if I have a full screen video playing on monitor 1 and I use my mouse to click on a window on monitor 2, it brings the appbar to the front above the fullscreen content on monitor 1. Not sure if I should be opening separate issues for these, but I only experience them in the redraw branch build.

@ramirezmike
Copy link
Collaborator

It happens both before moving workspaces and even after. The moving workspaces seems to irrelevant to this bug

When I encountered this bug yesterday I noticed it was always showing the "last" program in the workspace above everything. When a workspace is being interacted with there's always a "focused" window, but if there isn't one it falls back to just focusing the last window in the data structure that stores all the windows in the workspace. Usually that'll be whatever is the most right/most bottom of the screen.

Anyway, when I encountered this yesterday I had a workspace with three windows, the middle one was fullscreened and the right-most window was displaying on top and I couldn't see the first window. I suspect the bug is that when the workspace refreshes it somehow loses focus and resorts to focusing the last item despite being in fullscreen mode. Hopefully I'll find time to dig in further, but let me know if the above doesn't match what you've observed because that will help narrow this down.

I also found that in this redraw build if I have a full screen video playing on monitor 1 and I use my mouse to click on a window on monitor 2, it brings the appbar to the front above the fullscreen content on monitor 1.

Part of my approach was to refresh every visible grid when focus gets changed, so there may be something there that needs to get tweaked where it doesn't show the appbar on a refresh of a workspace if it's fullscreened...

But, I'm on the fence about if it should behave that way. After using it a bit a couple days I found this would happen

  1. two monitors with workspaces and multiple managed programs
  2. pull up another program on monitor 2 that remains unmanaged, floating above the workspace on that monitor
  3. go back to monitor 1 and use a managed app on that workspace

When you do this, it'll refresh both monitors, which causes all the managed programs on monitor 2 to display in front of the unmanaged window that you have there. Effectively, what I kept running into is pulling up some temporary info to display on my right monitor would get hidden once I start working on something on my left monitor... and it was somewhat frustrating.

I can revert that piece back and make it so that it only refreshes the workspace that you're currently interacting with which would likely fix this specific fullscreen issue as well. Is that ok with you?

Not sure if I should be opening separate issues for these, but I only experience them in the redraw branch build.

I'd like for any issues related to the redraw branch should be kept in here so it doesn't get confused with other branches.

@ramirezmike
Copy link
Collaborator

@keepitsane

I made a slight tweak on the #254 branch where if you switch to a workspace and it has a node fullscreened then it'll make sure that node is also what's currently focused. I also reverted the part that made all monitors refresh when you change focus. Additionally, the same changes are in #257 which is the pinning branch in case you want to check that out too. The changes are pretty isolated so it should be ok to try out both on the same branch.

Let me know if this addresses your original issue and feels right.

@keepitsane
Copy link
Contributor Author

I am still getting the issue where having two windows open side by side and then making one fullscreen causes the non fullscreen window to show above the fullscreen content. This is the same as the two screenshots from above. Other than this issue the rest seems to have fixed the issues and for the most part is also stable. Thanks once again!

@ramirezmike
Copy link
Collaborator

hmm.. does that happen with or without moving workspaces across monitors?

@keepitsane
Copy link
Contributor Author

It happens without moving workspaces.

@ramirezmike
Copy link
Collaborator

@keepitsane I know it has been some time; nog has had a few updates. I want to re-approach this issue from scratch and want to know A) does this still happen? and B) if so, can you re-explain exactly what the issue is or perhaps provide a video? I want to really isolate the fix and not introduce other strange behaviors.

@keepitsane
Copy link
Contributor Author

I am still currently running your pinning branch as that has yet to be merged back in. But the original issue seems to be fixed, to be honest I tried reading what I wrote and don't quite understand what the issue was in the first place 😄.

However, this issue that still remains in the playing fullscreen content when other windows are managed on the same workspace. Meaning lets say I have youtube open on one half and another browser window managed side by side, making the youtube video fullscreen doesn't go above the other browser window. So the fullscreen video will have the other browser video above it. Clicking anywhere on the workspace causes the fullscreen content to be displayed in only the side by side. See the screenshots above for an example.

@ramirezmike
Copy link
Collaborator

I am still currently running your pinning branch as that has yet to be merged back in

Good news! I updated the branch so it's on top of latest you can try here. I also reverted the pieces that I did related to this issue so we can get a fresh perspective. Checkout the Lua config guide here to update your config to the fancy lua style.

I added more features to the branch too you can check out on #255

So, I guess let me know if you check it out and how it behaves with regards to this issue and then we can go from there.

@TimUntersberger TimUntersberger added the bug Something isn't working label Jun 16, 2021
@keepitsane
Copy link
Contributor Author

Sorry for the delay. I just updated to the latest branch and am no longer experiencing any of the issues I describe in this thread.

@keepitsane
Copy link
Contributor Author

I lied. I am still experiencing the original issue, but no longer the full screen bug.

Here is a recording to maybe give you a better idea (sorry for the weird quality) :
2021-06-21 11-18-44

As you can see the grid doesn't refresh until I use ws_change(ws_id) keybind or manually click on the workspace id in the app bar.

Let me know if you need any additional information.

@keepitsane keepitsane reopened this Jun 21, 2021
@TimUntersberger
Copy link
Owner

That is very weird.

Can you reproduce this 100% or does it only occur sometimes?

@keepitsane
Copy link
Contributor Author

Yes I can reproduce it 100% even on different monitors/workspaces and with different applications.

@TimUntersberger
Copy link
Owner

What is your windows version?

@TimUntersberger
Copy link
Owner

Do you have anything else running that could cause this?

@keepitsane
Copy link
Contributor Author

What is your windows version?

I am running on an insider build so maybe that could be the reason. The issue was actually fixed prior to the changes @ramirezmike reverted in the latest pinning branch 13 days ago, but it introduced the fullscreen content bug I described above.

Edition	Windows 10 Pro
Version	Dev
Installed on	‎5/‎27/‎2021
OS build	21390.2025
Experience	Windows 10 Feature Experience Pack 321.13302.10.3

Do you have anything else running that could cause this?

Nothing that I can think of, do you have any examples of types of programs that could cause issues? I am using PowerToys for the PowerToys Run and the Video Conference Mute plugins. I do not use FancyZones however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants