jiayouxjh / grafx2

Automatically exported from code.google.com/p/grafx2
0 stars 0 forks source link

Switch completely to SDL_WaitEvent #368

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently we use SDL_PollEvent in input.c This leads to busy-looping, with a 
set of tricks developped around it to avoid eating all the CPU.

We should switch to SDL_WaitEvent instead. This allowsmuch less cpu use and is 
a drop-in replacement.

The only problem is SDL_WaitEvent freezes the system when nothing happens. To 
solve that we have to generate regular 'wake up' events to tick the system and 
make sure it's still alive. This is useful for things like the airbrush, and 
for scrolling scroll bars.
We can have the timer run all the time, or start it as needed in the different 
operations.

Original issue reported on code.google.com by pulkoma...@gmail.com on 6 Aug 2010 at 2:24

GoogleCodeExporter commented 9 years ago
Basically done in r1554. Some tweaks still needed as said in the commit message.

Original comment by pulkoma...@gmail.com on 6 Aug 2010 at 3:42

GoogleCodeExporter commented 9 years ago
If the target is a system where the system "ticks" at regular intervals, I've 
often coded with it. It will simplify many cases where I implemented a specific 
computation of Time difference.

It seems SDL_Timer doesn't try to average the granularity, so we'll need to 
code it ourselves in the handler, using SDL_GetTime to compute how much time 
has passed for real.

I worry a little about the size of the event queue. I kinda remember it's 
hard-coded in SDL.dll. In situations where the mouse lags behind (but 
faithfully reproduces mouse movement), the size of the queue determines how 
much maximum lag can happen. If the mouse is at 60Hz and the "ticker" is at 
60Hz too, there will be the same amount of events of each sort, so 50% less lag 
possible before the mouse start jumping to the last mouse position.

Original comment by yrizoud on 6 Aug 2010 at 4:40

GoogleCodeExporter commented 9 years ago
I put a timer that is always on for now, but most of the time it has no use.
The two exceptions are the spray/airbrush and the sliders.

So the idea is to only start the timer in these two cases and stop it as soon 
as it is not needed anymore. If there are other places where the same thing is 
needed, just start/stop the timer there too. This way it doesn't get in the way 
at all for other operations.

Original comment by pulkoma...@gmail.com on 6 Aug 2010 at 4:44

GoogleCodeExporter commented 9 years ago
After much experimenting, I understand what's happening: 
The callback keeps stacking Tick events, all the time, at an "average" of 100 
times per second. Every time Get_input finds a Tick, even if it's only at the 
top of a long queue, it will immediately return 'user_feedback_required=true', 
informing the caller that something significant has happened - so Grafx2 will 
do up to 100 graphic updates per second, unless the individual operation 
manages to detect that it's not needed (old_x==new_x etc.)

I have a fix but it doesn't follow your plans and it's worse than the old 
system.
So I'll rather try:
- make the timer insert events only on some specific situations
- remove all SDL_Delay() that we're not supposed to need anymore
This will point out the places where we must act.

Original comment by yrizoud on 8 Aug 2010 at 6:53

GoogleCodeExporter commented 9 years ago
Mh... running the timer only at needed places actually was my plan :)
The cleanest way isto call SDL_AddTimer when you want to enable it and 
SDL_removeTimer when you're done. But we can also let the timer run and make it 
not do events from a global var.

Original comment by pulkoma...@gmail.com on 8 Aug 2010 at 7:01

GoogleCodeExporter commented 9 years ago
I started with the global var, much less error-prone, and as easy to track in 
the code.
At the moment nothing sets the flag, so the timer keeps sleeping, and when I 
leave the Grafx2 window alone (stuck in SDL_WaitEvent()) it still takes 3-7% 
cpu. I kinda expected zero.

Original comment by yrizoud on 8 Aug 2010 at 7:39

GoogleCodeExporter commented 9 years ago
The timer is still running if you do it with a global var (it's a thread). It's 
fired up, sees it as nothing to do, sleeps again for some time. Also there may 
be some SDL low-levelredraw  going on.

Original comment by pulkoma...@gmail.com on 8 Aug 2010 at 7:57

GoogleCodeExporter commented 9 years ago
I made a test without our timer : still 4-6% cpu.
I can confirm SDL works, for example if you drag another window before Grafx2's 
one, it handles the window redraws, taking as much cpu as available to do it 
quickly. I think we have done all we could.

Original comment by yrizoud on 8 Aug 2010 at 8:05

GoogleCodeExporter commented 9 years ago
Oh, another possible cause is the drag and drop code. We now get syswm events 
directly from windows, and they are one more source of wakeup. Not sure if we 
get that many of them, however.

Original comment by pulkoma...@gmail.com on 8 Aug 2010 at 9:44

GoogleCodeExporter commented 9 years ago
Or the SDL implementation of SDL_WaitEvent() works by sleeping small intervals; 
so on a system that's not busy, it often gets a timeslice.
Not a problem anyway.

Original comment by yrizoud on 9 Aug 2010 at 10:19

GoogleCodeExporter commented 9 years ago
By r1569, it uses a single 60Hz timer that is only set at startup and is left 
running at all times. Repeatable buttons work,  cursor emulation works, 
airbrush works but now its frequency reduces when drawing takes time.

Original comment by yrizoud on 11 Aug 2010 at 12:32

GoogleCodeExporter commented 9 years ago
So far, the cases where we need wake-ups are:
- Airbrush. Easy, because there are only 2 entry and 1 exit points.
- Repeatable buttons. Now that's more problematic : The single entry point is 
evident when clicking the button or slider arrow, but then the wake up system 
must be kept on for a very long time, because you could move the mouse 
anywhere, and you still need repeats when you hover the original repeatable 
button. So the disabling of wakeup events would need be... in input.c, together 
with "Input_sticky_control = 0".

About the method for these wake-ups:
Timer is one method (installed/uninstalled as needed, or reading a flag)
An alternative is possible:
Get_input would use Wait_event() most of the time. But when the wake-ups are 
necessary: instead, pick events that are already in the queue, and only if it's 
empty, sleep for fixed time before returning.
What I like a lot in this idea is that it avoids posting useless events in the 
queue when Grafx2 is wayyy too busy.

Original comment by yrizoud on 12 Aug 2010 at 1:21

GoogleCodeExporter commented 9 years ago
Uh-oh...
Look at the implementation of SDL_WaitEvent :
http://hg.libsdl.org/SDL/file/5002d6aeb85c/src/events/SDL_events.c
Looks like the move to WaitEvent is completely pointless.

Original comment by yrizoud on 13 Aug 2010 at 10:38

GoogleCodeExporter commented 9 years ago

Original comment by pulkoma...@gmail.com on 22 Aug 2010 at 1:34

GoogleCodeExporter commented 9 years ago

Original comment by pulkoma...@gmail.com on 15 Dec 2010 at 4:24