supertuxkart / stk-code

The code base of supertuxkart
Other
4.37k stars 1.03k forks source link

Refactoring of OpenGL code #2225

Closed hiker closed 7 years ago

hiker commented 8 years ago

Our new OpenGL is in a horrible state: I've found memory leaks, global variables (sometimes declared in the middle of different functions in one file), hardly anything follows our coding style, hardly any OO is used, and for example irr_driver (originally a thin interface between STK and irrlicht) is still O, but it is now a 'god object' anti-pattern with implementations in at least 5 different files(!), and over 7000 lines of code.

Ideally our graphics rendering engine would be combined into a stand-alone (irrlicht based) library, but the amount of effort is probably huge (e.g. just designing a proper API to allow our new functionality to be used might not be easy). More realistic would be do just to small refactoring steps. Split big objects into smaller units, convert functions to be part of objects.

As an example, I have already done some work on the shaders (previously one shader was distributed across three files: one for declaration, one for implementation, and one for usage - now all shaders are declared, implemented and used in one file (except for a few shared shaders).

Also there is hardly any documentation for the system.

Elderme commented 8 years ago

I'm working on a branch to partially refactor the engine. https://github.com/Elderme/stk-code/tree/renderer_refactoring

Main modifications are:

I know there are many modifications, but it was very difficult to make small and coherent changes.

I am not satisfied with everything (for instance the different materials are hardcoded in several places, this makes the code difficult to maintain. But it could be a separate refactoring step). I am now adding comments and fixing minors things.

Could you please take a look at my work? If there are points that I can improved, please let me know. I've tested with nvidia proprietary driver on Linux; it is working fine on my computer. I'm particularly interested in tests on Intel and ATI hardware.

auriamg commented 8 years ago

Thanks a lot for your work! That is indeed quite a huge change set, but overall it seems pretty clean and a nice improvement.

Some concerns about visual studio compatibility, however

command_buffer.hpp should "#include "

But then I get

Error   1   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  658 1   supertuxkart
Error   2   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  659 1   supertuxkart
Error   3   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  660 1   supertuxkart
Error   4   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  661 1   supertuxkart
Error   5   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  663 1   supertuxkart
Error   6   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  664 1   supertuxkart
Error   7   error C2780: 'void ShadowCommandBuffer::drawIndirect(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided    E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  665 1   supertuxkart
Error   8   error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  676 1   supertuxkart
Error   9   error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  677 1   supertuxkart
Error   10  error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  678 1   supertuxkart
Error   11  error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  679 1   supertuxkart
Error   12  error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  680 1   supertuxkart
Error   13  error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  682 1   supertuxkart
Error   14  error C2780: 'void ShadowCommandBuffer::multidrawShadow(Uniforms...,unsigned int) const' : expects 2 arguments - 1 provided E:\mmg\Developer\Projects\stk\stk-code\src\graphics\draw_calls.cpp  683 1   supertuxkart

It does not seem to live it when variadics arguments are empty. Seems related to placing variadic arguments first in the call, if I moved the variadic argument after the unsigned int parameter then it works, possibly it's too ambiguous

On another note, I see quite a few places where there is a "switch" on material types. Possibly these switches should have a "default" statement that logs a warning, so that if there is a new material type we get a "not supported" warning on the console rather than just nothing happening?

Apart from that, the code works well on my computer (nvidia), I have not spotted any issue, nice work! More tests can be done by other people, of course

Benau commented 8 years ago

Doesn't work in my amd radeonsi driver :( crashed when i choose a kart(where RTT happens)

[warn   ] GLWrap: OpenGL debug callback - API
[warn   ] GLWrap:     Error type : ERROR
[warn   ] GLWrap:     Severity : HIGH
[warn   ] GLWrap:     Message : GL_INVALID_OPERATION in glMapBufferRange(buffer already mapped)

gdb:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000c5ad6f in fillOriginOrientationScale<InstanceDataDualTex> (node=0x23c8c8b0, instance=...)
    at /data/game/stk-code.elderme/src/graphics/command_buffer.hpp:50
50      instance.Origin.X = Origin.X;

instance is empty, further debug: (b command_buffer.hpp:209) (not sure if correct debug, noob in opengl)


Breakpoint 1, CommandBuffer<11>::fillInstanceData<InstanceDataDualTex> (this=0x1d27b80, mesh_map=0x1d26cd8, material_list=..., 
    instance_type=InstanceTypeDualTex) at /data/game/stk-code.elderme/src/graphics/command_buffer.hpp:211
211         if (CVS->supportsAsyncInstanceUpload())
(gdb) n
218             glBindBuffer(GL_ARRAY_BUFFER,
(gdb) p *instance_buffer
$3 = {Origin = {X = 4.41791873e-37, Y = 0, Z = 4.4179277e-37}, Orientation = {X = 0, Y = 4.41793308e-37, Z = 0}, Scale = {X = -7.62396254e+31, 
    Y = 4.59163468e-41, Z = 1.40129846e-45}, Texture = 21474836480, SecondTexture = 18446697825501708288}
(gdb) n
221                 glMapBufferRange(GL_ARRAY_BUFFER, 0,
(gdb) p *instance_buffer
$4 = {Origin = {X = 4.41791873e-37, Y = 0, Z = 4.4179277e-37}, Orientation = {X = 0, Y = 4.41793308e-37, Z = 0}, Scale = {X = -7.62396254e+31, 
    Y = 4.59163468e-41, Z = 1.40129846e-45}, Texture = 21474836480, SecondTexture = 18446697825501708288}
(gdb) n
220             instance_buffer = (InstanceData*)
(gdb) p *instance_buffer
$5 = {Origin = {X = 4.41791873e-37, Y = 0, Z = 4.4179277e-37}, Orientation = {X = 0, Y = 4.41793308e-37, Z = 0}, Scale = {X = -7.62396254e+31, 
    Y = 4.59163468e-41, Z = 1.40129846e-45}, Texture = 21474836480, SecondTexture = 18446697825501708288}
(gdb) n
224             glBindBuffer(GL_DRAW_INDIRECT_BUFFER,
(gdb) p *instance_buffer
Cannot access memory at address 0x7fffdefd7000

seems that my driver(?) doesn't like this line(?):

            instance_buffer = (InstanceData*)
                glMapBufferRange(GL_ARRAY_BUFFER, 0,
                                 10000 * sizeof(InstanceDataDualTex),
                                 GL_MAP_WRITE_BIT | GL_MAP_UNSYNCHRONIZED_BIT | GL_MAP_INVALIDATE_BUFFER_BIT);

Any idea?

deveee commented 8 years ago

@Elderme I will look at your branch, hopefully in the saturday/sunday. I'm rather busy recently :(

hiker commented 8 years ago

Hi,

With the intel integrated card (default settings, i.e. a fresh config file) on windows I get some error messages, though the one race looked alright. Runs done with a clear config file (i.e. default settings):

[info ] IrrDriver: OpenGL version: 4.3 [info ] IrrDriver: OpenGL vendor: Intel [info ] IrrDriver: OpenGL renderer: Intel(R) HD Graphics 4600 [info ] IrrDriver: OpenGL version string: 4.3.0 - Build 10.18.15.4279 ... [info ] shader: Compiling shader : screenquad.vert [info ] shader: Compiling shader : importance_sampling_specular.frag [warn ] GLWrap: OpenGL debug callback - API [warn ] GLWrap: Error type : ERROR [warn ] GLWrap: Severity : HIGH [warn ] GLWrap: Message : Error has been generated. GL error GL_INVALID_OPERATION in DrawArrays: (ID: 3178480681) Generic error [warn ] GLWrap: OpenGL debug callback - API [warn ] GLWrap: Error type : ERROR [warn ] GLWrap: Severity : HIGH [warn ] GLWrap: Message : Error has been generated. GL error GL_INVALID_OPERATION in DrawArrays: (ID: 3178480681) Generic error

... many repeats of this error, then:

[error ] EventHandler: While loading track 'C:\Users\joerg_000\AppData\Roaming/supertuxkart/0.8.2/../addons/tracks/abyss/track.xml' [error ] Irrlicht: GL_INVALID_OPERATION [error ] EventHandler: While loading track 'C:\Users\joerg_000\AppData\Roaming/supertuxkart/0.8.2/../addons/tracks/abyss/track.xml' [error ] Irrlicht: Could not bind Texture [info ] shader: Compiling shader : screenquad.vert

With the nvidia card: [info ] IrrDriver: OpenGL version: 4.3 [info ] IrrDriver: OpenGL vendor: NVIDIA Corporation [info ] IrrDriver: OpenGL renderer: GeForce GTX 960M/PCIe/SSE2 [info ] IrrDriver: OpenGL version string: 4.3.0 NVIDIA 358.91

And no error messages :)

Let me know what you need.

Cheers, Joerg

Elderme commented 8 years ago

Thanks for your answers, I will try to fix these errors.

Hiker, Benau, could you tell me which OpenGL extensions are available with your card? You can see it in the log. Just after the four lines with OpenGL version, OpenGL vendor, etc, you have lines like this:

[info   ] GLDriver: ARB Buffer Storage Present
[info   ] GLDriver: ARB Base Instance Present
[info   ] GLDriver: ARB Draw Indirect Present
[info   ] GLDriver: ARB Compute Shader Present
[info   ] GLDriver: ARB Arrays of Arrays Present
[info   ] GLDriver: ARB Texture Storage Present
[info   ] GLDriver: ARB Texture View Present
[info   ] GLDriver: ARB Image Load Store Present
[info   ] GLDriver: ARB Shader Atomic Counters Present
[info   ] GLDriver: ARB Shader Storage Buffer Object Present
[info   ] GLDriver: ARB Multi Draw Indirect Present
[info   ] GLDriver: EXT Texture Compression S3TC Present
[info   ] GLDriver: ARB Uniform Buffer Object Present
[info   ] GLDriver: ARB Geometry Shader 4 Present
Benau commented 8 years ago

Here you are:

[info   ] IrrDriver: OpenGL version: 4.1
[info   ] IrrDriver: OpenGL vendor: X.Org
[info   ] IrrDriver: OpenGL renderer: Gallium 0.4 on AMD OLAND (DRM 2.43.0, LLVM 3.7.0)
[info   ] IrrDriver: OpenGL version string: 4.1 (Core Profile) Mesa 11.0.2
[info   ] GLDriver: AMD Vertex Shader Layer Present
[info   ] GLDriver: ARB Buffer Storage Present
[info   ] GLDriver: ARB Base Instance Present
[info   ] GLDriver: ARB Draw Indirect Present
[info   ] GLDriver: ARB Texture Storage Present
[info   ] GLDriver: ARB Multi Draw Indirect Present
[info   ] GLDriver: ARB Uniform Buffer Object Present
[info   ] irr_driver: GLSL supported.
Elderme commented 8 years ago

@Benau Thanks! I think I have found what was wrong. I have updated the branch. Could you test it again?

@auriamg I moved the variadic argument in CommandBuffer. Is it now ok with visual studio?

Benau commented 8 years ago

yep it works!, thank you~~

auriamg commented 8 years ago

Thanks! You need to add

#include <array>

to command_buffer.hpp though (I think github stripped that away in my previous message, sorry didn't notice). Then it compiles correctly

deveee commented 8 years ago

I finally found some time to try your branch. Here is what I noticed:

1 - Central video settings are initialized twice:

(...)
Irrlicht Engine version 1.8.0
Linux 4.2.0-27-generic #32-Ubuntu SMP Fri Jan 22 04:49:08 UTC 2016 x86_64
[info   ] IrrDriver: OpenGL version: 4.3
[info   ] IrrDriver: OpenGL vendor: NVIDIA Corporation
[info   ] IrrDriver: OpenGL renderer: GeForce GT 635M/PCIe/SSE2
[info   ] IrrDriver: OpenGL version string: 4.3.0 NVIDIA 352.63
[info   ] GLDriver: ARB Buffer Storage Present
[info   ] GLDriver: ARB Base Instance Present
[info   ] GLDriver: ARB Draw Indirect Present
[info   ] GLDriver: ARB Compute Shader Present
[info   ] GLDriver: ARB Arrays of Arrays Present
[info   ] GLDriver: ARB Texture Storage Present
[info   ] GLDriver: ARB Texture View Present
[info   ] GLDriver: ARB Image Load Store Present
[info   ] GLDriver: ARB Shader Atomic Counters Present
[info   ] GLDriver: ARB Shader Storage Buffer Object Present
[info   ] GLDriver: ARB Multi Draw Indirect Present
[info   ] GLDriver: EXT Texture Compression S3TC Present
[info   ] GLDriver: ARB Uniform Buffer Object Present
[info   ] GLDriver: ARB Geometry Shader 4 Present
[info   ] IrrDriver: OpenGL version: 4.3
[info   ] IrrDriver: OpenGL vendor: NVIDIA Corporation
[info   ] IrrDriver: OpenGL renderer: GeForce GT 635M/PCIe/SSE2
[info   ] IrrDriver: OpenGL version string: 4.3.0 NVIDIA 352.63
[info   ] GLDriver: ARB Buffer Storage Present
[info   ] GLDriver: ARB Base Instance Present
[info   ] GLDriver: ARB Draw Indirect Present
[info   ] GLDriver: ARB Compute Shader Present
[info   ] GLDriver: ARB Arrays of Arrays Present
[info   ] GLDriver: ARB Texture Storage Present
[info   ] GLDriver: ARB Texture View Present
[info   ] GLDriver: ARB Image Load Store Present
[info   ] GLDriver: ARB Shader Atomic Counters Present
[info   ] GLDriver: ARB Shader Storage Buffer Object Present
[info   ] GLDriver: ARB Multi Draw Indirect Present
[info   ] GLDriver: EXT Texture Compression S3TC Present
[info   ] GLDriver: ARB Uniform Buffer Object Present
[info   ] GLDriver: ARB Geometry Shader 4 Present
[info   ] irr_driver: GLSL supported.
(...)

Is there a reason to check it twice?


2 - After changing from graphics level 6 to graphics level 1, the game doesn't work anymore. I see only grey screen.

It doesn't depend on drivers. I see it on both - windows and linux.

I don't see any errors in console, just typical messages. The kart selection screen looks like it still uses high graphics level.


3 - When I set force_legacy_device parameter in config.xml, the karts in kart selection screen are semi-transparent. zrzut ekranu z 2016-02-07 15-21-44

Again I noticed the same behaviour with different configurations (nvidia/intel/linux/windows).

Though it's still better because previously it wasn't working at all ;)


4 - Cannot compile code with gcc 4.7.2 (Debian Wheezy):

[ 44%] Building CXX object CMakeFiles/supertuxkart.dir/src/graphics/command_buffer.cpp.o
/home/Dokumenty/programowanie/supertuxkart/supertuxkart-git/src/graphics/command_buffer.cpp: In function 'void expandTexSecondPass(const GLMesh&, const std::vector<unsigned int>&) [with T = GrassMat]':
/home/Dokumenty/programowanie/supertuxkart/supertuxkart-git/src/graphics/command_buffer.cpp:59:64: error: 'template' (as a disambiguator) is only allowed within templates
/home/Dokumenty/programowanie/supertuxkart/supertuxkart-git/src/graphics/command_buffer.cpp: In function 'void expandHandlesSecondPass(const std::vector<long long unsigned int>&) [with T = GrassMat]':
/home/Dokumenty/programowanie/supertuxkart/supertuxkart-git/src/graphics/command_buffer.cpp:68:58: error: 'template' (as a disambiguator) is only allowed within templates
make[2]: *** [CMakeFiles/supertuxkart.dir/src/graphics/command_buffer.cpp.o] Error 1
make[1]: *** [CMakeFiles/supertuxkart.dir/all] Error 2
make: *** [all] Error 2

Well, it's not critical, because gcc compiler in Wheezy is rather outdated (version 4.7.2). But I use it for creating linux package. I prefer to use older distribution because it means that we require relatively old libgcc/libstdc++/glibc and this allows to run the game on more operating systems.

If the fix is not trivial, then I will just need switch to Debian Jessie.


5 - Linking error on Windows with MinGW 5.2.0:

[ 45%] Linking CXX executable bin\supertuxkart.exe
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text+0x2732): undefined reference to
`CommandBuffer<11>::~Comma
ndBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text+0x2749): undefined reference to
`CommandBuffer<44>::~Comma
ndBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text+0x275f): undefined reference to
`CommandBuffer<11>::~Comma
ndBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text$_ZN9DrawCallsD1Ev[__ZN9DrawCallsD1Ev]+0x16):
undefined ref
erence to `CommandBuffer<1>::~CommandBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text$_ZN9DrawCallsD1Ev[__ZN9DrawCallsD1Ev]+0x2b):
undefined ref
erence to `CommandBuffer<11>::~CommandBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text$_ZN9DrawCallsD1Ev[__ZN9DrawCallsD1Ev]+0x40):
undefined ref
erence to `CommandBuffer<44>::~CommandBuffer()'
CMakeFiles\supertuxkart.dir/objects.a(shader_based_renderer.cpp.obj):shader_base
d_renderer.cpp:(.text$_ZN9DrawCallsD1Ev[__ZN9DrawCallsD1Ev]+0x55):
undefined ref
erence to `CommandBuffer<11>::~CommandBuffer()'
collect2.exe: error: ld returned 1 exit status
CMakeFiles\supertuxkart.dir\build.make:10123: recipe for target
'bin/supertuxkar
t.exe' failed
mingw32-make[2]: *** [bin/supertuxkart.exe] Error 1
CMakeFiles\Makefile2:105: recipe for target
'CMakeFiles/supertuxkart.dir/all' fa
iled
mingw32-make[1]: *** [CMakeFiles/supertuxkart.dir/all] Error 2
makefile:126: recipe for target 'all' failed
mingw32-make: *** [all] Error 2

This error is rather strange because it works for me without problems when I cross-compile Windows 32-bit and 64-bit binaries using MinGW 4.9.2. I must check if this is something specific to this one compiler version.

I didn't look at your code yet. I hope that I will find some time to do it soon.

hiker commented 8 years ago

Hi,

sorry for the long delay, I'm just too busy atm :(

With hd4600: [info ] IrrDriver: OpenGL vendor: Intel [info ] IrrDriver: OpenGL renderer: Intel(R) HD Graphics 4600 [info ] IrrDriver: OpenGL version string: 4.3.0 - Build 20.19.15.4331 [info ] GLDriver: ARB Buffer Storage Present [info ] GLDriver: ARB Base Instance Present [info ] GLDriver: ARB Draw Indirect Present [info ] GLDriver: ARB Compute Shader Present [info ] GLDriver: ARB Arrays of Arrays Present [info ] GLDriver: ARB Texture Storage Present [info ] GLDriver: ARB Texture View Present [info ] GLDriver: ARB Bindless Texture Present [info ] GLDriver: ARB Image Load Store Present [info ] GLDriver: ARB Shader Atomic Counters Present [info ] GLDriver: ARB Shader Storage Buffer Object Present [info ] GLDriver: ARB Multi Draw Indirect Present [info ] GLDriver: EXT Texture Compression S3TC Present [info ] GLDriver: ARB Uniform Buffer Object Present [info ] GLDriver: ARB Geometry Shader 4 Present

Hmm - but similar to what Deve has mentioned this line appears twice, so it looks like it is initialising opengl twice(?)

I had some problems getting it to work because of a crash in addSkyBox

The top of the stack trace is certainly invalid, the parameters to printMessage are garbage.

If I can get past this point, it is running mostly fine (all with a clean config file). I see many error messages: [info ] shader: Compiling shader : tonemap.frag [warn ] LayoutManager: Statically sized widgets took all the place!! [verbose ] RaceManager: Nb of karts=1, ai:0 players:1

[info ] Singleton: Destroyed singleton. [info ] shader: Compiling shader : screenquad.vert [info ] shader: Compiling shader : importance_sampling_specular.frag [warn ] GLWrap: OpenGL debug callback - API [warn ] GLWrap: Error type : ERROR [warn ] GLWrap: Severity : HIGH [warn ] GLWrap: Message : Error has been generated. GL error GL_INVALID_OPERATION in DrawArrays: (ID: 3178480681) Generic error [warn ] GLWrap: OpenGL debug callback - API [warn ] GLWrap: Error type : ERROR [warn ] GLWrap: Severity : HIGH [warn ] GLWrap: Message : Error has been generated. GL error GL_INVALID_OPERATION in DrawArrays: (ID: 3178480681) Generic error ... [warn ] GLWrap: OpenGL debug callback - API [warn ] GLWrap: Error type : ERROR [warn ] GLWrap: Severity : HIGH [warn ] GLWrap: Message : Error has been generated. GL error GL_INVALID_OPERATION in DrawArrays: (ID: 3178480681) Generic error [error ] EventHandler: While loading track '../data/../../stk-assets/tracks/olivermath/track.xml' [error ] Irrlicht: GL_INVALID_OPERATION [error ] EventHandler: While loading track '../data/../../stk-assets/tracks/olivermath/track.xml' [error ] Irrlicht: Could not bind Texture [info ] shader: Compiling shader : screenquad.vert [info ] shader: Compiling shader : utils/getPosFromUVDepth.frag [info ] shader: Compiling shader : fog.frag [info ] shader: Compiling shader : sky.vert [info ] shader: Compiling shader : sky.frag [info ] shader: Compiling shader : pointlight.vert [info ] shader: Compiling shader : utils/getPosFromUVDepth.frag [info ] shader: Compiling shader : pointlightscatter.frag [info ] shader: Compiling shader : gaussian6v.comp [warn ] GLWrap: OpenGL debug callback - SHADER_COMPILER [warn ] GLWrap: Error type : OTHER [warn ] GLWrap: Severity : MEDIUM [warn ] GLWrap: Message : SHADER_ID_COMPILE other warning has been generated. GLSL compile warning(s) for shader 188, "": WARNING: 0:77: 'image format' : redundant image format layout qualifier

[info ] shader: Compiling shader : gaussian6h.comp [warn ] GLWrap: OpenGL debug callback - SHADER_COMPILER [warn ] GLWrap: Error type : OTHER [warn ] GLWrap: Severity : MEDIUM [warn ] GLWrap: Message : SHADER_ID_COMPILE other warning has been generated. GLSL compile warning(s) for shader 190, "": WARNING: 0:77: 'image format' : redundant image format layout qualifier

[info ] shader: Compiling shader : object_pass.vert [info ] shader: Compiling shader : transparentfog.frag[info ] shader: Compiling shader : pointemitter.vert [info ] shader: Compiling shader : particle.vert [info ] shader: Compiling shader : utils/getPosFromUVDepth.frag [info ] shader: Compiling shader : particle.frag [info ] shader: Compiling shader : primitive2dlist.vert [info ] shader: Compiling shader : transparent.frag

With Nvidia: [info ] IrrDriver: OpenGL version: 4.3 [info ] IrrDriver: OpenGL vendor: NVIDIA Corporation [info ] IrrDriver: OpenGL renderer: GeForce GTX 960M/PCIe/SSE2 [info ] IrrDriver: OpenGL version string: 4.3.0 NVIDIA 358.91

no problems at all (except that shadows are disabled by default - graphics setting 3 - maybe we should make shadows the default by now???)

Cheers, Joerg

Elderme commented 8 years ago

@deveee

1 - Central video settings are initialized twice

There was no reason to check it twice. I removed a call to CVS init.

2 - After changing from graphics level 6 to graphics level 1, the game doesn't work anymore. I see only grey screen.

Fixed. Can you confirm it works?

3 - When I set force_legacy_device parameter in config.xml, the karts in kart selection screen are semi-transparent.

Yes, it seems there is a wrong blending parameter somewhere with legacy device, but I didn't manage to find it :/

4 - 5 -

I will try to understand these errors, but I often have trouble with templates and compilation.

@hiker: I will try to find what is wrong this week-end.

deveee commented 8 years ago

@Elderme

1, 2 - It works fine now, thanks!

5 - I have no idea why it works when I cross-compile it, but it doesn't work when I use the same compiler version directly on Windows. Though I solved it by moving CommandBuffer destructor to header file:

diff --git a/src/graphics/command_buffer.cpp b/src/graphics/command_buffer.cpp
index 29ed13f..6cb4cec 100644
--- a/src/graphics/command_buffer.cpp
+++ b/src/graphics/command_buffer.cpp
@@ -120,12 +120,6 @@ m_poly_count(0)
     }    
 }

-template<int N>
-CommandBuffer<N>::~CommandBuffer()
-{
-    glDeleteBuffers(1, &m_draw_indirect_cmd_id);
-}
-
 SolidCommandBuffer::SolidCommandBuffer(): CommandBuffer()
 {
 }
diff --git a/src/graphics/command_buffer.hpp b/src/graphics/command_buffer.hpp
index f95738d..dfd8885 100644
--- a/src/graphics/command_buffer.hpp
+++ b/src/graphics/command_buffer.hpp
@@ -240,7 +240,7 @@ protected:

 public:
     CommandBuffer();
-    virtual ~CommandBuffer();
+    virtual ~CommandBuffer() { glDeleteBuffers(1, &m_draw_indirect_cmd_id); }

     inline size_t getPolyCount() const {return m_poly_count;}
Elderme commented 8 years ago

Hi,

I tried to understand the errors with Intel hardware, but I have no idea why there is a problem with DrawArrays. According to the specification, glDrawArrays generate an invalid operation (if a non-zero buffer object name is bound to an enabled array and the buffer object's data store is currently mapped" (but all buffers seems to be unmapped when they need to be unmapped) or "if a geometry shader is active and mode is incompatible with the input primitive type of the geometry shader in the currently installed program object" (but I don't see any problem with geometry shaders).

Are there other things I should fix before making a pull request?

Concerning semi-transparent karts in kart selection screen with force_legacy_device, I think we should open another issue, and fix it in a different branch.

Benau commented 8 years ago

Notice: I tested your branch in windows (using 4770k onboard GPU), and it works! (tested with highest settings too)

nado commented 8 years ago

Tested, got a compilation error

[ 65%] Building CXX object CMakeFiles/supertuxkart.dir/src/graphics/post_processing.cpp.o
/home/nado/dev/github/stk-code/src/graphics/lighting_passes.cpp: In member function ‘void ShadowedSunLightShaderPCF::render(GLuint, GLuint, const FrameBuffer&)’:
/home/nado/dev/github/stk-code/src/graphics/lighting_passes.cpp:339:35: error: ‘UserConfigParams’ was not declared in this scope
                             float(UserConfigParams::m_shadows_resolution)   );
                                   ^
/home/nado/dev/github/stk-code/src/graphics/lighting_passes.cpp: In member function ‘void LightingPasses::renderEnvMap(GLuint, GLuint, GLuint)’:
/home/nado/dev/github/stk-code/src/graphics/lighting_passes.cpp:454:9: error: ‘UserConfigParams’ has not been declared
     if (UserConfigParams::m_degraded_IBL)
         ^
CMakeFiles/supertuxkart.dir/build.make:3398: recipe for target 'CMakeFiles/supertuxkart.dir/src/graphics/lighting_passes.cpp.o' failed
make[2]: *** [CMakeFiles/supertuxkart.dir/src/graphics/lighting_passes.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
CMakeFiles/Makefile2:73: recipe for target 'CMakeFiles/supertuxkart.dir/all' failed
make[1]: *** [CMakeFiles/supertuxkart.dir/all] Error 2
Makefile:127: recipe for target 'all' failed
make: *** [all] Error 2

GCC 5.3.0 linux.

Benau commented 8 years ago

Include config/user_config.hpp

nado commented 8 years ago

Changes needed in order to compile on my computer. Didnt test it yet, will do in the evening.

diff --git a/src/graphics/draw_calls.cpp b/src/graphics/draw_calls.cpp
index e12f47d..66eb304 100644
--- a/src/graphics/draw_calls.cpp
+++ b/src/graphics/draw_calls.cpp
@@ -15,6 +15,7 @@
 //  along with this program; if not, write to the Free Software
 //  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.

+#include "config/user_config.hpp"
 #include "graphics/draw_calls.hpp"
 #include "graphics/draw_tools.hpp"
 #include "graphics/gpu_particles.hpp"
diff --git a/src/graphics/lighting_passes.cpp b/src/graphics/lighting_passes.cpp
index 3ba307d..29c108a 100644
--- a/src/graphics/lighting_passes.cpp
+++ b/src/graphics/lighting_passes.cpp
@@ -15,6 +15,7 @@
 //  along with this program; if not, write to the Free Software
 //  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.

+#include "config/user_config.hpp"
 #include "graphics/lighting_passes.hpp"
 #include "graphics/central_settings.hpp"
 #include "graphics/irr_driver.hpp"
diff --git a/src/graphics/shader_based_renderer.cpp b/src/graphics/shader_based_renderer.cpp
index c1f0532..14ae393 100644
--- a/src/graphics/shader_based_renderer.cpp
+++ b/src/graphics/shader_based_renderer.cpp
@@ -15,6 +15,7 @@
 //  along with this program; if not, write to the Free Software
 //  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.

+#include "config/user_config.hpp"
 #include "graphics/shader_based_renderer.hpp"

 #include "graphics/central_settings.hpp"
nado commented 8 years ago

Seems working fine. A few warnings, i attached the log. stdout.log.txt

MatthewsSam commented 8 years ago

I have a laptop with windows, I will try on Intel to gather more data.

auriamg commented 8 years ago

I tested on my intel HD 3000 laptop, I do get some issues

RTT on the kart selection screen is broken, I get a "GL_INVALID_FRAMEBUFFER_OPERATION" (haven't debugged more yet)

But a more important issue is that it crashes in particles, in ParticleSystemProxy::CommonRenderingVAO (around the "glVertexAttribDivisorARB" line), reason unknown

So unfortunately those issues prevent us from merging this branch :(

qwertychouskie commented 8 years ago

Any updates on this?

Elderme commented 7 years ago

Sorry, since a few months I don't have enough time to work on STK. I'd like to fix the issue with Intel drivers, but I really don't know when I will have time to do this.

Elderme commented 7 years ago

Hi,

I've just updated my branch with master. I had to solved conflicts, I hope I've managed them correctly. I've seen @deveee added a check for GL_ARB_explicit_attrib_location, if he can have a look at my code and check if it is ok? It was in one file with conflicts, and I cannot be sure that I have merged correctly for this point because the extension is available on my computer.

I've tested with my desktop computer (linux, nvidia graphics card) and with my laptop (linux with intel, but it is mesa 11.2, there are some graphical restrictions: I cannot test everything). It works on both, I cannot reproduce Auria issues, so it is difficult to fix them :(

@auriamg , could you please test again and tell me where you have this "GL_INVALID_FRAMEBUFFER_OPERATION" in RTT? And if you can give me more information about the crash with particles? Have you OpenGL errors before the crash?

auriamg commented 7 years ago

This is the trace for invalid framebuffer operation. Contrarily to what I thought, it isn't actually on the kart selection screen (which doesn't work very well, the RTT is weirdly colored and doesn't update, but it doesn't crash) but on the screen right after kart selection screen

    supertuxkart.exe!irr::video::COpenGLDriver::testGLError() Line 2643 C++
    supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::COpenGLTexture(irr::video::IImage * origImage, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData, irr::video::COpenGLDriver * driver) Line 53 C++
    supertuxkart.exe!irr::video::COpenGLDriver::createDeviceDependentTexture(irr::video::IImage * surface, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData) Line 2599    C++
    supertuxkart.exe!irr::video::CNullDriver::loadTextureFromFile(irr::io::IReadFile * file, const irr::core::string<char,irr::core::irrAllocator<char> > & hashName) Line 475  C++
    supertuxkart.exe!irr::video::CNullDriver::getTexture(const irr::core::string<char,irr::core::irrAllocator<char> > & filename) Line 418  C++
    supertuxkart.exe!IrrDriver::getTexture(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & filename, bool is_premul, bool is_prediv, bool complain_if_not_found) Line 1564 C++
>   supertuxkart.exe!GUIEngine::LayoutManager::readCoords(GUIEngine::Widget * self) Line 156    C++
    supertuxkart.exe!GUIEngine::RibbonWidget::add() Line 112    C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 87  C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 66  C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 66  C++
    supertuxkart.exe!GUIEngine::Screen::addWidgets() Line 194   C++
    supertuxkart.exe!GUIEngine::switchToScreen(const char * screen_name) Line 910   C++
    supertuxkart.exe!GUIEngine::AbstractStateManager::pushMenu(std::basic_string<char,std::char_traits<char>,std::allocator<char> > name) Line 113  C++
    supertuxkart.exe!GUIEngine::AbstractStateManager::pushScreen(GUIEngine::Screen * screen) Line 134   C++
    supertuxkart.exe!GUIEngine::Screen::push() Line 104 C++
    supertuxkart.exe!KartSelectionScreen::allPlayersDone() Line 1224    C++
    supertuxkart.exe!KartSelectionScreen::playerConfirm(const int player_id) Line 800   C++
    supertuxkart.exe!KartSelectionScreen::eventCallback(GUIEngine::Widget * widget, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & name, const int player_id) Line 1042   C++
    supertuxkart.exe!GUIEngine::EventHandler::sendEventToUser(GUIEngine::Widget * widget, std::basic_string<char,std::char_traits<char>,std::allocator<char> > & name, const int playerID) Line 528 C++
    supertuxkart.exe!GUIEngine::EventHandler::onWidgetActivated(GUIEngine::Widget * w, const int playerID) Line 571 C++
    supertuxkart.exe!GUIEngine::EventHandler::onGUIEvent(const irr::SEvent & event) Line 613    C++
    supertuxkart.exe!GUIEngine::EventHandler::OnEvent(const irr::SEvent & event) Line 149   C++
    supertuxkart.exe!irr::gui::CGUIEnvironment::OnEvent(const irr::SEvent & event) Line 399 C++
    supertuxkart.exe!irr::gui::CGUIButton::OnEvent(const irr::SEvent & event) Line 225  C++
    supertuxkart.exe!irr::gui::CGUIButton::OnEvent(const irr::SEvent & event) Line 215  C++
    supertuxkart.exe!irr::gui::CGUIEnvironment::postEventFromUser(const irr::SEvent & event) Line 570   C++
    supertuxkart.exe!irr::CIrrDeviceStub::postEventFromUser(const irr::SEvent & event) Line 227 C++
    supertuxkart.exe!WndProc(HWND__ * hWnd, unsigned int message, unsigned int wParam, long lParam) Line 813    C++
    [External Code]
    [Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]   
    supertuxkart.exe!irr::CIrrDeviceWin32::handleSystemMessages() Line 1953 C++
    supertuxkart.exe!irr::CIrrDeviceWin32::run() Line 1310  C++
    supertuxkart.exe!IrrDriver::update(float dt) Line 1923  C++
    supertuxkart.exe!MainLoop::run() Line 177   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1663    C++
    [External Code]

as for the particles issue, I also get a GL_INVALID_FRAMEBUFFER_OPERATION

>   supertuxkart.exe!irr::video::COpenGLDriver::testGLError() Line 2643 C++
    supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::unlock() Line 547  C++
    supertuxkart.exe!compressTexture(irr::video::ITexture * tex, bool srgb, bool premul_alpha) Line 74  C++
    supertuxkart.exe!STKMeshSceneNode::render() Line 497    C++
    supertuxkart.exe!DrawCalls::renderImmediateDrawList() Line 581  C++
    supertuxkart.exe!AbstractGeometryPasses::renderTransparent(const DrawCalls & draw_calls, const FrameBuffer & tmp_framebuffer, const FrameBuffer & displace_framebuffer, const FrameBuffer & colors_framebuffer, const PostProcessing * post_processing) Line 251    C++
    supertuxkart.exe!ShaderBasedRenderer::renderScene(irr::scene::ICameraSceneNode * const camnode, float dt, bool hasShadow, bool forceRTT) Line 493   C++
    supertuxkart.exe!ShaderBasedRenderer::render(float dt) Line 825 C++
    supertuxkart.exe!IrrDriver::update(float dt) Line 1955  C++
    supertuxkart.exe!MainLoop::run() Line 177   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1663    C++

Unfortunartely I have no further information, no other details

I also got another variation, this time related to billboards it seems

>   supertuxkart.exe!irr::video::COpenGLDriver::testGLError() Line 2643 C++
    supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::unlock() Line 547  C++
    supertuxkart.exe!compressTexture(irr::video::ITexture * tex, bool srgb, bool premul_alpha) Line 74  C++
    supertuxkart.exe!STKBillboard::render() Line 111    C++
    supertuxkart.exe!DrawCalls::renderBillboardList() Line 588  C++
    supertuxkart.exe!AbstractGeometryPasses::renderTransparent(const DrawCalls & draw_calls, const FrameBuffer & tmp_framebuffer, const FrameBuffer & displace_framebuffer, const FrameBuffer & colors_framebuffer, const PostProcessing * post_processing) Line 282    C++
    supertuxkart.exe!ShaderBasedRenderer::renderScene(irr::scene::ICameraSceneNode * const camnode, float dt, bool hasShadow, bool forceRTT) Line 493   C++
    supertuxkart.exe!ShaderBasedRenderer::render(float dt) Line 825 C++
    supertuxkart.exe!IrrDriver::update(float dt) Line 1955  C++
    supertuxkart.exe!MainLoop::run() Line 177   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1663    C++

However, both of these are warnings, and I no longer get crashes! Not sure what changed since last time. But now it actually seems to be fully playable, without crashing!

Benau commented 7 years ago

In your branch:

This branch is 82 commits ahead, 441 commits behind supertuxkart:master.

I think you you did a bad resolving conflict... it should be 82 commits ahead. (without commits behind supertuxkart:master)

Elderme commented 7 years ago

Indeed I did something wrong, sorry. I have merged again, now it is 83 commits ahead of supertuxkart:master.

Thanks Auria, I will try to fix these warnings. I don't know why it doesn't crash anymore, maybe it is related to deveee's work.

deveee commented 7 years ago

@Elderme The check for GL_ARB_explicit_attrib_location extension has been added in this commit: https://github.com/supertuxkart/stk-code/commit/41283ad408ad23d4178e89796e6d888ea569a884

This extension is needed for InstancedColorizeShader and also for some of our geometry shaders (basically for shadows).

If you want to test it, you can override opengl version to 3.2 in mesa or disable this extension using MESA_EXTENSION_OVERRIDE.

Btw. the alpha blending issue in kart selection screen in legacy pipeline is now a regression because it works fine in current git ;)

The problem with locking/unlocking textures may be related to glActiveTexture() command. We set it in few different places and then don't restore it to glActiveTexture(GL_TEXTURE0). Irrlicht expects that it is set to GL_TEXTURE0.

Elderme commented 7 years ago

Thanks deveee. I didn't know you can disable an extension with command line, it will be usefull! It seems to work without GL_ARB_explicit_attrib_location extension :)

Btw. the alpha blending issue in kart selection screen in legacy pipeline is now a regression because it works fine in current git ;)

There is no more this issue in my branch (at least with my computers... hope it will works for other people too!). I have looked at what you have done concerning kart selection screen with legacy pipeline, and I have adapted it in my branch.

Concerning the GL_INVALID_FRAMEBUFFER_OPERATION, according to OpenGL specification, you can have this error "when doing anything that would attempt to read from or write/render to a framebuffer that is not complete." That is strange because when a framebuffer is created in the Framebuffer class there is next a call to glCheckFramebufferStatus in order to check if the framebuffer is complete.

Actually we didn't call glCheckFramebufferStatus for layered framebuffers, so I have just added it. But I don't think layered framebuffers are related to Auria issue: in the engine, we use a layered framebuffer to blur shadows (and not for every configuration), it is not related to particles or billboards.

The problem with locking/unlocking textures may be related to glActiveTexture() command. We set it in few different places and then don't restore it to glActiveTexture(GL_TEXTURE0). Irrlicht expects that it is set to GL_TEXTURE0.

Thanks. I will look for calls to glActiveTexture(...) without calls to glActiveTexture(GL_TEXTURE0).

Elderme commented 7 years ago

I added calls to glActiveTexture(GL_TEXTURE0) before calls to Irrlicht functions in renderBillboardList() and renderImmediateDrawList().

@auriamg , can you tell me if you have less OpenGL errors?

auriamg commented 7 years ago

Sorry, I got something wrong in my previous post :( The particles crash is actually not fixed, the reason why I no longer had it was simply because I had commented that code out in order to be able to continue testing, and I had forgotten that there. Sorry for the confusion!

So I still get aninvalid framebuffer operation

>   supertuxkart.exe!irr::video::COpenGLDriver::testGLError() Line 2643 C++
    supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::COpenGLTexture(irr::video::IImage * origImage, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData, irr::video::COpenGLDriver * driver) Line 53 C++
    supertuxkart.exe!irr::video::COpenGLDriver::createDeviceDependentTexture(irr::video::IImage * surface, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData) Line 2599    C++
    supertuxkart.exe!irr::video::CNullDriver::loadTextureFromFile(irr::io::IReadFile * file, const irr::core::string<char,irr::core::irrAllocator<char> > & hashName) Line 475  C++
    supertuxkart.exe!irr::video::CNullDriver::getTexture(const irr::core::string<char,irr::core::irrAllocator<char> > & filename) Line 418  C++
    supertuxkart.exe!IrrDriver::getTexture(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & filename, bool is_premul, bool is_prediv, bool complain_if_not_found) Line 1564 C++
    supertuxkart.exe!GUIEngine::LayoutManager::readCoords(GUIEngine::Widget * self) Line 156    C++
    supertuxkart.exe!GUIEngine::RibbonWidget::add() Line 112    C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 87  C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 66  C++
    supertuxkart.exe!GUIEngine::AbstractTopLevelContainer::addWidgetsRecursively(PtrVector<GUIEngine::Widget,1> & widgets, GUIEngine::Widget * parent) Line 66  C++
    supertuxkart.exe!GUIEngine::Screen::addWidgets() Line 194   C++
    supertuxkart.exe!GUIEngine::switchToScreen(const char * screen_name) Line 910   C++
    supertuxkart.exe!GUIEngine::AbstractStateManager::pushMenu(std::basic_string<char,std::char_traits<char>,std::allocator<char> > name) Line 113  C++
    supertuxkart.exe!GUIEngine::AbstractStateManager::pushScreen(GUIEngine::Screen * screen) Line 134   C++
    supertuxkart.exe!GUIEngine::Screen::push() Line 104 C++
    supertuxkart.exe!KartSelectionScreen::allPlayersDone() Line 1224    C++
    supertuxkart.exe!KartSelectionScreen::playerConfirm(const int player_id) Line 800   C++
    supertuxkart.exe!KartSelectionScreen::eventCallback(GUIEngine::Widget * widget, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & name, const int player_id) Line 1042   C++
    supertuxkart.exe!GUIEngine::EventHandler::sendEventToUser(GUIEngine::Widget * widget, std::basic_string<char,std::char_traits<char>,std::allocator<char> > & name, const int playerID) Line 528 C++
    supertuxkart.exe!GUIEngine::EventHandler::onWidgetActivated(GUIEngine::Widget * w, const int playerID) Line 571 C++
    supertuxkart.exe!GUIEngine::EventHandler::onGUIEvent(const irr::SEvent & event) Line 613    C++
    supertuxkart.exe!GUIEngine::EventHandler::OnEvent(const irr::SEvent & event) Line 149   C++
    supertuxkart.exe!irr::gui::CGUIEnvironment::OnEvent(const irr::SEvent & event) Line 399 C++
    supertuxkart.exe!irr::gui::CGUIButton::OnEvent(const irr::SEvent & event) Line 225  C++
    supertuxkart.exe!irr::gui::CGUIButton::OnEvent(const irr::SEvent & event) Line 215  C++
    supertuxkart.exe!irr::gui::CGUIEnvironment::postEventFromUser(const irr::SEvent & event) Line 570   C++
    supertuxkart.exe!irr::CIrrDeviceStub::postEventFromUser(const irr::SEvent & event) Line 227 C++
    supertuxkart.exe!WndProc(HWND__ * hWnd, unsigned int message, unsigned int wParam, long lParam) Line 813    C++
    [External Code] 
    [Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]    
    supertuxkart.exe!irr::CIrrDeviceWin32::handleSystemMessages() Line 1953 C++
    supertuxkart.exe!irr::CIrrDeviceWin32::run() Line 1310  C++
    supertuxkart.exe!IrrDriver::update(float dt) Line 1923  C++
    supertuxkart.exe!MainLoop::run() Line 177   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1663    C++
    [External Code] 

and that's the crash :


        00000000()  Unknown
    [Frames below may be incorrect and/or missing]  
    supertuxkart.exe!ParticleSystemProxy::CommonRenderingVAO(unsigned int PositionBuffer) Line 424  C++
>   supertuxkart.exe!ParticleSystemProxy::generateVAOs() Line 605   C++
    supertuxkart.exe!ParticleSystemProxy::render() Line 625 C++
    supertuxkart.exe!DrawCalls::renderParticlesList() Line 598  C++
    supertuxkart.exe!ShaderBasedRenderer::renderParticles() Line 527    C++
    supertuxkart.exe!ShaderBasedRenderer::renderScene(irr::scene::ICameraSceneNode * const camnode, float dt, bool hasShadow, bool forceRTT) Line 503   C++
    supertuxkart.exe!ShaderBasedRenderer::render(float dt) Line 825 C++
    supertuxkart.exe!IrrDriver::update(float dt) Line 1955  C++
    supertuxkart.exe!MainLoop::run() Line 177   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1663    C++
    [External Code]
deveee commented 7 years ago

@auriamg The crash in particles is a different problem. But could you check if do you still have problems with unlocking textures? I mean:

    supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::unlock() Line 547  C++

We had the same error in our master branch after skybox refactoring and I solved it using glActiveTexture(GL_TEXTURE0) command.

auriamg commented 7 years ago

@deveee I'm not sure what you want me to check exactly or how. I still get the invalid framebuffer warnings as mentionned

Elderme commented 7 years ago

@auriamg We think there are two different issues: one related to particles (which makes the game crash), another related to the active texture when renderImmediateDrawList() or renderBillboardList() functions are called (with just an OpenGL error but no crash). Can you tell me is you still have errors with renderImmediateDrawList() and renderBillboardList()?

You said RTT is weirdly colored in kart selection screen: could you make a screenshot? It may help me to understand what happens. I'm also interested in your graphics settings and available OpenGL extensions for your graphics card: if you can just copy the beginning of the log, where you have lines like this:

[info   ] IrrDriver: OpenGL version: 4.3
[info   ] IrrDriver: OpenGL vendor: NVIDIA Corporation
[info   ] IrrDriver: OpenGL renderer: GeForce GTX 560 Ti/PCIe/SSE2
[info   ] IrrDriver: OpenGL version string: 4.3.0 NVIDIA 361.28
[info   ] GLDriver: ARB Buffer Storage Present
[info   ] GLDriver: ARB Base Instance Present
[info   ] GLDriver: ARB Draw Indirect Present
[info   ] GLDriver: ARB Compute Shader Present
[info   ] GLDriver: ARB Arrays of Arrays Present
[info   ] GLDriver: ARB Texture Storage Present
[info   ] GLDriver: ARB Texture View Present
[info   ] GLDriver: ARB Image Load Store Present
[info   ] GLDriver: ARB Shader Atomic Counters Present
[info   ] GLDriver: ARB Shader Storage Buffer Object Present
[info   ] GLDriver: ARB Multi Draw Indirect Present
[info   ] GLDriver: EXT Texture Compression S3TC Present
[info   ] GLDriver: ARB Uniform Buffer Object Present
[info   ] GLDriver: ARB Explicit Attrib Location Present
[info   ] GLDriver: Geometry Shaders Present
[info   ] irr_driver: GLSL supported.
auriamg commented 7 years ago

Yes I understand that we are dealing with separate issues here, i'm just reporting all issues that I can find :) I can still reproduce all issues I posted in https://github.com/supertuxkart/stk-code/issues/2225#issuecomment-227028086 , that includes the warnings in "unlock" (RenderImmediateDrawList and renderBillboardList)

Here's the extensions info from the log :

[info   ] IrrDriver: OpenGL version: 3.1
[info   ] IrrDriver: OpenGL vendor: Intel
[info   ] IrrDriver: OpenGL renderer: Intel(R) HD Graphics Family
[info   ] IrrDriver: OpenGL version string: 3.1.0 - Build 8.15.10.2455
[info   ] GLDriver: ARB Uniform Buffer Object Present
[warn   ] [IrrDriver Temp Logger]: Level 3: GL_INVALID_ENUM
[warn   ] [IrrDriver Temp Logger]: Level 3: Could not bind Texture
[info   ] irr_driver: GLSL supported.

And here's a screenshot : 15f5e83e81571d8567ed81620d8be390

Elderme commented 7 years ago

Thanks @auriamg

I don't understand what is wrong :(

Do you have the GL_INVALID_ENUM at the beginning of the log when you run the game with the master branch? Or is it another issue from my branch?

auriamg commented 7 years ago

I'm afraid the GL_INVALID_ENUM warning doesn't occur on master either.

Here's where this warning is logged :

    supertuxkart.exe!irr::video::COpenGLDriver::testGLError() Line 2628 C++
>   supertuxkart.exe!irr::video::COpenGLTexture::uploadTexture(bool newTexture, void * mipmapData, unsigned int level) Line 354 C++
    supertuxkart.exe!irr::video::COpenGLTexture::COpenGLTexture(irr::video::IImage * origImage, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData, irr::video::COpenGLDriver * driver) Line 53 C++
    supertuxkart.exe!irr::video::COpenGLDriver::createDeviceDependentTexture(irr::video::IImage * surface, const irr::core::string<char,irr::core::irrAllocator<char> > & name, void * mipmapData) Line 2599    C++
    supertuxkart.exe!irr::video::CNullDriver::loadTextureFromFile(irr::io::IReadFile * file, const irr::core::string<char,irr::core::irrAllocator<char> > & hashName) Line 475  C++
    supertuxkart.exe!irr::video::CNullDriver::getTexture(const irr::core::string<char,irr::core::irrAllocator<char> > & filename) Line 418  C++
    supertuxkart.exe!IrrDriver::getTexture(const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & filename, bool is_premul, bool is_prediv, bool complain_if_not_found) Line 1564 C++
    supertuxkart.exe!IrrDriver::getTexture(FileManager::AssetType type, const std::basic_string<char,std::char_traits<char>,std::allocator<char> > & filename, bool is_premul, bool is_prediv, bool complain_if_not_found) Line 1544    C++
    supertuxkart.exe!AbstractGeometryPasses::AbstractGeometryPasses() Line 178  C++
    [External Code]
    supertuxkart.exe!ShaderBasedRenderer::ShaderBasedRenderer() Line 645    C++
    supertuxkart.exe!IrrDriver::initDevice() Line 522   C++
    supertuxkart.exe!initRest() Line 1285   C++
    supertuxkart.exe!main(int argc, char * * argv) Line 1456    C++
    [External Code]
    [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
deveee commented 7 years ago

@Elderme I noticed that you added glActiveTexture(GL_TEXTURE0); before render calls.

The problem which I had was precisely this one: https://github.com/supertuxkart/stk-code/issues/2504 (of course the title doesn't have a sense :P)

When glActiveTexture was set to for example:

glActiveTexture(GL_TEXTURE0 + SpecularIBLGenerator::getInstance()->m_tu_samples);

And then not restored to:

glActiveTexture(GL_TEXTURE0);

then the command:

m_spherical_harmonics_textures[idx]->unlock();

was producing an error:

Mesa: User error: GL_INVALID_VALUE in glTexSubImage2D(xoffset+width)

because of function:

uploadTexture(false, 0, MipLevelStored);

which is available in:

void COpenGLTexture::unlock()

So if this is the same issue, then it's not related to render calls, but rather to places where textures are created/modified.

The uploadTexture line is available in every backtrace that auria pasted here (except the crash in particles), so this can be the same issue.

deveee commented 7 years ago

Btw. @Elderme it looks like you are sometimes also fixing the bugs in your branch and not only do a refactoring (for example fixed kart selection screen in legacy pipeline, which is now fixed in our master branch too). If there is something that is easily portable to our master branch, it would be great if you could make a pull requests with these particular modifications. It would allow to push these fixes to master much quicker.

Elderme commented 7 years ago

You're right, @deveee , I did not only refactoring in this branch. I should have done small branches with independant modifications and made small pull requests. I don't know if there is something that is easily portable in the branch.

I'm trying to understand the GL_INVALID_ENUM at beginning of Auria's log. There is a call to uploadTexture in the backtrace (like in the other backtraces), but I don't see calls to glActiveTexture before this point.

But when ShaderBasedRenderer has been created, instances of CommandBuffer were automatically created, with calls to
glBindBuffer(GL_DRAW_INDIRECT_BUFFER, m_draw_indirect_cmd_id); As GL_DRAW_INDIRECT_BUFFER depends on GL_ARB_draw_indirect extension (not available for Auria), maybe this can create an issue?

So now CommandBuffers are created only if GL_ARB_draw_indirect extension is available (if there is no GL_ARB_draw_indirect, geometry is rendered with a different method, without CommandBuffers).

@auriamg , could you test again and tell me if it changes something?

(hope my message is understandable, my english is so bad :( )

qwertychouskie commented 7 years ago

(hope my message is understandable, my english is so bad :( )

Not really... :)

auriamg commented 7 years ago

I can confirm that the GL_INVALID_ENUM warning is fixed!

The other problems remain, unfortunately

auriamg commented 7 years ago

After some more checks, I found that the stack trace I gave you for the invalid framebuffer is probably irrelevant :( this is the stack trace where irrlicht notices a problem, but irrlicht doens't check for OpenGL errors nowehere near often enough. I will check if I can find a better trace

EDIT: yeah I'm having trouble figuring out where the error occurs, nothing whatsoever is logged, but the actual error seems to occur way before the moment it is detected, nowhere in that stack trace at all :(

Elderme commented 7 years ago

The cause of the GL_INVALID_ENUM warning wasn't very difficult to find, because it occurred in ShaderBasedRenderer constructor, so there wasn't too much lines of code to check.

Other issues may be more difficult to fix.

I'm trying to understand the GL_INVALID_FRAMEBUFFER_OPERATION in the kart selection screen (actually you said Irrlicht notices an error in the screen right after kart selection screen, but errors are not checked everywhere, and there is an issue in kart selection screen. So I think the GL_INVALID_FRAMEBUFFER_OPERATION occurs here).

auriamg commented 7 years ago

It is very likely that the actual issue occurs on the kart selection screen, indeed, since it doesn't work correctly. It's just a shame that the intel driver doesn't seem to provide logging (AMD and nvidia drivers call the logging function as soon as issues occur). I am not quite sure how to find the exact location of the problem apart from calling glGetError all over the place :/

PS: For future contributions, might I suggest doing smaller, incremental, pull requests? ;) That would likely make it a lot easier to figure out what's wrong should any problems occur

qwertychouskie commented 7 years ago

(AMD and Nvidia drivers call the logging function as soon as issues occur)

I have a system with an Nvidia GeForce 8000 series card and Xubuntu 16.04, if you give me commands to run for testing, I can post the results here.

Benau commented 7 years ago

in your stk-code folder, do: git remote add elderme https://github.com/Elderme/stk-code.git git fetch elderme git checkout renderer_refactoring

then re-build

qwertychouskie commented 7 years ago

I don't have an stk-code folder on this computer, should I run the commands in an empty folder?

Benau commented 7 years ago

then you can do git clone https://github.com/Elderme/stk-code.git you will need stk-assets too