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

Lost of focus when use any different renderer than "surface" (i.e. texture, opengl, etc). #178

Closed
kas1e opened this issue Feb 16, 2020 · 24 comments · Fixed by #222
Closed
Assignees
Labels
bug Something isn't working

Comments

@kas1e
Copy link

kas1e commented Feb 16, 2020

I found another issue as well: when we run with "output=texture" then by default focus is lost. With the same settings, 1:1 but with "output=surface" focus is not lost and all fine.

In real life, it means that once we run DOSBox with "output=texture", then speed is low-priority till we not click on the desktop, and then back to the DOSBox window (so to gain focus again).

I added prints to SDL_WINDOWEVENT_FOCUS_LOST event, and can confirm that I see prints from this event when I set "output=texture", and no prints from this event when use "output=surface".
It seems we need to compare window events between texture/surface modes, just in case there is some difference that can cause that.

It can be also not a general bug of course, but something with our SDL2 port, but it feels like it can happen everywhere, just with more modern OSes it can be left unnoticed as speed can be ok even with low priority because of good raw-power and memory-bandwidth and stuff.

@kas1e
Copy link
Author

kas1e commented Feb 17, 2020

I do compile the latest version of DOSBox-staging for win32 (MinGW) , and put printfs to the SDL_WINDOWEVENT_FOCUS_LOST:, and the same: when we use output=texture then focus is lost from the start, while when we use output=surface focus is ok.

That means we have some general bug there in the SDL2 handling code.

The same issue happens and with original Patch from vogon's thread too.

@dreamer
Copy link
Member

dreamer commented Feb 17, 2020

Three topics, which might (or might not) affect focus lost events analysis:

  1. When starting, DOSBox creates splash screen, which always uses output surface - only after that surface window is destroyed and window using proper renderer is created - so be careful about which window you're analyzing when looking at early events.

  2. Do include mouse capture_mouse / autolock settings in analysis.

  3. The order and number of received SDL2 events is very much platform-specific (maybe also Desktop Environment specific, but I haven't seen this yet), it does not automatically indicate an error.

@kas1e
Copy link
Author

kas1e commented Feb 17, 2020

Probably not 1) because the tests i do is put single printf in single focus_lost event of whole sdmain file. Then run with output=surface difnt show that printf ever. Not at start when splash screen, not when recreate renderer. But when i set output=texture, the i can see printf about loosing focus.

And its also probably not 3), because bug reproduced not only on amigaos, but on windows build too: you can try to put printf to loosefocus event, and run win version with output=texture and you will see.

As for 2), can you clarify what you mean ?

@dreamer
Copy link
Member

dreamer commented Feb 17, 2020

I am trying to track down why exactly Windows and macOS versions of DOSBox go into pause-like mode when the window is resizing or being moved (while on Linux it just works correctly), so maybe this issue is somehow related...

As for (2): these control how window should behave regarding mouse (is mouse cursor grabbed on click, on start or not at all), and consequently might affect focus. But there should be no difference between texture and surface in such case, hmm…

@kas1e
Copy link
Author

kas1e commented Feb 17, 2020

Maybe for resizing issue try the same as I put printf to that SDL_WINDOWEVENT_FOCUS_LOST and so when you go to "pause" mode if the focus will be lost then that it?

@dreamer
Copy link
Member

dreamer commented Feb 17, 2020

I went this route of investigation few days ago, it does not go to pause mode - it just stops rendering window altogether. But some small changes in related code landed recently in SVN and I haven't had time to retest again.

@kas1e
Copy link
Author

kas1e commented Feb 17, 2020

At the moment I can only say that:

  1. focus lost before processing of GFX_StartUpdate() (i put prints at the beginning of a function, and at time print come, focus already lost).

  2. focus lost after processing of GFX_SetSize() (print at the end of case TEXTURE: happens before losing focus).

Should be something in between

@kas1e
Copy link
Author

kas1e commented Feb 18, 2020

Some more progress: focus lost also with "output=opengl" and with any other than "surface" output, because it recreates a window (while with pure output=surface window stays the same).

What it means, that when we recreate a window for a new renderer which is not surface (for which window is not recreated) we didn't do something which does for the original, first window.

For output=surface, I even can see that after "splash" screen window not recreated, renderer is already created and used. While for any other renderer I can see how window closes and new ones created.

@dreamer

When starting, DOSBox creates a splash screen, which always uses output surface - only after that surface window is destroyed and window using proper renderer is created - so be careful about which window you're analyzing when looking at early events.

Through, that kind of strange, why we did not create from beginning a window with necessary renderer? I mean from the beginning, why we create surface one, then close it, and create a new one? I mean it doesn't sound very logical, we can create a window with necessary renderer from the beginning, right?

It seems, that we just lose focus because we do call GUI_StartUp() only once, for the first window. Then it got destroyed, and a new one creates without anything which is called in GUI_StartUp. For example, all "focus" code is not called at all.

So, as I see there a few solutions:

1). Proper one: Rewrite SDL2 code so "renderer" options will be parsed from the beginning, and window creation will be done for the necessary renderer from the start. Without needs to recreate windows.

2). Faster way: Find out where we "recreate windows" with any renderers which are not surface, and put there "focus based" code from GUI_StartUp(). Or maybe just call GUI_StartUp() there again.

Maybe issue with non-proper creating of the second window also the reasons why you have crashes on resizing, etc.

@kas1e kas1e changed the title Lost of focus when use "output=texture" Lost of focus when use any different renderer than "surface" (i.e. texture, opengl, etc). Feb 18, 2020
@dreamer
Copy link
Member

dreamer commented Feb 18, 2020

For output=surface, I even can see that after "splash" screen window not recreated, renderer is already created and used. While for any other renderer I can see how window closes and new ones created.

@dreamer

When starting, DOSBox creates a splash screen, which always uses output surface - only after that surface window is destroyed and window using proper renderer is created - so be careful about which window you're analyzing when looking at early events.

Through, that kind of strange, why we did not create from beginning a window with necessary renderer? I mean from the beginning, why we create surface one, then close it, and create a new one? I mean it doesn't sound very logical, we can create a window with necessary renderer from the beginning, right?

These are all remnants of old code, that existed in this area for years and nobody cared to fix it so far. SDL2 patch implemented the bare minimum of functionality to be accepted by upstream (which didn't happen, but might happen still) and needed to preserve the same code structure to work with SDL1 and SDL2, so it means preserving the problem.

dosbox-staging definitely throws away SDL1 compatibility (no ifdefs for SDL1 at all), so we are in a position to address it - upstream will have harder time due to compatibility with Windows 9x, OS/2 and other ancient OSes.

I guess it was implemented this way, because it was the quickest way to hack-in splash screen.
[edit] there are of course other ways to do it - they are just somewhat harder or less convenient to implement.

So, as I see there a few solutions:

1). Proper one: Rewrite SDL2 code so "renderer" options will be parsed from the beginning, and window creation will be done for the necessary renderer from the start. Without needs to recreate windows.

2). Faster way: Find out where we "recreate windows" with any renderers which are not surface, and put there "focus based" code from GUI_StartUp(). Or maybe just call GUI_StartUp() there again.

I very much prefer option (1). I already had parts of this implemented, but upstream started to work in sdlmain area again and it threw a wrench into my plans.

I was implementing it to address a different bug, also caused by changing renderer after showing splash screen: when starting DOSBox in fullscreen, first splash screen is shown, then nothing (for split second as single-threaded dosbox re-creates the renderer), then the window in fullscreen. It disrupts starting games from Steam in Big Picture mode via Boxtron.

The same problem happens also when key mapper UI is triggered via Ctlr-F1.

Maybe issue with non-proper creating of the second window also the reasons why you have crashes on resizing, etc.

I do not have crashes on resizing - if you're talking about my experiments with resizable window - they do not involve re-creating renderer on every resize (that would be plain stupid).

If you're talking about crashes on Windows XP - shaders code require OpenGL 2.0 (at least), so shaders won't work on Windows 9x at all and on Windows XP it depends on the drivers. I guess that's why glshader is disabled by default in upstream...

@kas1e
Copy link
Author

kas1e commented Feb 18, 2020

@dreamer
I may a bit wrongly describe when says about your "crashes", I mean that all problems you have of late with resizing not mean to be related to a recreation of a window, but it may be related to non-proper SDL2 implementation in whole.

Anyway, that seems a very big task to make all be right, maybe in the meantime, something just about focus can be done for the time being, like add some focus-set code to a newly-created window.

@kas1e
Copy link
Author

kas1e commented Feb 18, 2020

@ALL
In meantime do some scary/ugly/crappy hack for local build:

int hack_focus=0; at top

and in case SDL_WINDOWEVENT_FOCUS_LOST:

				case SDL_WINDOWEVENT_FOCUS_LOST:

					if (sdl.desktop.want_type != SCREEN_SURFACE)
					{
						if (hack_focus==0) 
						{
							hack_focus++;
							break;
						}
					}
				
					if (sdl.mouse.locked) {
#ifdef WIN32
						if (sdl.desktop.fullscreen) {
							VGA_KillDrawing();
							GFX_ForceFullscreenExit();
						}
#endif
						GFX_CaptureMouse();
					}
					SetPriority(sdl.priority.nofocus);
					GFX_LosingFocus();
					CPU_Enable_SkipAutoAdjust();
				
				
					break;
				default: ;
			}

So if output not surface, but anything else, then we skip first focus lost like we didn't (so it keeps gained by code, even if we receive such an event). Very scary/ugly of course, but works :)

@dreamer dreamer self-assigned this Feb 20, 2020
@dreamer
Copy link
Member

dreamer commented Feb 20, 2020

I am in the process of fixing a different bug, which is also caused by splash screen using surface, but my WIP/investigation branch might be interesting to test here.

@kas1e Test the following branch: po/gfx-splash-1, however use the following settings:

[sdl]
fullscreen = false           # 'true' sort-of works for texture only
windowresolution = original  # !@$% this feature, only 'original' works for now
output = <any>               # I tested with 'surface', 'opengl' and 'texture' for now

glshader also affects the behaviour, but it should work ok as long as fullscreen = false.

Changes on this branch use DOSBox own internal GFX_* API for displaying splash screen (instead of hacked-in SDL surface API). This means window is no longer destroyed and re-created after splash screen - so we get smoother startup in the result.

I added logs to indicate focus gained/lost - right now on Linux I get only a single 'focus gained' on startup.

This branch is investigation/WIP - the proper solution will need a lot of cleanup and bugfixing before it'll be acceptable on master.

@kas1e
Copy link
Author

kas1e commented Feb 20, 2020

Tested!

With that branch OpenGL resources not cleaned on exit as well (but that probably cleaning not fits in the master repo at moment), but what is more important is that focus is not lost when I use "output=texture" and "texture_renderer=opengl". When I use "texture_renderer=compositing" (one of our native renderers), it then deadlocks as there still used those Lock/Unlock textures.

But at least with OpenGL and OpenGLES2 texture_renderers focus didn't lose, and I can see that window created for splash screen already our one.

I noticed that at first there some black window creates (at the very beginning), then another one creates again of the same size in the same place. In which then splash screen draws. For output=surface there is not that first black window, all like in one single window.

I then check anyway window resolution be not original but something like 1024x768 with output=texture and texture_renderer=opengles2: it also works (just not centered), but what I can see with scaling, now is that there still 2 windows anyway. In the log I see that:

:: set windwow mode : 640x400 type 1
:: create window
:: gfx_stop
:: gfx_start
:: set window mode: 1024x640 type 1
Using driver "opengles2" for texture renderer
:: gfx_start
:: start update status: 1 pitch: 2560
:: gfx_stop
:: splash 1
balblbalbalablabllablblba
:: set window mode: 104x640 type 1
Using driver "opengles2" for texture renderer
:: gfx_start

So seem that before (i.e. as it now in repo), we even have 3 windows. First one, then splash one, and then texture one. Now we have first one, and then one and for a splash and for rendering. Which is much better, but what is that first window for?

@dreamer
Copy link
Member

dreamer commented Feb 20, 2020

We don't need to worry about window with id=1 - it is parent window drawing window decorations (it depends on OS, display server, and SDL videodriver selected).

When running dosbox-staging on Linux:

  • Using X11, our dosbox-staging first window has id=2
  • Using Wayland under DE, that does not support SSD protocol (server-side decorations): first dosbox-staging window has id=1
  • Using Wayland under DE with SSD support, I presume parent will be the same as with X11

id=0 is reserved in SDL2 for indicating errors. I don't know about other OSes.

BTW, when working on this I discovered, that leaks from #180 seem to affect stability on Wayland. Also, some initial work for fixing window positioning (cross-platform), which is also vaguely related is in PR #183

@dreamer dreamer added the bug Something isn't working label Feb 20, 2020
@dreamer
Copy link
Member

dreamer commented Feb 22, 2020

@kas1e please test: po/gfx-splash-3

The exact window created for splash screen is now reused, instead of re-created - this means splash is now rendered via OpenGL or Texture (and not only surface) - as a side effect this focusing bug should be fixed. I'm overall happy with quality of implementation now - just need few more tweaks/fixes to be ready for review.

When starting with fullscreen=false: works as before, except startup is smoother (window does not disappear for a moment).

When starting with fullscreen=true: splash is fullscreen from the beginning, nicely scaled up (this is the issue I was pursuing, it's not perfect but much better than before).

When starting with fullscreen=false and windowresolution=<custom-resolution>: splash screen is scaled up/down to match window size selected by the user.

Also, code for displaying splash is now largely ripped out of GFX initialization - thus #139 will be now easier to implement (we can't just remove it yet, as initial window initialization is quite tricky in details). As part of implementation I addressed #180 (hopefully works ok now).

I did not address #177 yet - but the code is closely related, I think I will take over that bug.

One problem I know is happening: when using fullscreen=true output=surface, the content is broken :/ Overall surface is such a PITA, requiring special handling and specialized algorithms :( I'm not sure yet what's the best way to address this issue.

@dreamer
Copy link
Member

dreamer commented Feb 23, 2020

Just pushed po/gfx-splash-4 - the same as previous branch, just rebased on top of r4330 (plus some fixes to fix static analysis issues and compiler warnings).

@kas1e
Copy link
Author

kas1e commented Feb 23, 2020

Btw, as a side note, while I testing a new branch, I find out that when we use fullscreen=desktop, then when we run in full screen and press alt+enter, the window wasn't centered. Then I found that some time ago you added fixes for this, and I checked them: but no, the window still not centered. It almost centered, but still not right :) See what I mean after I run it with fullscreen=desktop (my one 1920x1080) and then press alt+enter to switch to my 1024x768 window:

http://kas1e.mikendezign.com/aos4/dosbox/fullscreen_desktop_non_center.jpg

It should be much up and left to be centered.

ps. don't see that window name are DOSBox-svn, it still uses fully SDLmain from staging as I just rechecked things in both versions.

@kas1e
Copy link
Author

kas1e commented Feb 23, 2020

Oh found what wrong: its take sdl.draw.width and sdl.draw.height as 640x400. While I have in config "windowresolution=1024x768". So it seems code didn't take values from window resolution into account and threat it as "original" all the time.

@dreamer
Copy link
Member

dreamer commented Feb 23, 2020

Yeah, welcome to the problem I was trying to fix ;) Not exactly window centering - just window placement in various cases when going in/out of fullscreen and when starting in fullscreen. It's a mess, especially when using multiple screens. The solution right now is still-buggy-compromise :(

sdl.draw.width and sdl.draw.height describe size of rendered output (so not affected by windowresolution), which is then scaled to sdl.window.width and sdl.window.height. Overall this seems a bit messy, as sometimes some widths and heights are the same as desktop size, and it breaks internal SDL calculation of what centered position is (that's why SDL_WINDOWPOS_CENTERED* macros are not used). I use sdl.draw.width in one calculation now as a workaround.

Window position is still work in progress, but I hope window focus is ok and there are no memory leaks in branch po/gfx-splash-4.

@dreamer
Copy link
Member

dreamer commented Feb 24, 2020

@kas1e when testing on AmigaOS, you mentioned in other thread you need to apply two PPC-related patches; the one was merged to SVN trunk and master already, the other one is this: 26ecc85, I presume?

@kas1e
Copy link
Author

kas1e commented Feb 24, 2020

@dreamer
Yes, that this one. It probably not in main repo at moment because it needs some ajustement to configure scrpts, so to have something like --enable-ppc-dynrec, or something, which when used will add to config.h those:

#define C_TARGETCPU POWERPC
#define C_DYNREC 1
#define WORDS_BIGENDIAN

As i remember 2 of them added already when configuring, and one are need to be set manually.

@kcgen
Copy link
Member

kcgen commented Feb 24, 2020

Given the PPC dynrec code is meant to be universal across 32-bit PPC processors (ie: Macs, Amigas and even jmarsh's Nintendo GameCube or Wii), I think it should be enabled by default for POWERPC targets, without any need for --enable or --disable flags.

Worst case, a user on some corner-case-hardware might experience and issue, report it to jmarsh, and in the meantime they can manually edit config.h or use the normal core until it's fixed.

@dreamer
Copy link
Member

dreamer commented Mar 21, 2020

@kas1e I bundled the PPC-related patch we talked about earlier on po/gfx-splash-6-ppc for you to test. Please report if the issue is fixed on this branch :)

@dreamer
Copy link
Member

dreamer commented Mar 26, 2020

If the problem returns (or was not completely fixed), feel free to reopen this issue :)

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
No open projects
3 participants