Good news, everyone!
While I've still not been able to figure out the issue with the libretro GL support, it seems that at least we've been able to find out causes for most of the remaining bugs in the OpenGL shaders backend during our little local hackathon. I'm now doing a rebase and a clean up of the code. Hopefully I will have the patches ready tomorrow.
Here is the list of solved bugs:
#1 - Active shader thumbnail overdraws the video filter GUI window (affects both GL and GLES)
This is caused by the order of calls to
videoShader->PrepareParameters() in
CShaderPresetGL :: PrepareParameters() (
https://github.com/garbear/xbmc/blob/455...L.cpp#L411).
We need to set the parameters for each shader pass after we update the
m_dest and the viewport, not before. It can be solved by moving the loop with
videoShader->PrepareParameters() below the viewport update, but later we have discovered, that the
videoShader->PrepareParameters() needs to be moved elsewhere to fix the bug #3, so this fix gets overwritten by a new fix, which solves both #1 and #3.
#2 - The shader overdraws OSD GUI textures (affects GLES only)
This one is caused by not properly unbinding the GL_ARRAY_BUFFER after rendering the shader. The
CGUITextureGLES::End() (unlike the
CGUITextureGL::End()) is not using
glBindBuffer() to rebind the array before using it, so it ends up rendering the GUI texture to a wrong buffer. This can be solved by either avoiding the use of
glBindBuffer() in the GLES code path completely, or by correctly unbinding the GL_ARRAY_BUFFER before the GUI is rendered. The first method brings more GLES #ifdefs, so we have choosen the second method and have added the unbind of the array to
CShaderGL :: Render() after the
glDrawElements() (
https://github.com/garbear/xbmc/blob/455...L.cpp#L120).
#3 - Multipass shaders doesn't work in GLES
This issue is caused by calling the
videoShader->PrepareParameters() from the loop (
https://github.com/garbear/xbmc/blob/455...L.cpp#L413). Only parameters of the last shader pass are preserved, the parameters of the other passes are being overwritten before they are rendered. The use of VAO saves the day for the GL, but it doesn't work for GLES (
https://github.com/garbear/xbmc/blob/455...L.cpp#L129). We tried to solve this by removing the
videoShader->PrepareParameters() from the loop in
CShaderPresetGL :: PrepareParameters() and instead calling it directly from the
CShaderPresetGL :: RenderUpdate() before each pass is rendered. After this change we get much more shaders working in GLES (most shaders affected by this bug were rendered in a wrong scale and upside down in GLES before this change).
#4 - Broken LUT support (lookup textures) (affects both GL and GLES)
First we have found out that the textures for LUTs are currently not being uploaded to GPU at all (
https://github.com/garbear/xbmc/blob/455...GL.cpp#L60). After fixing the texture loading we needed to uncomment the code in
CShaderGL :: Render() (
https://github.com/garbear/xbmc/blob/455...L.cpp#L106) and set the texture name by
glUniform1f(). After these fixes we get even more shaders working in both GL and GLES (e.g. those with images of the handheld consoles around the screen). Missing LUTs were also a point of failure for many multipass shader presets.
What needs to be done next
Now we've got all of the GUI bugs solved and most of the shaders working, but the UI is limited. While playing with the shaders in RetroArch, I've found out why there's so many of them. The use of shaders is very subjective and it depends on the used emulator/system and even on the used hardware (performance issues and driver compatibility), so it is impossible to create a reasonable fixed list of shaders to suite everyone. We need a way to browse the shader folder and to add any shader to the list.
Here is my proposal for the UI changes:
- Move the
ShaderPresetsXXXX.xml to
userdata/addon_data/game.shader.presers/, so it is user editable.
- Let's reuse what we can from the in-game savestates UI.
- Keep the Nearest and Bilinear filters fixed in the first 2 slots.
- Make the other filters removable from the list by a context menu (just like we did for savestates).
- Create a "+" button in the end of the list which will open the standard browse file dialog and allow adding new filters (we can reuse the "+" button from savestates).
- Get rid of the "Category" and "Name" translated strings. It is impossible to get strings for every shader preset out there. Instead use the preset filename without suffix.
- Optionally add a context menu item to move the current filter left or right in the list, so users can change order of the filters.
This way we can still ship the default xml with the most common presets, but allow every user to test and use any preset (even custom downloaded) and to make their own list of preffered shaders in the "Video filter" settings. I'd say this UI is going to be perfect for shader testing and comparision, as you can easily place 2 shaders next to each other and switch between them in real time.
@
garbear Can you do this?
With the UI update like this, we can call it the Shader support 1.0 and finally get it merged upstream, as it doesn't break anything anymore and it should be fully usable!
