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

Fix for #5620 #6870

Closed
wants to merge 1 commit into from
Closed

Conversation

RealDeuce
Copy link

The CallAfter() on Linux causes weird window focus issues.

For me, using ALT-Tab to switch away from PrusaSlicer would cause
PrusaSlicer to take back focus after about one second. Stranger
things reportedly occur with focus-follows-mouse.

The #ifdef is there because it is assumed the CallAfter() was added
in 20f5b7a for a good reason.

The CallAfter() on Linux causes weird window focus issues.

For me, using ALT-Tab to switch away from PrusaSlicer would cause
PrusaSlicer to take back focus after about one second.  Stranger
things reportedly occur with focus-follows-mouse.

The #ifdef is there because it is assumed the CallAfter() was added
in 20f5b7a for a good reason.
@Lenbok
Copy link

Lenbok commented Sep 2, 2021

Is there a way to get an AppImage of this to test, or do I have to build it myself? It would be great to get this fix into the 2.4.0.

@RealDeuce
Copy link
Author

I honestly don't know, I use SuperSlicer myself.

@Lenbok
Copy link

Lenbok commented Sep 6, 2021

Perhaps we can request a review from @bubnikv ?

@bubnikv
Copy link
Collaborator

bubnikv commented Sep 6, 2021

I asked @lukasmatena to look into it.

@Lenbok
Copy link

Lenbok commented Sep 6, 2021

Thanks.

BTW, in terms of specific cases to help reproduce issues with focus-follows-mouse, not all extra windows seem to trigger the problems. For example, opening "Help -> About PrusaSlicer" opens the dialog in front of the main window and it says there. Whereas right clicking on an object on the plate and selecting "Add settings -> Advanced" opens a dialog that is immediately pushed behind the main window.

@lukasmatena
Copy link
Collaborator

@RealDeuce I tried several times but never managed to reproduce either of the issues. However, removing the CallAfter reintroduces the original problem (why we started messing with focus in the first place) for me: after adding a model through Open file dialog, canvas does not get focus and e.g. pressing R to activate 'Rotate' does nothing. This is not visible when that awesome 'Focus follows mouse' function is active, but merging this PR as it is would probably break it for everyone not using it.

@Lenbok
Copy link

Lenbok commented Sep 15, 2021

#5141 sounds like it was about fixing for Windows but it introduced this regression on Linux. @lukasmatena When you say that removing the CallAfter reintroduces the original problem, do you mean on Linux or Windows? This PR as it currently is shouldn't change the behaviour on Windows.

@lukasmatena
Copy link
Collaborator

@Lenbok Sorry. I was talking about Linux. I know what you mean: we didn't need the CallAfter on Linux before, why would we need it now. The behavior may have changed with some of the wxWidgets updates. But I don't know, really.

@RealDeuce
Copy link
Author

This is not visible when that awesome 'Focus follows mouse' function is active, but merging this PR as it is would probably break it for everyone not using it.

I have issues when not using focus follows mouse. When I use Alt-Tab to change to a different program, PrusaSlicer often steals focus back from the program I switched to. That is to say I'm using PrusaSlicer, and press Alt-Tab to switch to my browser, the browser gets focus then focus immediately changes back to PrusaSlicer with no action on my part, effectively forcing me to use the mouse to switch away from using PrusaSlicer. This is completely unacceptable behaviour for any program.

Scheduling a focus change for "a (slightly) later time" as CallAfter() does is an inherently dangerous thing to do because it very much matters when the focus change occurs, and CallAfter() makes no guarantees about that. It's completely believable that on some systems it causes many problems and on other systems there's no problem at all.

@lukasmatena
Copy link
Collaborator

@RealDeuce

When I use Alt-Tab to change to a different program, PrusaSlicer often steals focus back from the program I switched to

I'm unfortunately unable to reproduce it.

Scheduling a focus change for "a (slightly) later time" as CallAfter() does is an inherently dangerous thing to do

None of the CallAfters in the code is there because we would like it.

Btw, are you all guys running our AppImages, or only a distro-provided/self-compiled binaries?

@Lenbok
Copy link

Lenbok commented Sep 21, 2021

@lukasmatena I am running your AppImages, on Ubuntu 20.04 LTS.

@RealDeuce
Copy link
Author

I was running a self-compiled binary.

@RealDeuce
Copy link
Author

Also for what it's worth, I use the XFCE4 desktop environment.

@Lenbok
Copy link

Lenbok commented Nov 26, 2021

