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
1=== modified file 'clutk/ctk-gfx-private.c'
2--- clutk/ctk-gfx-private.c 2010-09-14 06:57:48 +0000
3+++ clutk/ctk-gfx-private.c 2010-09-20 18:31:43 +0000
4@@ -186,7 +186,6 @@
5 /* Set texture 0 environment mode */
6 {
7 CHECKGL( glActiveTexture(GL_TEXTURE0) );
8- CHECKGL( glEnable (GL_TEXTURE_2D) );
9 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
10 }
11
12@@ -258,7 +257,6 @@
13 /* Set texture 0 environment mode */
14 {
15 CHECKGL( glActiveTexture(GL_TEXTURE0) );
16- CHECKGL( glEnable (GL_TEXTURE_2D) );
17 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid /*ctk_render_target_get_color_buffer_ogl_id(rt)*/) );
18 }
19
20@@ -344,7 +342,6 @@
21 /* Set texture 0 environment mode */
22 {
23 CHECKGL( glActiveTexture(GL_TEXTURE0) );
24- CHECKGL( glEnable (GL_TEXTURE_2D) );
25 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt)) );
26 }
27
28@@ -439,14 +436,12 @@
29 /* Set texture 0 environment mode */
30 {
31 CHECKGL( glActiveTexture(GL_TEXTURE0) );
32- CHECKGL( glEnable (GL_TEXTURE_2D) );
33 CHECKGL( glBindTexture(GL_TEXTURE_2D, base_texid) );
34 }
35
36 /* Set texture 1 environment mode */
37 {
38 CHECKGL( glActiveTexture(GL_TEXTURE1) );
39- CHECKGL( glEnable (GL_TEXTURE_2D) );
40 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
41 }
42
43@@ -525,7 +520,6 @@
44 /* Set texture 0 environment mode */
45 {
46 CHECKGL( glActiveTexture(GL_TEXTURE0) );
47- CHECKGL( glEnable (GL_TEXTURE_2D) );
48 CHECKGL( glBindTexture(GL_TEXTURE_2D, ctk_render_target_get_color_buffer_ogl_id(rt_src)) );
49 }
50
51@@ -654,7 +648,6 @@
52 /* Set texture 0 environment mode */
53 {
54 CHECKGL( glActiveTexture(GL_TEXTURE0) );
55- CHECKGL( glEnable (GL_TEXTURE_2D) );
56 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
57 }
58
59@@ -662,7 +655,6 @@
60 {
61 CHECKGL( glActiveTexture(GL_TEXTURE1) );
62 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
63- CHECKGL( glEnable (GL_TEXTURE_2D) );
64 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
65 }
66
67@@ -762,7 +754,6 @@
68 CHECKGL( glUseProgram(g_shMultipassBlur->shprog) );
69
70 CHECKGL (glActiveTexture (GL_TEXTURE0));
71- CHECKGL( glEnable (GL_TEXTURE_2D) );
72 CHECKGL (glBindTexture (GL_TEXTURE_2D, pSrcTexture));
73 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST));
74 CHECKGL (glTexParameteri (GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST));
75@@ -850,7 +841,6 @@
76 /* Set texture 0 environment mode */
77 {
78 CHECKGL( glActiveTexture(GL_TEXTURE0) );
79- CHECKGL( glEnable (GL_TEXTURE_2D) );
80 CHECKGL( glBindTexture(GL_TEXTURE_2D, texid) );
81 }
82
83@@ -934,7 +924,6 @@
84 /* Set texture 0 environment mode */
85 {
86 CHECKGL( glActiveTexture(GL_TEXTURE0) );
87- CHECKGL( glEnable (GL_TEXTURE_2D) );
88 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_id) );
89 }
90
91@@ -942,7 +931,6 @@
92 {
93 CHECKGL( glActiveTexture(GL_TEXTURE1) );
94 CHECKGL( glBindTexture(GL_TEXTURE_2D, 0) )
95- CHECKGL( glEnable (GL_TEXTURE_2D) );
96 CHECKGL( glBindTexture(GL_TEXTURE_2D, tex_mask_id) );
97 }
98
99
100=== modified file 'clutk/ctk-gfx-private.h'
101--- clutk/ctk-gfx-private.h 2010-09-03 09:22:19 +0000
102+++ clutk/ctk-gfx-private.h 2010-09-20 18:31:43 +0000
103@@ -33,6 +33,12 @@
104 #include <clutk/ctk-render-target.h>
105 #include <clutk/ctk-effect-context.h>
106
107+#ifdef WITH_GLES
108+#include <GLES2/gl2.h>
109+#else
110+#include <GL/gl.h>
111+#endif
112+
113 G_BEGIN_DECLS
114
115 #ifdef WITH_GLES
116
117=== modified file 'configure.ac'
118--- configure.ac 2010-09-03 09:22:19 +0000
119+++ configure.ac 2010-09-20 18:31:43 +0000
120@@ -101,7 +101,7 @@
121 )
122 AM_CONDITIONAL([WITH_GLES], [test "x$enable_gles" = "xyes"])
123
124-if ! test "x$enable_gles" = "xyes"; then
125+if test "x$enable_gles" = "xyes"; then
126 PKG_CHECK_MODULES(GLXX, glesv2 egl)
127 else
128 PKG_CHECK_MODULES(GLXX, gl)

Subscribers

People subscribed via source and target branches