CluTK

Merge lp:~afrantzis/clutk/gles2-shaders.alf-fixes into lp:~jammy-zhou/clutk/gles2-shaders.hacky

  1. gles2-shaders.alf-fixes
  2. Merge into gles2-shaders.hacky
Proposed by Alexandros Frantzis
Status: Merged
Approved by: Jammy Zhou
Approved revision: 274
Merged at revision: 272
Proposed branch: lp:~afrantzis/clutk/gles2-shaders.alf-fixes
Merge into: lp:~jammy-zhou/clutk/gles2-shaders.hacky
Diff against target: 128 lines (+7/-13)
3 files modified
clutk/ctk-gfx-private.c (+0/-12)
clutk/ctk-gfx-private.h (+6/-0)
configure.ac (+1/-1)
To merge this branch: bzr merge lp:~afrantzis/clutk/gles2-shaders.alf-fixes
Reviewer Review Type Date Requested Status
Jammy Zhou Approve
Review via email: mp+36046@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jammy Zhou (jammy-zhou) wrote :
Download full text (4.8 KiB)

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))...

Read more...

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> 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?

In clutter-1.0-linaro (the packages in my PPA) the cogl/clutter headers have been made independent of GL headers (they don't include them). This was done so that we are able to build against libclutter-1.0-dev even when the application itself is using GLES (eg when creating debian packages).

This means that applications have to explicitly include the GL headers they need in order to build.
See also http://bugzilla.clutter-project.org/show_bug.cgi?id=2305.

I should have been more verbose about this in the commit message :)

Revision history for this message
Jammy Zhou (jammy-zhou) wrote :

Hi Alexandros,

When trying latest clutter-eglx package, I have already known the reason for change in ctk-gfx-private.h. So this patch is OK to me now.

