[cairo] [PATCH 3/3] gl: add support for OpenGL ES 3.0
Uli Schlachter
psychon at znc.in
Sat Dec 10 15:22:05 UTC 2016
Again, I don't know OpenGL, so feel free to ignore anything stupid that
I say.
On 08.12.2016 00:46, Bryce Harrington wrote:
> @@ -87,7 +89,7 @@ _cairo_boilerplate_egl_create_surface (const char *name,
> EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
> #if CAIRO_HAS_GL_SURFACE
> EGL_RENDERABLE_TYPE, EGL_OPENGL_BIT,
> -#elif CAIRO_HAS_GLESV2_SURFACE
> +#elif CAIRO_HAS_GLESV2_SURFACE || CAIRO_HAS_GLESV3_SURFACE
> EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
> #endif
> EGL_NONE
Why ES2_BIT instead of ES3_BIT?
[...]
> diff --git a/cairo-glesv3-uninstall.pc b/cairo-glesv3-uninstall.pc
> new file mode 100644
> index 0000000..00ea39a
> --- /dev/null
> +++ b/cairo-glesv3-uninstall.pc
> @@ -0,0 +1,7 @@
> +Name: cairo-glesv3
> +Description: OpenGLESv3 surface backend for cairo graphics library
> +Version: 1.12.14
> +
> +Requires: cairo glesv3
> +Libs:
> +Cflags: -I${pc_top_builddir}/${pcfiledir}/./src
What's up with this file? There are no other such files in git (but
build/configure.ac.features generates such files at configure-time) and
the file name is wrong (it would need to be "uninstalled" instead of
"uninstall" so that pkg-config picks it up).
Giving the above, I'd say this file can just be dropped.
> diff --git a/configure.ac b/configure.ac
> index 93953a7..ea71ad6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -386,6 +386,24 @@ CAIRO_ENABLE_SURFACE_BACKEND(glesv2, OpenGLESv2, no, [
> ])
>
> dnl ===========================================================================
> +CAIRO_ENABLE_SURFACE_BACKEND(glesv3, OpenGLESv3, no, [
> + glesv3_REQUIRES="glesv2"
> + PKG_CHECK_MODULES(glesv3, $glesv3_REQUIRES,, [
> + dnl Fallback to searching for headers
> + AC_CHECK_HEADER(GLES3/gl3.h,, [use_glesv3="no (glesv2.pc nor OpenGL ES 3.0 headers not found)"])
> + if test "x$use_glesv3" = "xyes"; then
> + glesv3_NONPKGCONFIG_CFLAGS=
> + glesv3_NONPKGCONFIG_LIBS="-lGLESv2"
> + fi])
Really? The pkg-config file for glesv3 is calles glesv2 and the library
is called libGLESv2? Shouldn't there be more "3"s in there?
[...]
> diff --git a/src/cairo-gl-composite.c b/src/cairo-gl-composite.c
> index a95712e..76bc1ef 100644
> --- a/src/cairo-gl-composite.c
> +++ b/src/cairo-gl-composite.c
> @@ -52,6 +52,91 @@
[...]
> +static cairo_int_status_t
> +_blit_texture_to_renderbuffer (cairo_gl_surface_t *surface)
> +{
> + cairo_gl_context_t *ctx = NULL;
> + cairo_gl_composite_t setup;
> + cairo_surface_pattern_t pattern;
> + cairo_rectangle_int_t extents;
> + cairo_int_status_t status;
> +
> + /* FIXME: we need to take care of certain glesv2 extension too */
That FIXME is quite vague to me. Could something like "for example FOO"
be added, or anything to make this more precise?
[...]
> @@ -624,14 +656,19 @@ bind_multisample_framebuffer (cairo_gl_context_t *ctx,
> cairo_bool_t scissor_test_enabled;
>
> assert (surface->supports_msaa);
> - assert (ctx->gl_flavor == CAIRO_GL_FLAVOR_DESKTOP);
> + assert (ctx->gl_flavor == CAIRO_GL_FLAVOR_DESKTOP ||
> + ctx->gl_flavor == CAIRO_GL_FLAVOR_ES3);
>
> _cairo_gl_ensure_framebuffer (ctx, surface);
> _cairo_gl_ensure_multisampling (ctx, surface);
>
> if (surface->msaa_active) {
> +#if CAIRO_HAS_GL_SURFACE
> glEnable (GL_MULTISAMPLE);
> +#endif
This seems fishy to me. Shouldn't there be something like ctx->gl_flavor
== CAIRO_GL_FLAVOR_DESKTOP around this call to glEnable()? Otherwise it
will be done with GLESv3 if and only if the "desktop gl" stuff was enabled.
> ctx->dispatch.BindFramebuffer (GL_FRAMEBUFFER, surface->msaa_fb);
> + if (ctx->gl_flavor == CAIRO_GL_FLAVOR_ES3)
> + surface->content_in_texture = FALSE;
> return;
> }
>
> @@ -642,7 +679,9 @@ bind_multisample_framebuffer (cairo_gl_context_t *ctx,
> glDisable (GL_STENCIL_TEST);
> glDisable (GL_SCISSOR_TEST);
>
> +#if CAIRO_HAS_GL_SURFACE
> glEnable (GL_MULTISAMPLE);
> +#endif
Same as above.
[...]
> @@ -669,11 +710,15 @@ bind_singlesample_framebuffer (cairo_gl_context_t *ctx,
> cairo_bool_t stencil_test_enabled;
> cairo_bool_t scissor_test_enabled;
>
> - assert (ctx->gl_flavor == CAIRO_GL_FLAVOR_DESKTOP);
> + assert (ctx->gl_flavor == CAIRO_GL_FLAVOR_DESKTOP ||
> + ctx->gl_flavor == CAIRO_GL_FLAVOR_ES3);
> _cairo_gl_ensure_framebuffer (ctx, surface);
>
> if (! surface->msaa_active) {
> +#if CAIRO_HAS_GL_SURFACE
> glDisable (GL_MULTISAMPLE);
> +#endif
> +
Same.
> ctx->dispatch.BindFramebuffer (GL_FRAMEBUFFER, surface->fb);
> return;
> }
> @@ -685,7 +730,9 @@ bind_singlesample_framebuffer (cairo_gl_context_t *ctx,
> glDisable (GL_STENCIL_TEST);
> glDisable (GL_SCISSOR_TEST);
>
> +#if CAIRO_HAS_GL_SURFACE
> glDisable (GL_MULTISAMPLE);
> +#endif
Same.
[...]
> diff --git a/src/cairo-gl-info.c b/src/cairo-gl-info.c
> index 39541aa..29b8768 100644
> --- a/src/cairo-gl-info.c
> +++ b/src/cairo-gl-info.c
> @@ -67,6 +67,14 @@ _cairo_gl_get_flavor (void)
> flavor = CAIRO_GL_FLAVOR_NONE;
> else if (strstr (version, "OpenGL ES 2") != NULL)
> flavor = CAIRO_GL_FLAVOR_ES2;
> + else if (strstr (version, "OpenGL ES 3") != NULL)
> +#if CAIRO_HAS_GLESV3_SURFACE
> + flavor = CAIRO_GL_FLAVOR_ES3;
> +#elif CAIRO_HAS_GLESV2_SURFACE
> + flavor = CAIRO_GL_FLAVOR_ES2;
> +#else
> + flavor = CAIRO_GL_FLAVOR_NONE;
> +#endif
> else
> flavor = CAIRO_GL_FLAVOR_DESKTOP;
This patch is about adding support for OpenGL ES 3. But the above
changes the behaviour for GLES 2 if the new GLES 3 support is disabled.
Is that intended?
>
> diff --git a/src/cairo-gl-msaa-compositor.c b/src/cairo-gl-msaa-compositor.c
> index 507459d..fcd4144 100644
> --- a/src/cairo-gl-msaa-compositor.c
> +++ b/src/cairo-gl-msaa-compositor.c
> @@ -280,8 +280,10 @@ can_use_msaa_compositor (cairo_gl_surface_t *surface,
> /* Multisampling OpenGL ES surfaces only maintain one multisampling
> framebuffer and thus must use the spans compositor to do non-antialiased
> rendering. */
> - if (((cairo_gl_context_t *) surface->base.device)->gl_flavor == CAIRO_GL_FLAVOR_ES2
> + if ((((cairo_gl_context_t *) surface->base.device)->gl_flavor == CAIRO_GL_FLAVOR_ES2 ||
> + ((cairo_gl_context_t *) surface->base.device)->gl_flavor == CAIRO_GL_FLAVOR_ES3)
> && surface->supports_msaa
> + && surface->num_samples > 1
> && antialias == CAIRO_ANTIALIAS_NONE)
> return FALSE;
The above also changes things even if ES3 is disabled, doesn't it?
[...]
> @@ -1374,10 +1408,14 @@ _cairo_gl_surface_resolve_multisampling (cairo_gl_surface_t *surface)
> if (unlikely (status))
> return status;
>
> - ctx->current_target = surface;
> + _cairo_gl_composite_flush (ctx);
> +
> + ctx->current_target = NULL;
[...]
Things like this look weird to me since the code will behave differently
even if GLES3 is not enabled. This is the only such place I mention,
because I do not know the code to say more about it...
Hopefully something from this was useful.
Uli
--
Happiness can't be found -- it finds you.
- Majic
More information about the cairo
mailing list