raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
155 stars 56 forks source link

Reflection revisited #178

Closed gvissers closed 2 years ago

gvissers commented 2 years ago

Some drivers, notably the Intel 630 UHD windows drivers (1, 2), no longer support versions of GLSL before 1.5.0. This creates a problem with the water reflections, as the current water shader is very much old-style GLSL.

This PR adds a new water shader compatible with GLSL 1.5.0 and up. It also sets up an OpenGL context explicitly, trying with OpenGL 3.3 and working its way down until it finds something the drivers do support. This is done because if we do not set a context profile, the Intel drivers will happily report OpenGL 2.1 even though they don't support the associated GLSL version (1.2.0). Doing it this way will probably move all players that have bought a new computer in the last decade or so over to the new ~server~ shader, so this could use some testing.

Diligent testing done by Raz on a system that previously had broken reflections shows that it works on the new system. Testing on my local Linux machine also shows no problems, and testing in VirtualBox/Windows shows it still works with the old driver.

pjbroad commented 2 years ago

I've just tried on my main machine: Video card: NVIDIA GeForce GTX 750 Ti/PCIe/SSE2 Vendor ID: NVIDIA Corporation OpenGL Version: 4.6.0 NVIDIA 470.86 OpenGL Context version: OpenGL Compatibility 3.3

I get clean reflections but no ripples on water, I only have water shader quality of 0 or 1, with the old shader I have quality 2 also.

The log shows "Info: Using 1.50 water shader". I'm using the shaders from the client source.

gvissers commented 2 years ago

(copied from issue #175)

Very strange. If you find time, could you do me a favour and drill down in reflection.c, function get_max_supported_water_shader_quality() to see where it sets the max water shader quality to 1? I am specifically interested in whether do_disable_water_ripples near the end of the function is set to 1, or if it's because of the block above of shader tests above it.

~Having said that, I should update that function to test for new shaders if those are to be used.~ EDIT: I don't, the correct shader version is automatically tested. I will have to retest though, if the new option to toggle between shader version is changed.

pjbroad commented 2 years ago

If you find time, could you do me a favour and drill down in reflection.c, function get_max_supported_water_shader_quality() to see where it sets the max water shader quality to 1? I

The function exits matching this test: if (get_shader(st_water, sst_shadow_receiver, sft_disabled, 1) == 0)

All later test fail match too and if I force it to exit with a value 2, I get solid blue (overwriting the edge of the land) and no reflections at all.

gvissers commented 2 years ago

Thanks for testing and finding where the problem was located.

Having an option to select the shader make it much easier to compare them and find new bugs, so thank you too for that :) I still need to fix the lighting, but I was wondering if 7a37fc7 fixed your water shader quality options (it's a shader fix, so you'll need to copy the fragment shader again).

pjbroad commented 2 years ago

Having an option to select the shader make it much easier to compare them and find new bugs, so thank you too for that :) I still need to fix the lighting, but I was wondering if 7a37fc7 fixed your water shader quality options (it's a shader fix, so you'll need to copy the fragment shader again).

With the latest commits, the new shader now works on this system. I have 0,1,2 water quality options and the ripples are back. The new/old option works well. With the new shader, the water looks lighter which I assume is the lighting issue you refer too. Looking good though. I'll try it out on some other systems.

gvissers commented 2 years ago

All right, I believe the new shader now matches the behaviour of the old shader exactly. But I also think the old shader makes the water too light. This is especially noticable in the night time, where the water has a light blueish sheen (or is just plain light blue when water qulity is set to 0). The new water fragment shader contains two commented out lines which use the actual light color[*] for blending the tile color with, instead of plain white as is done in the old shader. I believe this is more realistic (though dark) in the night time. A disadvantage of using this color is that with the current color values, the shadows are barely visible in twilight.

Anyway, I think this is more or less it. @pjbroad , if you find no further problems, I'll merge it.

[*] it currently still disregards light from eye candy effects.

gvissers commented 2 years ago

Added the effect og eye candy lights to the new water shader, and found out it didn't matter :facepalm: See d473d1a commit message for details.

While doing so, I noticed the reflection off eye candy effects aren't drawn. Is this something we want to fix?

In other news, Raz has confirmed reflections on his system still work when he enables the new water shader in the settings.

pjbroad commented 2 years ago

I've compare the lighting level between this version and current git. At night the new version is very bright compared to before, it does not look like night time at all. I presume that's not what you were intending?

When this branch was just the reflections, it was close to being merged, now it looks like you've made a whole host of additional changes for lighting. Any chance the two pieces of work could be separated?

Weirdly lightning looks to make the scene darker too.