@lukasmatena Is it possible to commit this in a way that we can turn it on via some preference file? #5620 is so unbelievably frustrating it makes me want to stop using PrusaSlicer :-(.

(It would be super useful if this repo had build actions that would let you download an AppImage corresponding to a PR to allow much easier PR testing)

(edit: get right bug number)

@bubnikv
Copy link
Collaborator

bubnikv commented Nov 29, 2021

We need to reproduce the issue first. None of us developers have this issue on Linux and the issue seems to be very rare judging from the number of people reporting it (two).

@RealDeuce Is there anything special about your XFCE4 desktop configuration?
@Lenbok How about you? What desktop are you using? Do you use any special configuration?

@RealDeuce
Copy link
Author

There is nothing special about my XFCE4 configuration, but it's only triggered when I use ALT-Tab to switch windows, not when I use the mouse, and it's not triggered every time.

In general, I can never rely on the keyboard to behave in any predictable way in PrusaSlicer, mostly because the 3D view will eat focus any time the cursor comes close to it. I find it highly irritating, but assume that's how the developers want it. As a result, I generally lump all irritating focus behaviours together with that and don't bother reporting them. The only reason I bothered reporting this is because someone else was impacted such that PrusaSlicer was made unusable and I happened to have fixed it in my copy.

If I thought it was helpful, I'm sure I could get dozens of people to pile on and chime in with a nonhelpful "me too" on this issue, but we generally just accept that the Slic3r fork interfaces behave in strange ways and live with it.

@bubnikv
Copy link
Collaborator

bubnikv commented Nov 29, 2021

@RealDeuce PrusaSlicer relies on wxWidgets toolkit providing the "Activate" callback, we are not doing anything special. PrusaSlicer only moves focus inside PrusaSlicer main window if the main window gets activated. Nothing rotten here if you ask me. We need to reproduce and then to debug and dig around.

@RealDeuce
Copy link
Author

I actually keep a local branch with every call to SetFocus() removed so I can check if any specific focus weirdness is explicitly desired by the developers. With that branch, ALT-Tab works every time, and moving the mouse over the platter does not take focus away from the input box. In every case of focus weirdness that's irritated me, it's been due to a call to SetFocus() in the code indicating that it's explicitly what the developer wanted to happen.

Except in this specific case where the focus change happens when the PrusaSlicer app doesn't have focus at all (due to being delayed), it's clearly by design and just because I find something irritating doesn't make it a bug. It's certainly not "rotten", it's just irritating to me personally, and is just a known quirk of using Slic3r forks... as someone said in chat when I asked if anyone else had focus weirdness "Yeah that's a thing, and has been for as long as I've been using SS. I just assumed that's how it works and kept that in mind."

@Lenbok
Copy link

Lenbok commented Nov 29, 2021

@bubnikv I am using Ubuntu 20.04 with the Windowmaker window manager, with the input focus mode in the WPrefs app to focus follows mouse. My PC is several years old. It sounds like CallAfter is doing some asynchronous call and perhaps is susceptible to timing issues?

@Lenbok
Copy link

Lenbok commented Nov 30, 2021

@bubnikv I was easily able to reproduce the problem in a fresh VirtualBox VM.

Setup:

  • Install Ubuntu 20.04 in the VM
  • "sudo apt install wmaker"
  • Log out and log in selecting Window Maker as the login environment.
  • Start WPrefs app (the yin/yang icon in the top right), and in the first "tab" of the settings app select "Input Focus Mode" as "Auto: Set keyboard input focus to the window under the mouse pointer". Click Save and Close.
  • Restart Window Maker (Either log out / in, or over the root window, right click to bring up the "Debian" menu -> Window Maker -> Restart)
  • Now you should be using focus-follows mouse, verify with a couple of windows.
  • Download the PrusaSlicer 2.3.3 app image (I did the non-gtk3 one), and in a terminal chmod +x it and start it.
  • Next next finish through the PS wizard.
  • Right click on the build plate and add a box.

Example 1:

  • Slice it and in the plater view, add a color change on some layer.
  • Now edit the color on the existing color change (right click -> edit color) - the color picker flashes up and immediately the main window jumps in front.

Example 2:

  • Right click on the box and select Export STL file... The dialog opens behind the main window.

Example 3:

  • Enable advanced options on the plater
  • Right click on the object and select "Set number of instances", if you're lucky you'll see the dialog flash before the main window jumps to the front again.

(I also tested the 2.4.0-beta2 appimage and got the same result)

