veandco / go-sdl2

SDL2 binding for Go
https://godoc.org/github.com/veandco/go-sdl2
BSD 3-Clause "New" or "Revised" License
2.17k stars 219 forks source link

Running readme example yields 30% CPU usage #594

Closed kv-o closed 3 months ago

kv-o commented 3 months ago

Go version: 1.21.5 Go-SDL2 version: 0.4.38 SDL2 version: 2.30.1 OS: Windows 11 Architecture: amd64


I built the example from README.md, with all dependencies linked statically, and the program runs as expected (filled purple rectangle in top left corner), but CPU usage hovers between 20-30%.

I built the example like so:

CGO_ENABLED=1 CC=gcc GOOS=windows GOARCH=amd64 go build -tags static -ldflags "-s -w -H windowsgui" example.go

Here is a screenshot of the CPU usage as shown by Task Manager:

sdlcpu

veeableful commented 3 months ago

Hi @kv-o, you can add sleep using sdl.Delay() between the loop to reduce CPU usage.

kv-o commented 3 months ago

sdl.Delay seems somewhat like a hacky solution... Would sdl.WaitEvent be a better approach? I am unfamiliar with SDL2 and have little experience with graphics, so I apologise if I have overlooked/misunderstood something obvious.

veeableful commented 3 months ago

Hi @kv-o, could you elaborate on why you find it to be hacky? My impression is that most games use similar approach, unless it updates only on events.

kv-o commented 3 months ago

If I understand correctly, specifying a hardcoded delay could introduce latencies between event occurrence and handling. sdl.WaitEvent seems (at least on initial impression) to block until an event actually occurs, thus reducing CPU time without introducing additional latencies. My interpretation might be wrong though; this is my first time interacting with SDL.

veeableful commented 3 months ago

Hi @kv-o, you can calculate the time it takes to process one loop and pass appropriate value to sdl.Delay(). It doesn't need to be hard-coded. I suppose this approach makes sense when you have a target framerate but otherwise, feel free to use sdl.WaitEvent() if it's the better approach for your application.

veeableful commented 3 months ago

Hi @kv-o, I have updated the README with two more examples that demonstrate the use of both WaitEvent() and PollEvent(). Let me know if that addresses your concern!

kv-o commented 3 months ago

The sdl.WaitEvent example only runs WaitEvent on the for loop's init statement, not the post statement. (I haven't tested it but this seems to be a bug)

I've found the following to work decently for my use case:

running := true
for event := sdl.WaitEvent();; event = sdl.WaitEvent() {
    if event == nil {
        continue
    }
    switch event := event.(type) {
    case *sdl.QuitEvent:
        running = false
    }
    if !running {
        break
    }
}

I've had a look at the updated readme and I think there is a lot of code now (that newcomers might get lost in). Perhaps the WaitEvent and PollEvent examples could be reduced to two minimal code snippets that illustrate the difference?

Thanks for explaining the difference between using WaitEvent and Delay though — it made things a lot clearer!

veeableful commented 3 months ago

Hi @kv-o, thanks for noticing the typo! I have fixed it in the latest commit. What do you think can be done to make it simpler? I wanted to illustrate by drawing moving rectangle that reacts to user input for sdl.WaitEvent and another that reacts to user input while continuing to move for sdl.PollEvent.

kv-o commented 3 months ago

I think it's more about keeping the readme itself concise so that newcomers don't get overwhelmed by having to digest a lot of new information upfront. It might be too confusing for someone who is reading the readme for the first time.

Perhaps reduce the explanation of the difference between WaitEvent and Delay to just a couple of lines? I think the difference between the two is easy to understand, so maybe a short explanation with a minimal code snippet should suffice.

In my opinion it's probably better to have only one complete example in the readme, supplemented by a few guiding words of advice. Other standalone examples could probably be moved into the examples folder.

veeableful commented 3 months ago

Hi @kv-o, I have moved the two examples into the examples repository which is referred to in the README. Do you think that looks alright now?

kv-o commented 3 months ago

Looks great!

veeableful commented 3 months ago

Thank you! Would it be okay if I close the issue then?

kv-o commented 3 months ago

Certainly!