gvissers commented 2 years ago

Strange. Yes, I do think the water is a bit bright, but it looks the same as with the old shader to me: elscreen003 elscreen004

Did you copy the new water shader files to your shaders directory?

pjbroad commented 2 years ago

Its not the difference between the old and new sharers, its the difference to this branch and master.

Introduced in commit 43671dfafffbb347bb18b68364c05b7b9da4f526 I think.

gvissers commented 2 years ago

Its not the difference between the old and new sharers, its the difference to this branch and master.

Yes, you're right. Will check.

gvissers commented 2 years ago

Should be fixed now, the problem was actually in b443244. At least now I understand why I thought the old shader always mixed with solid white :unamused:

When this branch was just the reflections, it was close to being merged, now it looks like you've made a whole host of additional changes for lighting. Any chance the two pieces of work could be separated?

The light color setting before 43671df was a horrible kludge and only took the global light into account. At some point something like this will have to be done anyway, so might as well do it now.

pjbroad commented 2 years ago

Thanks, that looks to have fixed it. I may be making it up but the new code looks better in terms of light graduation. What should we me noticing in the reflections that's different.

gvissers commented 2 years ago

Thanks, that looks to have fixed it. I may be making it up but the new code looks better in terms of light graduation.

That's possible. The new shader applies the lighting per fragment, whereas the old one calculates lighting per vertex and interpolates, I believe.

What should we me noticing in the reflections that's different.

Between the old shader and the new one? Not much, apart from perhaps the graduation effect you noticed. The lighting work was done to get any light effect in the new shader at all, as it can no longer access the implicit light color from OpenGL.

pjbroad commented 2 years ago

I've tried several machines with different GPU's and they all work fine bar one machine. The new shader is not supported on my Raspberry Pi 400 or in QEMU/libvirt on my system but the old shader works fine. The new shader even works on my ancient HP laptop running windows 10 with a AMD Radeon HD 6320 GPU.

The machine with issues is running Ubuntu 20.04 with an Intel 4600 GPU. This machine aborts on start with:

[2022-02-09 20:59:26, /home/paul/el/newrefl/load_gl_extensions.c:1191] Error: OpenGL invalid operation
[2022-02-09 20:59:26, /home/paul/el/newrefl/load_gl_extensions.c:1192] Error: glGetString() returned NULL for GL_EXTENSIONS=0x1f03

The pre-change #glinfo

[21:02:39] Video card: Mesa DRI Intel(R) HD Graphics 4600 (HSW GT2)
[21:02:39] Vendor ID: Intel Open Source Technology Center
[21:02:39] OpenGL Version: 3.0 Mesa 21.0.3
[21:02:32] GL_ARB_multitexture extension found, using it.
[21:02:32] GL_ARB_texture_env_combine extension found, using it.
[21:02:32] GL_EXT_compiled_vertex_array extension found, using it.
[21:02:32] GL_ARB_point_sprite extension found, using it.
[21:02:32] GL_ARB_texture_compression extension found, using it.
[21:02:32] GL_EXT_texture_compression_s3tc extension found, using it.
[21:02:32] GL_SGIS_generate_mipmap extension found, using it.
[21:02:32] GL_ARB_shadow extension found, using it.
[21:02:32] GL_ARB_vertex_buffer_object extension found, using it.
[21:02:32] GL_EXT_framebuffer_object extension found, using it.
[21:02:32] GL_EXT_draw_range_elements extension found, using it.
[21:02:32] GL_ARB_texture_non_power_of_two extension found, using it.
[21:02:32] GL_ARB_fragment_program extension found, using it.
[21:02:32] GL_ARB_vertex_program extension found, using it.
[21:02:32] GL_ARB_fragment_shader extension found, using it.
[21:02:32] GL_ARB_vertex_shader extension found, using it.
[21:02:32] GL_ARB_shader_objects extension found, using it.
[21:02:32] GL_ARB_shading_language_100 extension found, using it.
[21:02:32] GL_ARB_texture_mirrored_repeat extension found, NOT using it...
[21:02:32] GL_ARB_texture_rectangle extension found, NOT using it...
[21:02:32] GL_EXT_fog_coord extension found, NOT using it...
[21:02:32] Couldn't find the GL_ATI_texture_compression_3dc extension, not using it...
[21:02:32] Couldn't find the GL_EXT_texture_compression_latc extension, not using it...
[21:02:32] GL_EXT_texture_filter_anisotropic extension found, using it.
[21:02:32] Not using vertex program for actor animation.

Reading around a bit, should glGetStringi() be used rather than glGetString() ?

gvissers commented 2 years ago

