We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3355

Summary: false "Invalid renderer" after creating an "opengles2" renderer.
Product: SDL Reporter: Sylvain <sylvain.becker>
Component: renderAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: philipp.wiesemann
Version: 2.0.4   
Hardware: All   
OS: All   

Description Sylvain 2016-06-06 06:58:45 UTC 
After creating successfully an opengles2 renderer, 
"SDL_GetError" reports anyway the error "Invalid renderer".

Because of the following call stack: 

SDL_CreateRenderer()
GLES2_CreateRenderer()
GLES2_ResetState()
GLES2_ActivateRenderer()
GLES2_UpdateViewport()
SDL_GetRendererOutputSize(renderer) 
-> it calls CHECK_MAGIC whereas the renderer is not fully created.
Comment 1 Sylvain 2016-06-06 07:05:34 UTC 
Although this is a false error, there might be something:

in GLES2_UpdateViewport() :

 389         int w, h;
 390 
 391         SDL_GetRendererOutputSize(renderer, &w, &h);
 392         data->glViewport(renderer->viewport.x, (h - renderer->viewport.y - renderer->viewport.h),
 393                          renderer->viewport.w, renderer->viewport.h);

Since "SDL_GetRendererOutputSize" fails, "h" wont be initialized.
Comment 2 Sylvain 2016-06-06 07:11:29 UTC 
Indeed, "w" and "h" variable are not initialized.
Valgrind: "Conditional jump or move depends on uninitialised value(s)"
Comment 3 Sylvain 2016-06-06 08:05:12 UTC 
opengles1 renderer has the same issue.
Comment 4 Sylvain 2016-06-06 08:23:18 UTC 
opengl has not this issue.


What seems different is the "ResetState" function, called at the end of CreateRenderer:


opengl, will directly follow the "UpdateViewport" path:

      if (SDL_GL_GetCurrentContext() == data->context) {
          GL_UpdateViewport(renderer);
      } else {
      SDL_Log("GL_ActivateRenderer..");
          GL_ActivateRenderer(renderer);
      }

opengles and opengles2, will follow the "ActivateRenderer" path:

     if (SDL_CurrentContext == data->context) {
         GLES2_UpdateViewport(renderer);
     } else {
         GLES2_ActivateRenderer(renderer);
     }
Comment 5 Philipp Wiesemann 2016-06-06 20:52:15 UTC 
See also bug 3106.
Comment 6 Sylvain 2016-06-09 08:04:03 UTC 
Bug 3106 has been marked out "not a bug" because no investigation has been done at that time.

Now that we have more accurate information, there is definitively something to do:

renderer opengles, opengles2: fix the access to un-initialized value.
This is bad, and especially because we don't know how drivers can react to this. It looks like to work on all device, but it may break some very few others.

renderer opengl: review the two lines of code that are different from opengles/opengles2. And if needed, re-impact some modification on opengles/opengles2 !
Comment 7 Sylvain 2016-06-09 08:05:07 UTC 
Here's the valgrind output when using opengles renderer:


==15869== Conditional jump or move depends on uninitialised value(s)
==15869==    at 0xD2B7A23: ??? (in /usr/lib/nvidia-361/libnvidia-glcore.so.361.42)
==15869==    by 0x4EEA4EF: GLES_UpdateViewport (SDL_render_gles.c:690)
==15869==    by 0x4EEC791: GLES_ActivateRenderer (SDL_render_gles.c:249)
==15869==    by 0x4EEC6A8: GLES_ResetState (SDL_render_gles.c:263)
==15869==    by 0x4EE96E9: GLES_CreateRenderer (SDL_render_gles.c:408)
==15869==    by 0x4ED4F97: SDL_CreateRenderer_REAL (SDL_render.c:254)
==15869==    by 0x4EB1D4F: SDL_CreateRenderer (SDL_dynapi_procs.h:337)
==15869==    by 0x402F19: main (main.cpp:29)
==15869==  Uninitialised value was created by a stack allocation
==15869==    at 0x4EEA400: GLES_UpdateViewport (SDL_render_gles.c:675)
Comment 8 Sylvain 2016-07-01 18:16:09 UTC 
Opengles2 renderer can be forced on linux desktop by calling: SDL_SetHint(SDL_HINT_RENDER_DRIVER, "opengles2");

Valgrind log for opengles2 renderer is:

==20889== Conditional jump or move depends on uninitialised value(s)
==20889==    at 0xD2B8A23: ??? (in /usr/lib/nvidia-361/libnvidia-glcore.so.361.42)
==20889==    by 0x4EF2015: GLES2_UpdateViewport (SDL_render_gles2.c:392)
==20889==    by 0x4EF4576: GLES2_ActivateRenderer (SDL_render_gles2.c:342)
==20889==    by 0x4EF420E: GLES2_ResetState (SDL_render_gles2.c:1926)
==20889==    by 0x4EED448: GLES2_CreateRenderer (SDL_render_gles2.c:2104)
==20889==    by 0x4ED4FE7: SDL_CreateRenderer_REAL (SDL_render.c:254)
==20889==    by 0x4EB1D9F: SDL_CreateRenderer (SDL_dynapi_procs.h:337)
==20889==    by 0x402FA9: main (main.cpp:31)
==20889==  Uninitialised value was created by a stack allocation
==20889==    at 0x4EF1F20: GLES2_UpdateViewport (SDL_render_gles2.c:377)
Comment 9 Sam Lantinga 2016-10-13 15:47:25 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/563503b6b4d1