Optimize the selection of pixel format in WGL to early exit if found

[dtrebilco]

Another startup saving on Windows - around ~20ms unless you select a crazy multisampling number that cannot be found. Also removes a temp buffer allocation. Same change could be applied to GLX looking at code. (not familiar enough to test myself)

[floooh]

Apologies for not responding yet. I'll need to focus on getting the WebGPU backend finally merged, but after that I'll give the PR a whirl.

[dtrebilco]

Just an additional note: I see a minor issue with the GLX startup in existing code. If ARB_multisample is not supported, this code is not run if (_sapp.glx.ARB_multisample) { u->samples = _sapp_glx_attrib(n, GLX_SAMPLES); } As the init of "u" goes through _sapp_gl_init_fbconfig () - the samples value will be left as -1 when passed to _sapp_gl_choose_fbconfig. (or _sapp_gl_select_fbconfig if you use my new code). I don't think this method is built to handle the values being negative).

This does not affect the Windows code as my recent change assigns it to 0 even if the extension does not exist.

[floooh]

Starting to test this PR now...

[floooh]

PS: I'm not bothering with GLX fixes too much at the moment, since it will probably be dropped with the alternative EGL initialization in place (first step would be to make EGL the default, and after a while drop GLX completely).

[floooh]

Btw... I would suggest creating a dedicated branch for each PR on your side in the future :)

(makes it a bit easier for me to test those changes on my side, e.g. I already have branch called dtrebilco-master here)

[floooh]

Ok merged! I did a couple of small coding style fixes (also one indentation I missed in the last PR):

https://github.com/floooh/sokol/commit/1db04318439b18fa775ce58eee3a75cb4341c763

[floooh]

PS: many thanks for the PR!

[dtrebilco]

OK, I'll try - I am not really up on how I should use github - I just create a fork for each new pull request I do to projects here. I don't have any other pending changes.

FYI: This was all done/found while doing a line by line conversion to Rust of Windows startup code - I don't plan on taking it further, was just a leaning exercise to see how easy it was or what issues I would have.

https://github.com/dtrebilco/TestLoad/blob/main/src/sapp.rs

[floooh]

Basically, if you are on the master branch of your sokol fork, just create a new branch with a somewhat descriptive name:

git checkout -b wgl-optimize-select-pixelformat

...or I think the new-fangled method is:

git branch wgl-optimize-select-pixelformat

...then implement the change in there and create a pull request from that branch.

Then when the pull request gets merged you can delete that branch and sync your fork from the upstream sokol repo:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

PS:

I just create a fork for each new pull request I do to projects here.

That works too to avoid problems on your side, but in this case I had an old local branch from your previous PR around and had a name-collision (that's how I usually merge PRs, instead of just pressing the Merge button on the GH side - I only do this for very trivial PRs).

It's not a big deal though, I just deleted the old intermediate branch.

[floooh]

FYI: I'm seeing a pretty serious problem on my old Asus laptop with integrated Intel GPU with this fix (GL programs are stuck at startup with a white screen). I'll try to investigate, but maybe I need to revert this PR.

[floooh]

The difference between your code and the old code seems to be that the old code returns 28 from _sapp_wgl_find_pixel_format(), and the new code returns 27.

Some sort of off-by-one error?

[floooh]

Ok, the difference between pixel format 27 (which is broken) and format 28 (which works) is the doublebuffer flag.

Format 27 has doublebuffer=false (which doesn't work), and format 28 has doublebuffer=true.

[floooh]

Ah found the issue, the u.doublebuffer here will only ever be set to true, but never reset to false:

https://github.com/floooh/sokol/blob/4eb208b2d9a0eb87567f0817551ecea2934d7bcb/sokol_app.h#L6825-L6827

...in the old code u wasn't reused, so there it worked correctly.

I'll implement a fix straight in master.

[floooh]

Ok, fixed in https://github.com/floooh/sokol/commit/85d65418c145b2b809498122d0d2bb75e4a96573

[dtrebilco]

Opps - sorry - initially I had that struct initialized in the loop like the old code when testing. Pulling it out of the loop I thought was fine.

[floooh]

tbf the original code with the if (...) was a bit wonky too ;)