(edit: Actually, focus-follows-mouse doesn't seem to be needed for these above examples - they still give problems if I revert the WPrefs setting back to click to focus)

bubnikv added a commit that referenced this pull request Dec 3, 2021
window without having to resort to CallAfter(), which breaks
on Linux with some window managers that follow mouser cursor.
May fix #5620 #6870 #6992
@bubnikv
Copy link
Collaborator

bubnikv commented Dec 3, 2021

Would you please test the current master?
3622f06
shall solve the focus issue without resorting to CallAfter().
Thanks.

@Lenbok
Copy link

Lenbok commented Dec 4, 2021

Can someone please make a build for me to try?

I tried building myself off master following the linux build instructions and it fails with:

[...]
[ 57%] Linking CXX executable libslic3r_tests
/usr/bin/ld: ../../src/libslic3r/liblibslic3r.a(PNGReadWrite.cpp.o): in function `Slic3r::png::png_read_callback(png_struct_def*, unsigned char*, unsigned long)':
PNGReadWrite.cpp:(.text+0x10): undefined reference to `prusaslicer_png_get_io_ptr'
/usr/bin/ld: ../../src/libslic3r/liblibslic3r.a(PNGReadWrite.cpp.o): in function `Slic3r::png::is_png(Slic3r::png::ReadBuf const&)':
PNGReadWrite.cpp:(.text+0x6f): undefined reference to `prusaslicer_png_sig_cmp'
/usr/bin/ld: ../../src/libslic3r/liblibslic3r.a(PNGReadWrite.cpp.o): in function `Slic3r::png::decode_png(Slic3r::png::IStream&, Slic3r::png::Image<unsigned char>&)':
PNGReadWrite.cpp:(.text+0xd5): undefined reference to `prusaslicer_png_sig_cmp'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x126): undefined reference to `prusaslicer_png_create_read_struct'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x13b): undefined reference to `prusaslicer_png_create_info_struct'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x15c): undefined reference to `prusaslicer_png_set_read_fn'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x16a): undefined reference to `prusaslicer_png_set_sig_bytes'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x178): undefined reference to `prusaslicer_png_read_info'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x186): undefined reference to `prusaslicer_png_get_image_width'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x19a): undefined reference to `prusaslicer_png_get_image_height'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x1ae): undefined reference to `prusaslicer_png_get_color_type'
[...more like this...]
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x489): undefined reference to `prusaslicer_png_free'
/usr/bin/ld: PNGReadWrite.cpp:(.text+0x49d): undefined reference to `prusaslicer_png_destroy_write_struct'
/usr/bin/ld: ../../src/libslic3r/liblibslic3r.a(PNGReadWrite.cpp.o): in function `Slic3r::png::decode_png(Slic3r::png::IStream&, Slic3r::png::Image<unsigned char>&) [clone .cold]':
PNGReadWrite.cpp:(.text.unlikely+0x17): undefined reference to `prusaslicer_png_destroy_info_struct'
/usr/bin/ld: PNGReadWrite.cpp:(.text.unlikely+0x2a): undefined reference to `prusaslicer_png_destroy_read_struct'
collect2: error: ld returned 1 exit status
make[2]: *** [tests/libslic3r/CMakeFiles/libslic3r_tests.dir/build.make:432: tests/libslic3r/libslic3r_tests] Error 1
make[1]: *** [CMakeFiles/Makefile2:1411: tests/libslic3r/CMakeFiles/libslic3r_tests.dir/all] Error 2
[ 87%] Built target libslic3r_gui
make: *** [Makefile:141: all] Error 2

@Lenbok
Copy link

Lenbok commented Dec 4, 2021

I have tried both the regular and GTK3 AppImage builds and they both seem to be working perfectly so far for me! Thanks @bubnikv!

@bubnikv
Copy link
Collaborator

bubnikv commented Dec 4, 2021

@Lenbok Thanks for testing. I just hope that the fix did not break anyone's workflow.

@bubnikv bubnikv closed this Dec 4, 2021
bubnikv added a commit that referenced this pull request Dec 5, 2021
Work around 3D scene focus after de-activation of the main
window without having to resort to CallAfter(), which breaks
on Linux with some window managers that follow mouser cursor.
Fixes #5620 #6870 #6992

3622f06 was not a correct solution,
it broke focus for non-modal windows.
Fixes #7419

The actual issue seems to be caused by wxProgressDialog not playing
well with modal dialogs closed just before wxProgressDialog opens.
If wxProgressDialog parent was not a main frame, keyboard focus
was not restored correctly after the wxProgressDialog closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants