Possible redraw failure fix

[Shigerello]

I found a Bug forum topic "[Beta 18] Startup screen is blank on MacBook Pro" (http://forums.te4.org/viewtopic.php?t=24259, I experienced the same problem in my MacBook), and perhaps found the solution.

Easy fix
First, an easy fix:
(diff of src/main.c, between rev. 2584 and local)

Code: Select all

@@ -433,7 +433,7 @@
 
 	if (!redraw_pending && isActive) {
 		SDL_PushEvent(&event);
-		redraw_pending = 1;
+		//redraw_pending = 1;
 	}
 	return(interval);
 }

In realtime mode (e.g. startup menu's background animation), screen redraw is performed in SDL's event poll mechanism.
In the initial code, until it reaches line 853, every screen redraw event prevents new screen redraw events from pushed into the event queue.
But somehow (I'll explain it later), the event which locked out SDL_PushEvent() away disappears and fails to reach the line 853 sometimes, and in that case, SDL_PushEvent() becomes eternally unavailable to screen redraw events: means no more screen redraw.

Code: Select all

850			case SDL_USEREVENT:
851				if (event.user.code == 0 && isActive) {
852					on_redraw();
853					redraw_pending = 0;
854				}

So, the easy fix remove the lock, redraw_pending.
Even though this fix does not "fix" disappearance of events and allows redundant screen redraw events to be fired, it seems to perform screen redraw.

Fix
Then forget the easy fix now, here comes a much "legitimate" fix:
(diff of src/main.c, between rev. 2584 and local)

Code: Select all

@@ -738,14 +738,16 @@
 		if (!strncmp(arg, "--no-debug", 10)) no_debug = TRUE;
 	}
 
-	boot_lua(1, FALSE, argc, argv);
-
 	// initialize engine and set up resolution and depth
 	Uint32 flags=SDL_INIT_VIDEO | SDL_INIT_TIMER;
 	if (SDL_Init (flags) < 0) {
 		printf("cannot initialize SDL: %s\n", SDL_GetError ());
 		return -1;
 	}
+	// Filter events, to catch the quit event
+	SDL_SetEventFilter(event_filter);
+	
+	boot_lua(1, FALSE, argc, argv);
 
 	SDL_WM_SetIcon(IMG_Load_RW(PHYSFSRWOPS_openRead("/engines/default/data/gfx/te4-icon.png"), TRUE), NULL);
 
@@ -794,9 +796,6 @@
 
 	pass_command_args(argc, argv);
 
-	// Filter events, to catch the quit event
-	SDL_SetEventFilter(event_filter);
-
 	SDL_Event event;
 	while (!exit_engine)
 	{

I found a nasty side effect of SDL_SetEventFilter(), maybe a bug.
SDL_SetEventFilter(), when called, repeatedly calls SDL_PollEvent() internally to consume all events in the event queue and returns.
And while SDL_PollEvent() is repeatedly called, because the event queue is not locked during the whole operation of emptying the event queue (this is the bug, I think), new events are pushed to the event queue by other threads.
And sometimes, some of these newly pushed events happen to be a screen redraw event. Then, these screen redraw events will be consume by SDL_SetEventFilter()'s SDL_PollEvent(), not by our SDL_PollEvent().
Please note that, SDL uses realtime-base timers. They tick even if a debugger interrupted the process. So, if you repeat interrupt/continue in a debugger, there can be a sudden surge of event pushes because you spent SDL timers' interval while you were operating the debugger.

The solution is to execute SDL_SetEventFilter() much earlier, before any possible user-defined event firing mechanisms are initialized.
So that I moved SDL_SetEventFilter() before any of Lua boot functions (and also moved SDL_init() in favor of SDL_SetEventFilter()).
In this fix, redraw_pending can coexists: it means firing of redundant screen redraw events is suppressed.

Still, using a mutex/lock such as redraw_pending requires a clear picture of multithreading. Maybe applying above 2 fixes can be much safer than choosing only one of them.

[yufra]

Tricky! I am using your second patch now and testing it out. When I casually looked at this bug I thought the problem was the timer in setupDisplayTimer not getting set up correctly, but what you say makes sense, too. Here is hoping!

EDIT: Early testing looks good.

[darkgod]

Ohh nifty!

Applied

[Shigerello]

I peeked into the source code of SDL 1.2 and 1.3.
In both SDL versions, SDL_SetEventFilter() is, while it's emptying the event queue, somewhat defenseless against event push.

I filed this problem in Bugzilla of SDL and suggested to 1) use proper mutex or 2) remove emptying process all along.
Personally, I prefer 2) because it does not delete pending events, while 1) does.