GNOME Bugzilla – Bug 751540
gltestsrc: implement missing patterns, port to GL3 / ES3, load shaders with GIO
Last modified: 2016-03-31 10:20:53 UTC
This set of patches implements all missing patterns except circular, ports them to modern OpenGL, so it can be used with the default GL3 context. Shaders are loaded with GIO gsresource, so shader files in gst-gl can now be stored not only inline, for better readability, syntax highlighting and maintainability.
Created attachment 306164 [details] [review] gltestsrc: make mandelbrot pattern work with opengl3
Created attachment 306165 [details] [review] gltestsrc: implement modern GL SMPTE pattern
Created attachment 306166 [details] [review] gltestsrc: Ports Checker pattern to GLSL 330 / es 300.
Created attachment 306167 [details] [review] gltestsrc: load shaders with GIO gresources
Created attachment 306168 [details] [review] gltestsrc: remove version header from shaders. add header for gl3 / gles3 based on env
I didn't look at the patches yet, but you keep support for GL2/GLES2?
No in the current implementation this support is dropped. But it can be achieved by not using vertex buffer arrays for GL2 and writing additional shaders for GL2 / ES2. I wonder how the folder stucture for the shaders should look like in that case. GL2 / GLES2 and GL3 /GLES3 shaders look pretty similar, except for the version header. Also there is currently no separation between GLES2 and GLES3 in the framework.
That doesn't sound like a good plan then. Also are we explictly requesting GLES3 contexts at all currently? I think it's time to add some templating to the shaders, and if this here is really just about the #version thing... Matthew did something related to that when he added the GL3 core profile support for OSX. Check that code :)
Right, so the shaders should be backported to GL2/GLES2 and then they will work everywhere (even GL3). (In reply to Lubosz Sarnecki from comment #7) > I wonder how the folder stucture for the shaders should look like in that > case. > GL2 / GLES2 and GL3 /GLES3 shaders look pretty similar, except for the > version header. If you can use GLES2 shaders, then they can be used for GLES3/GL3 as well as is done everywhere else. > Also there is currently no separation between GLES2 and GLES3 in the > framework. There doesn't need to be. GLES3 is a superset of GLES2. As such a simple version check is enough. In fact, GLES3 is returned by many implementation by default when you ask for a GLES2 context anyway.
Review of attachment 306164 [details] [review]: The shader should be ported to GLES2 instead. ::: ext/gl/gltestsrc.c @@ +240,3 @@ attr_uv_loc = gst_gl_shader_get_attribute_location (v->shader, "uv"); + glGenVertexArrays (1, &vertex_array); Generating VAO's every frame is not a good idea. @@ +249,3 @@ + glEnableVertexAttribArray (attr_position_loc); + glBufferData (GL_ARRAY_BUFFER, 16 * sizeof (GLfloat), positions, + GL_STATIC_DRAW); Uploading unchanging data every frame is a bad idea :) @@ +256,3 @@ + glVertexAttribPointer (attr_uv_loc, 2, GL_FLOAT, GL_FALSE, 0, 0); + glEnableVertexAttribArray (attr_uv_loc); + glBufferData (GL_ARRAY_BUFFER, 8 * sizeof (GLfloat), uvs, GL_STATIC_DRAW); Uploading unchanging data every frame is a bad idea :) @@ +262,3 @@ + glBindBuffer (GL_ELEMENT_ARRAY_BUFFER, index_buffer); + glBufferData (GL_ELEMENT_ARRAY_BUFFER, 5 * sizeof (GLuint), indices, + GL_STATIC_DRAW); Uploading unchanging data every frame is a bad idea :)
Review of attachment 306165 [details] [review]: Port shaders to GLES2 and don't upload vertex/index every frame when it's not needed. ::: ext/gl/gstgltestsrc.c @@ +743,3 @@ + if (src->display) { + gst_object_unref (src->display); + src->display = NULL; This is done already in READY -> NULL.
Review of attachment 306166 [details] [review]: Port the shader to GLES2 and don't upload unchanging index data every frame. ::: ext/gl/gltestsrc.c @@ +447,3 @@ + w, h, + w, 0, + 0, 0, Are you sure that these should be 0->width/height? 0->width/height might mean we're overdrawing outside the framebuffer dimensions. ::: ext/gl/gstgltestsrc.c @@ -320,3 @@ void main() \ { \ - vec2 xy_index= floor((gl_FragCoord.xy-vec2(0.5,0.5))/checker_width); \ To remove the gl_FragCoord one has to pass the dimensions of the framebuffer into the shader.
Review of attachment 306167 [details] [review]: Port the shaders to GLES2. ::: configure.ac @@ +301,3 @@ +dnl gio-2.0 is optional and used in gltransformation +HAVE_GIO=NO +PKG_CHECK_MODULES(GIO, gio-1.0 >= 1.0.0, HAVE_GIO=yes, HAVE_GIO=no) gio-1.0 doesn't exist (gio-2.0 does). Also need to find the version that started having GResource. ::: ext/gl/Makefile.am @@ +148,3 @@ $(LIBM) \ + $(GRAPHENE_LIBS) \ + $(GIO_LIBS) Incorrect tab usage for a makefile ::: ext/gl/gltestsrc.c @@ +240,3 @@ + const char *color_fragment = gst_gl_test_src_read_shader ("color.frag"); + const char *snow_vertex = gst_gl_test_src_read_shader ("snow.vert"); + const char *snow_fragment = gst_gl_test_src_read_shader ("snow.frag"); Shouldn't leak the shaders.
Review of attachment 306168 [details] [review]: This shouldn't be needed if the shaders are in GLES2 and is currently taken care of by GstGLShader where it matters.
commit a2d82e329a6695bef3fe7dc0bb7883e7d6666837 Author: Matthew Waters <matthew@centricular.com> Date: Fri Feb 26 20:55:47 2016 +1100 gltestsrc: port to gles2/gl3 This makes gltestsrc work everywhere \o/ - workaround RPi returning invalid values for positive coords in the checker shader - reduce the number of iterations in the mandelbrot shader for gles2 https://bugzilla.gnome.org/show_bug.cgi?id=751540 commit 00828b8c4ce3a3af85ab9b86056b44ccca2db4b5 Author: Matthew Waters <matthew@centricular.com> Date: Fri Feb 26 16:57:47 2016 +1100 gltestsrc: port smpte pattern to shaders Loosely based on patch by Lubosz Sarnecki <lubosz.sarnecki@collabora.co.uk> https://bugzilla.gnome.org/show_bug.cgi?id=751540