Revision history for this message
Jammy Zhou (jammy-zhou) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:43 +0000
@@ -186,7 +186,6 @@
186 /* Set texture 0 environment mode */186 /* Set texture 0 environment mode */
187 {187 {
188 CHECKGL( glActiveTexture(GL_TEXTURE0) );188 CHECKGL( glActiveTexture(GL_TEXTURE0) );
189 CHECKGL( glEnable (GL_TEXTURE_2D) );
190 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );189 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
191 }190 }
192191
@@ -258,7 +257,6 @@
258 /* Set texture 0 environment mode */257 /* Set texture 0 environment mode */
259 {258 {
260 CHECKGL( glActiveTexture(GL_TEXTURE0) );259 CHECKGL( glActiveTexture(GL_TEXTURE0) );
261 CHECKGL( glEnable (GL_TEXTURE_2D) );
262 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid /*ctk_render_target_get_color_buffer_ogl_id(rt)*/) );260 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid /*ctk_render_target_get_color_buffer_ogl_id(rt)*/) );
263 }261 }
264262
@@ -344,7 +342,6 @@
344 /* Set texture 0 environment mode */342 /* Set texture 0 environment mode */
345 {343 {
346 CHECKGL( glActiveTexture(GL_TEXTURE0) );344 CHECKGL( glActiveTexture(GL_TEXTURE0) );
347 CHECKGL( glEnable (GL_TEXTURE_2D) );
348 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt)) );345 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt)) );
349 }346 }
350347
@@ -439,14 +436,12 @@
439 /* Set texture 0 environment mode */436 /* Set texture 0 environment mode */
440 {437 {
441 CHECKGL( glActiveTexture(GL_TEXTURE0) );438 CHECKGL( glActiveTexture(GL_TEXTURE0) );
442 CHECKGL( glEnable (GL_TEXTURE_2D) );
443 CHECKGL( glBindTexture(GL_TEXTURE_2D, base_texid) );439 CHECKGL( glBindTexture(GL_TEXTURE_2D, base_texid) );
444 }440 }
445441
446 /* Set texture 1 environment mode */442 /* Set texture 1 environment mode */
447 {443 {
448 CHECKGL( glActiveTexture(GL_TEXTURE1) );444 CHECKGL( glActiveTexture(GL_TEXTURE1) );
449 CHECKGL( glEnable (GL_TEXTURE_2D) );
450 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );445 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
451 }446 }
452447
@@ -525,7 +520,6 @@
525 /* Set texture 0 environment mode */520 /* Set texture 0 environment mode */
526 {521 {
527 CHECKGL( glActiveTexture(GL_TEXTURE0) );522 CHECKGL( glActiveTexture(GL_TEXTURE0) );
528 CHECKGL( glEnable (GL_TEXTURE_2D) );
529 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt_src)) );523 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt_src)) );
530 }524 }
531525
@@ -654,7 +648,6 @@
654 /* Set texture 0 environment mode */648 /* Set texture 0 environment mode */
655 {649 {
656 CHECKGL( glActiveTexture(GL_TEXTURE0) );650 CHECKGL( glActiveTexture(GL_TEXTURE0) );
657 CHECKGL( glEnable (GL_TEXTURE_2D) );
658 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );651 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
659 }652 }
660653
@@ -662,7 +655,6 @@
662 {655 {
663 CHECKGL( glActiveTexture(GL_TEXTURE1) );656 CHECKGL( glActiveTexture(GL_TEXTURE1) );
664 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )657 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
665 CHECKGL( glEnable (GL_TEXTURE_2D) );
666 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );658 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
667 }659 }
668660
@@ -762,7 +754,6 @@
762 CHECKGL( glUseProgram(g_shMultipassBlur->shprog) );754 CHECKGL( glUseProgram(g_shMultipassBlur->shprog) );
763755
764 CHECKGL (glActiveTexture (GL_TEXTURE0));756 CHECKGL (glActiveTexture (GL_TEXTURE0));
765 CHECKGL( glEnable (GL_TEXTURE_2D) );
766 CHECKGL (glBindTexture (GL_TEXTURE_2D, pSrcTexture));757 CHECKGL (glBindTexture (GL_TEXTURE_2D, pSrcTexture));
767 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST));758 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST));
768 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST));759 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST));
@@ -850,7 +841,6 @@
850 /* Set texture 0 environment mode */841 /* Set texture 0 environment mode */
851 {842 {
852 CHECKGL( glActiveTexture(GL_TEXTURE0) );843 CHECKGL( glActiveTexture(GL_TEXTURE0) );
853 CHECKGL( glEnable (GL_TEXTURE_2D) );
854 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );844 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
855 }845 }
856846
@@ -934,7 +924,6 @@
934 /* Set texture 0 environment mode */924 /* Set texture 0 environment mode */
935 {925 {
936 CHECKGL( glActiveTexture(GL_TEXTURE0) );926 CHECKGL( glActiveTexture(GL_TEXTURE0) );
937 CHECKGL( glEnable (GL_TEXTURE_2D) );
938 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );927 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
939 }928 }
940929
@@ -942,7 +931,6 @@
942 {931 {
943 CHECKGL( glActiveTexture(GL_TEXTURE1) );932 CHECKGL( glActiveTexture(GL_TEXTURE1) );
944 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )933 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
945 CHECKGL( glEnable (GL_TEXTURE_2D) );
946 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );934 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
947 }935 }
948936
949937
=== 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:43 +0000
@@ -33,6 +33,12 @@
33#include <clutk/ctk-render-target.h>33#include <clutk/ctk-render-target.h>
34#include <clutk/ctk-effect-context.h>34#include <clutk/ctk-effect-context.h>
3535
36#ifdef WITH_GLES
37#include <GLES2/gl2.h>
38#else
39#include <GL/gl.h>
40#endif
41
36G_BEGIN_DECLS42G_BEGIN_DECLS
3743
38#ifdef WITH_GLES44#ifdef WITH_GLES
3945
=== modified file 'configure.ac'
--- configure.ac 2010-09-03 09:22:19 +0000
+++ configure.ac 2010-09-20 18:31:43 +0000
@@ -101,7 +101,7 @@
101)101)
102AM_CONDITIONAL([WITH_GLES], [test "x$enable_gles" = "xyes"])102AM_CONDITIONAL([WITH_GLES], [test "x$enable_gles" = "xyes"])
103103
104if ! test "x$enable_gles" = "xyes"; then104if test "x$enable_gles" = "xyes"; then
105 PKG_CHECK_MODULES(GLXX, glesv2 egl)105 PKG_CHECK_MODULES(GLXX, glesv2 egl)
106else106else
107 PKG_CHECK_MODULES(GLXX, gl)107 PKG_CHECK_MODULES(GLXX, gl)

Subscribers

People subscribed via source and target branches