Code review comment for lp:~afrantzis/clutk/gles2-shaders.alf-fixes

Jammy Zhou (jammy-zhou) wrote :

Hi Alexandros,

clutk/ctk-gfx-private.c:
yes, "CHECKGL( glEnable (GL_TEXTURE_2D) );" can be removed, because
GL_TEXTURE_2D is always enabled by default in gles2.

clutk/ctk-gfx-private.h:
I don't think we need to include <GLES2/gl2.h> or <GL/gl.h> here. Any reason
for this?

configure.ac:
Good catch. :)

Regards,
Jammy

2010/9/21 Alexandros Frantzis <email address hidden>

> Alexandros Frantzis has proposed merging
> lp:~afrantzis/clutk/gles2-shaders.alf-fixes into
> lp:~jammy-zhou/clutk/gles2-shaders.hacky.
>
> Requested reviews:
> Jammy Zhou (jammy-zhou)
>
> --
>
> https://code.launchpad.net/~afrantzis/clutk/gles2-shaders.alf-fixes/+merge/36046<https://code.launchpad.net/%7Eafrantzis/clutk/gles2-shaders.alf-fixes/+merge/36046>
> You are requested to review the proposed merge of
> lp:~afrantzis/clutk/gles2-shaders.alf-fixes into
> lp:~jammy-zhou/clutk/gles2-shaders.hacky.
>
> === modified file 'clutk/ctk-gfx-private.c'
> --- clutk/ctk-gfx-private.c 2010-09-14 06:57:48 +0000
> +++ clutk/ctk-gfx-private.c 2010-09-20 18:31:13 +0000
> @@ -186,7 +186,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
> }
>
> @@ -258,7 +257,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, texid
> /*ctk_render_target_get_color_buffer_ogl_id(rt)*/) );
> }
>
> @@ -344,7 +342,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D,
> ctk_render_target_get_color_buffer_ogl_id(rt)) );
> }
>
> @@ -439,14 +436,12 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, base_texid) );
> }
>
> /* Set texture 1 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE1) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
> }
>
> @@ -525,7 +520,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D,
> ctk_render_target_get_color_buffer_ogl_id(rt_src)) );
> }
>
> @@ -654,7 +648,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
> }
>
> @@ -662,7 +655,6 @@
> {
> CHECKGL( glActiveTexture(GL_TEXTURE1) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
> }
>
> @@ -762,7 +754,6 @@
> CHECKGL( glUseProgram(g_shMultipassBlur->shprog) );
>
> CHECKGL (glActiveTexture (GL_TEXTURE0));
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL (glBindTexture (GL_TEXTURE_2D, pSrcTexture));
> CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,
> GL_NEAREST));
> CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> GL_NEAREST));
> @@ -850,7 +841,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
> }
>
> @@ -934,7 +924,6 @@
> /* Set texture 0 environment mode */
> {
> CHECKGL( glActiveTexture(GL_TEXTURE0) );
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
> }
>
> @@ -942,7 +931,6 @@
> {
> CHECKGL( glActiveTexture(GL_TEXTURE1) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
> - CHECKGL( glEnable (GL_TEXTURE_2D) );
> CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
> }
>
>
> === modified file 'clutk/ctk-gfx-private.h'
> --- clutk/ctk-gfx-private.h 2010-09-03 09:22:19 +0000
> +++ clutk/ctk-gfx-private.h 2010-09-20 18:31:13 +0000
> @@ -33,6 +33,12 @@
> #include <clutk/ctk-render-target.h>
> #include <clutk/ctk-effect-context.h>
>
> +#ifdef WITH_GLES
> +#include <GLES2/gl2.h>
> +#else
> +#include <GL/gl.h>
> +#endif
> +
> G_BEGIN_DECLS
>
> #ifdef WITH_GLES
>
> === modified file 'configure.ac'
> --- configure.ac 2010-09-03 09:22:19 +0000
> +++ configure.ac 2010-09-20 18:31:13 +0000
> @@ -101,7 +101,7 @@
> )
> AM_CONDITIONAL([WITH_GLES], [test "x$enable_gles" = "xyes"])
>
> -if ! test "x$enable_gles" = "xyes"; then
> +if test "x$enable_gles" = "xyes"; then
> PKG_CHECK_MODULES(GLXX, glesv2 egl)
> else
> PKG_CHECK_MODULES(GLXX, gl)
>
>
>

« Back to merge proposal