ioquake / ioq3

The ioquake3 community effort to continue supporting/developing id's Quake III Arena
https://ioquake3.org/
GNU General Public License v2.0
2.42k stars 529 forks source link

OpenGL2 renderer crash #242

Closed ec- closed 7 years ago

ec- commented 7 years ago

OpenGL2 renderer constantly crashing on windows+radeon drivers if you rename ioq3 executable to quake3.exe. First crash point is on https://github.com/ioquake/ioq3/blob/master/code/renderergl2/tr_dsa.c#L210 where qglBindFramebufferEXT is looks like NULL. This is probably caused by amd/radeon driver optimization that turns off some interfaces/functions in favour of speed

ensiform commented 7 years ago

You can't rename to quake3.exe because the driver is doing things specifically with quake3.exe application name. It strips the extensions list to avoid crashing that occurs in basically all Quake 3 based games that are on the original version with newer drivers/newer cards.

timangus commented 7 years ago

You should probably talk to AMD; it's their bug. If they're going to do special things for particular targets (and really, they shouldn't be, in 2016), they ought to check more than just the file/process name.

ensiform commented 7 years ago

NVIDIA does it too, its not just an "AMD" bug. It's a quake3 bug. R_PrintLongString was the fix to the quake3.exe crash. But it seems that in their driver workaround fix its going to mess with things like rend2.

I don't see any reason to rename from ioquake3.arch.exe anyway? You can run ioquake3 on steam through a non-steam shortcut with overlay etc.

The driver vendors also do this for jasp.exe and jamp.exe too (JKA, presumably plenty of others as well).

I'm assuming that rend2 is making assumptions that those features are available always now, but they necessarily are not if the string is cut off.

ec- commented 7 years ago

You can't rename to quake3.exe_

No, you can but you shouldn't - that's a big difference.

You should probably talk to AMD; it's their bug

Original msogl.dll from MS contains following strings: "codsp.exe", "codmp.exe", "doom3.exe", "quake3.exe", "quake4.exe", "wolfsp.exe", "wolfmp.exe", "nvidia", "all-in-wonder", "ati", etc.

As long as you're resolving some functions dynamically - YOU must ensure that you got valid data to work with it, for example - I'm doing that even during resolving core opengl1 functions.

So it is better to assume 'aggressive' environment by default instead of "ok, it works for me, let's hope it won't crash in other configurations too"

ensiform commented 7 years ago

Well of course you can its not read-only or anything but you can't from the perspective that it will be guaranteed to work because of said workarounds in the driver.

SmileTheory commented 7 years ago

So I looked into it, and saying the radeon drivers are cutting off the GL_EXTENSIONS string is understating the situation by a fairly large margin.

From what I can tell, renaming the executable to quake3.exe makes the drivers only advertise four extensions: GL_ARB_multitexture, GL_EXT_texture_env_add, GL_EXT_compiled_vertex_array, and GL_S3_s3tc.

Naturally, this gimps rend2 quite a bit. The missing GL_EXT_framebuffer_object prevents SSAO, sun shadows, HDR, runtime-generated cube maps, and probably a lot of other things from working at all.

Don't rename the ioquake3 executable to quake3.exe. It causes problems.

There is a bug here though, in that GL_EXT_framebuffer_object isn't strictly required to run rend2, so I've pushed a commit in https://github.com/ioquake/ioq3/commit/730207817ef1e4cf09b7b7ee215c1d16065099bb to fix it.

ensiform commented 7 years ago

Technically if you can look for the truncation issue in the original extension string, couldn't you then set to core profile 3.2 and use

...

glGetIntegerv(GL_NUM_EXTENSIONS, &numExtensions);

...

glGetStringi(GL_EXTENSIONS, i)

...

Since I'm sure they are only cutting off the extension string for the legacy function.