The machine with issues is running Ubuntu 20.04 with an Intel 4600 GPU. This machine aborts on start with:

[2022-02-09 20:59:26, /home/paul/el/newrefl/load_gl_extensions.c:1191] Error: OpenGL invalid operation
[2022-02-09 20:59:26, /home/paul/el/newrefl/load_gl_extensions.c:1192] Error: glGetString() returned NULL for GL_EXTENSIONS=0x1f03

Inconceivable!

I guess "compatibility profile" does not mean what I think it means.

Reading around a bit, should glGetStringi() be used rather than glGetString() ?

Indeed, 5735b2a should fix it, I hope we don't run into any more of these issues.

EDIT: interesting BTW, that all of these issues pop up with Intel drivers.

pjbroad commented 2 years ago

Unfortunately, that does not fix it, it just breaks elsewhere The client does not exit, but I now have a completely blank window with several "OpenGL invalid operation" messages and "OpenGL invalid enumerant" message that seem difficult to track down. The code no longer works on Raspberry PI, exiting at the same place the Intel 4600 GPU machine exit, it worked fine before "Failed to get OpenGL extensions string". I think this is because the code is wrongly detecting OpenGL 3.0 support. I'll try to trace the other errors later.

gvissers commented 2 years ago

Unfortunately, that does not fix it, it just breaks elsewhere The client does not exit, but I now have a completely blank window with several "OpenGL invalid operation" messages and "OpenGL invalid enumerant" message that seem difficult to track down. The code no longer works on Raspberry PI, exiting at the same place the Intel 4600 GPU machine exit, it worked fine before "Failed to get OpenGL extensions string". I think this is because the code is wrongly detecting OpenGL 3.0 support. I'll try to trace the other errors later.

Don't bother, it will probably choke all over the place. It looks like for some reason it is using a Core profile, even though we explicitly did not request that. We will have to rethink our approach to requesting a 3.3 profile first and then working our way down.

I'm hoping 503a4bf will fix the Pi at least.

EDIT: On the offending system, can you check after the line (349?)

    SDL_GL_GetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, &gl_context_info.profile);

if gl_context_info.profile == SDL_GL_CONTEXT_PROFILE_COMPATIBILITY? If so, we have a problem autodetecting what context is actually used. If it equal SDL_GL_CONTEXT_PROFILE_CORE, we could try just downgrading to OpenGL 2.1 is a core profile is selected.

pjbroad commented 2 years ago

I'm hoping 503a4bf will fix the Pi at least.

Yes it did.

EDIT: On the offending system, can you check after the line (349?)

init_video:350 gl_context_info.profile == SDL_GL_CONTEXT_PROFILE_COMPATIBILITY = 1
init_video:351 gl_context_info.profile == SDL_GL_CONTEXT_PROFILE_CORE = 0
gvissers commented 2 years ago

The pre-change #glinfo

[21:02:39] Video card: Mesa DRI Intel(R) HD Graphics 4600 (HSW GT2)
[21:02:39] Vendor ID: Intel Open Source Technology Center
[21:02:39] OpenGL Version: 3.0 Mesa 21.0.3

Right, OpenGL 3.0, so no compatibility profile exists (even if SDL reports it). But OpenGL 3.0 should not have removed glGetString(GL_EXTENSIONS), only deprecated it, if I'm not mistaken.

In 94064e9 I added a check to see if that particular call works, and if it doesn't, we continue with the next OpenGL version (which in this case would be 2.1). Can you check if this help the affected machine?

pjbroad commented 2 years ago

In 94064e9 I added a check to see if that particular call works, and if it doesn't, we continue with the next OpenGL version (which in this case would be 2.1). Can you check if this help the affected machine?

Yes, that works. The old shader works fine and the client says the new shader is not supported.

gvissers commented 2 years ago

Yes, that works. The old shader works fine and the client says the new shader is not supported.

Phew. Yeah, the new shader should not work on this system, it needs at least OpenGL 3.2.

pjbroad commented 2 years ago

Phew. Yeah, the new shader should not work on this system, it needs at least OpenGL 3.2.

Nice work, thanks for sorting it out.

gvissers commented 2 years ago

Thank you for testing all this, it must be a lot of work.

gvissers commented 2 years ago

A few more data points (besides my main system, which works):

pjbroad commented 2 years ago

I've re-testing the latest changes on several machines and VMs, mostly Linux but one physical windows install. Everything works fine, either supporting both shaders or just the old one. I have no machines where the full water effect does not work properly now. The light looks fine too (other than the minor brightness difference of water using the new shader).

You suggested in chat that I could merge and do some build but I've not had time. I'll leave the merge for you and do some builds after that.