[cogl] Ensure well defined semantics for COGL_INVALID_HANDLE material layers

Fixes and adds a unit test for creating and drawing using materials with
COGL_INVALID_HANDLE texture layers.

This may be valid if for example the user has set a texture combine string
that only references a constant color.

_cogl_material_flush_layers_gl_state will bind the fallback texture for any
COGL_INVALID_HANDLE layer, later though we could explicitly check when the
current blend mode does't actually reference a texture source in which case
binding the fallback texture is redundant.

This tests drawing using cogl_rectangle, cogl_polygon and
cogl_vertex_buffer_draw.

clutter/cogl/common/cogl-material.c

@@ -1336,7 +1336,13 @@ _cogl_material_flush_layers_gl_state (CoglMaterial *material,
         (disable_mask & (1<<i)) ? TRUE : FALSE;
 
       tex_handle = layer->texture;
+      if (tex_handle != COGL_INVALID_HANDLE)
         cogl_texture_get_gl_texture (tex_handle, &gl_texture, &gl_target);
+      else
+        {
+          new_gl_layer_info.fallback = TRUE;
+          gl_target = GL_TEXTURE_2D;
+        }
 
       if (new_gl_layer_info.layer0_overridden)
         gl_texture = layer0_override_texture;

clutter/cogl/common/cogl-primitives.c

@@ -942,7 +942,6 @@ _cogl_multitexture_unsliced_quad (float        x_1,
   for (tmp = (GList *)layers, i = 0; tmp != NULL; tmp = tmp->next, i++)
     {
       CoglHandle         layer = (CoglHandle)tmp->data;
-      /* CoglLayerInfo   *layer_info; */
       CoglHandle         tex_handle;
       CoglTexture       *tex;
       const float       *in_tex_coords;
@@ -950,12 +949,13 @@ _cogl_multitexture_unsliced_quad (float        x_1,
       CoglTexSliceSpan  *x_span;
       CoglTexSliceSpan  *y_span;
 
-      /* layer_info = &layers[i]; */
-
-      /* FIXME - we shouldn't be checking this stuff if layer_info->gl_texture
-       * already == 0 */
-
       tex_handle = cogl_material_layer_get_texture (layer);
+
+      /* COGL_INVALID_HANDLE textures are handled by
+       * _cogl_material_flush_gl_state */
+      if (tex_handle == COGL_INVALID_HANDLE)
+        continue;
+
       tex = _cogl_texture_pointer_from_handle (tex_handle);
 
       in_tex_coords = &user_tex_coords[i * 4];
@@ -1131,14 +1131,23 @@ _cogl_rectangles_with_multitexture_coords (
   for (tmp = layers, i = 0; tmp != NULL; tmp = tmp->next, i++)
     {
       CoglHandle     layer = tmp->data;
-      CoglHandle     tex_handle = cogl_material_layer_get_texture (layer);
+      CoglHandle     tex_handle;
-      CoglTexture   *texture = _cogl_texture_pointer_from_handle (tex_handle);
+      CoglTexture   *texture = NULL;
       gulong         flags;
 
       if (cogl_material_layer_get_type (layer)
 	  != COGL_MATERIAL_LAYER_TYPE_TEXTURE)
 	continue;
 
+      tex_handle = cogl_material_layer_get_texture (layer);
+
+      /* COGL_INVALID_HANDLE textures are handled by
+       * _cogl_material_flush_gl_state */
+      if (tex_handle == COGL_INVALID_HANDLE)
+        continue;
+
+      texture = _cogl_texture_pointer_from_handle (tex_handle);
+
       /* XXX:
        * For now, if the first layer is sliced then all other layers are
        * ignored since we currently don't support multi-texturing with
@@ -1503,6 +1512,13 @@ _cogl_multitexture_unsliced_polygon (CoglTextureVertex *vertices,
           float        tx, ty;
 
           tex_handle = cogl_material_layer_get_texture (layer);
+
+          /* COGL_INVALID_HANDLE textures will be handled in
+           * _cogl_material_flush_layers_gl_state but there is no need to worry
+           * about scaling texture coordinates in this case */
+          if (tex_handle == COGL_INVALID_HANDLE)
+            continue;
+
           tex = _cogl_texture_pointer_from_handle (tex_handle);
 
           y_span = &g_array_index (tex->slice_y_spans, CoglTexSliceSpan, 0);
@@ -1585,6 +1601,11 @@ cogl_polygon (CoglTextureVertex *vertices,
       CoglHandle   layer = (CoglHandle)tmp->data;
       CoglHandle   tex_handle = cogl_material_layer_get_texture (layer);
 
+      /* COGL_INVALID_HANDLE textures will be handled in
+       * _cogl_material_flush_layers_gl_state */
+      if (tex_handle == COGL_INVALID_HANDLE)
+        continue;
+
       if (i == 0 && cogl_texture_is_sliced (tex_handle))
         {
 #if defined (HAVE_COGL_GLES) || defined (HAVE_COGL_GLES2)

clutter/cogl/common/cogl-vertex-buffer.c

@@ -1618,8 +1618,14 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer)
     {
       CoglHandle layer = (CoglHandle)tmp->data;
       CoglHandle tex_handle = cogl_material_layer_get_texture (layer);
-      CoglTexture *texture =
+      CoglTexture *texture;
-        _cogl_texture_pointer_from_handle (tex_handle);
+
+      /* invalid textures will be handled correctly in
+       * _cogl_material_flush_layers_gl_state */
+      if (tex_handle == COGL_INVALID_HANDLE)
+        continue;
+
+      texture = _cogl_texture_pointer_from_handle (tex_handle);
 
       if (cogl_texture_is_sliced (tex_handle)
           || _cogl_texture_span_has_waste (texture, 0, 0))

tests/conform/Makefile.am

@@ -30,6 +30,7 @@ test_conformance_SOURCES = 		\
 	test-color.c			\
 	test-clutter-units.c		\
 	test-premult.c			\
+	test-materials.c		\
         $(NULL)
 
 # For convenience, this provides a way to easily run individual unit tests:

tests/conform/test-conform-main.c

@@ -149,6 +149,7 @@ main (int argc, char **argv)
   TEST_CONFORM_SIMPLE ("/model", test_list_model_iterate);
   TEST_CONFORM_SIMPLE ("/model", test_list_model_filter);
 
+  TEST_CONFORM_SIMPLE ("/material", test_materials);
   TEST_CONFORM_SIMPLE ("/material", test_blend_strings);
 
   TEST_CONFORM_SIMPLE ("/color", test_color_from_string);

tests/conform/test-materials.c

@@ -0,0 +1,195 @@
+
+#include <clutter/clutter.h>
+#include <cogl/cogl.h>
+#include <string.h>
+
+#include "test-conform-common.h"
+
+static const ClutterColor stage_color = { 0x0, 0x0, 0x0, 0xff };
+
+#define QUAD_WIDTH 20
+
+#define RED 0
+#define GREEN 1
+#define BLUE 2
+#define ALPHA 3
+
+#define MASK_RED(COLOR)   ((COLOR & 0xff000000) >> 24);
+#define MASK_GREEN(COLOR) ((COLOR & 0xff0000) >> 16);
+#define MASK_BLUE(COLOR)  ((COLOR & 0xff00) >> 8);
+#define MASK_ALPHA(COLOR) (COLOR & 0xff);
+
+#define SKIP_FRAMES 2
+
+typedef struct _TestState
+{
+  guint frame;
+  ClutterGeometry stage_geom;
+} TestState;
+
+
+static void
+check_pixel (TestState *state, int x, int y, guint32 color)
+{
+  GLint y_off;
+  GLint x_off;
+  GLubyte pixel[4];
+  guint8 r = MASK_RED (color);
+  guint8 g = MASK_GREEN (color);
+  guint8 b = MASK_BLUE (color);
+  guint8 a = MASK_ALPHA (color);
+
+  /* See what we got... */
+
+  /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */
+  y_off = y * QUAD_WIDTH + (QUAD_WIDTH / 2);
+  x_off = x * QUAD_WIDTH + (QUAD_WIDTH / 2);
+
+  /* XXX:
+   * We haven't always had good luck with GL drivers implementing glReadPixels
+   * reliably and skipping the first two frames improves our chances... */
+  if (state->frame <= SKIP_FRAMES)
+    return;
+
+  cogl_read_pixels (x_off, y_off, 1, 1,
+                    COGL_READ_PIXELS_COLOR_BUFFER,
+                    COGL_PIXEL_FORMAT_RGBA_8888,
+                    pixel);
+  if (g_test_verbose ())
+    g_print ("  result = %02x, %02x, %02x, %02x\n",
+             pixel[RED], pixel[GREEN], pixel[BLUE], pixel[ALPHA]);
+
+  if (g_test_verbose ())
+    g_print ("  expected = %x, %x, %x, %x\n",
+             r, g, b, a);
+  /* FIXME - allow for hardware in-precision */
+  g_assert (pixel[RED] == r);
+  g_assert (pixel[GREEN] == g);
+  g_assert (pixel[BLUE] == b);
+
+  /* FIXME
+   * We ignore the alpha, since we don't know if our render target is
+   * RGB or RGBA */
+  /* g_assert (pixel[ALPHA] == a); */
+}
+
+static void
+test_invalid_texture_layers (TestState *state, int x, int y)
+{
+  CoglHandle        material = cogl_material_new ();
+  CoglTextureVertex verts[4] = {
+    { .x = 0,          .y = 0,          .z = 0 },
+    { .x = 0,          .y = QUAD_WIDTH, .z = 0 },
+    { .x = QUAD_WIDTH, .y = QUAD_WIDTH, .z = 0 },
+    { .x = QUAD_WIDTH, .y = 0,          .z = 0 },
+  };
+  CoglHandle vbo;
+
+  cogl_push_matrix ();
+
+  cogl_translate (x * QUAD_WIDTH, y * QUAD_WIDTH, 0);
+
+  /* explicitly create a layer with an invalid handle. This may be desireable
+   * if the user also sets a texture combine string that e.g. refers to a
+   * constant color. */
+  cogl_material_set_layer (material, 0, COGL_INVALID_HANDLE);
+
+  cogl_set_source (material);
+  cogl_rectangle (0, 0, QUAD_WIDTH, QUAD_WIDTH);
+
+  cogl_translate (0, QUAD_WIDTH, 0);
+  cogl_polygon (verts, 4, FALSE);
+
+  cogl_translate (0, QUAD_WIDTH, 0);
+  vbo = cogl_vertex_buffer_new (4);
+  cogl_vertex_buffer_add (vbo,
+                          "gl_Vertex",
+                          2, /* n components */
+                          COGL_ATTRIBUTE_TYPE_FLOAT,
+                          FALSE, /* normalized */
+                          sizeof (CoglTextureVertex), /* stride */
+                          verts);
+  cogl_vertex_buffer_draw (vbo,
+			   COGL_VERTICES_MODE_TRIANGLE_FAN,
+			   0, /* first */
+			   4); /* count */
+  cogl_handle_unref (vbo);
+
+  cogl_pop_matrix ();
+
+  cogl_handle_unref (material);
+
+  /* We expect a white fallback material to be used */
+  check_pixel (state, x, y,   0xffffffff);
+  check_pixel (state, x, y+1, 0xffffffff);
+  check_pixel (state, x, y+2, 0xffffffff);
+}
+
+static void
+on_paint (ClutterActor *actor, TestState *state)
+{
+  int frame_num;
+
+  test_invalid_texture_layers (state,
+                               0, 0 /* position */
+                               );
+
+  /* XXX: Experiments have shown that for some buggy drivers, when using
+   * glReadPixels there is some kind of race, so we delay our test for a
+   * few frames and a few seconds:
+   */
+  frame_num = state->frame++;
+  if (frame_num < SKIP_FRAMES)
+    g_usleep (G_USEC_PER_SEC);
+
+  /* Comment this out if you want visual feedback for what this test paints */
+#if 1
+  if (frame_num > SKIP_FRAMES)
+    clutter_main_quit ();
+#endif
+}
+
+static gboolean
+queue_redraw (gpointer stage)
+{
+  clutter_actor_queue_redraw (CLUTTER_ACTOR (stage));
+
+  return TRUE;
+}
+
+void
+test_materials (TestConformSimpleFixture *fixture,
+                gconstpointer data)
+{
+  TestState state;
+  ClutterActor *stage;
+  ClutterActor *group;
+  guint idle_source;
+
+  state.frame = 0;
+
+  stage = clutter_stage_get_default ();
+
+  clutter_stage_set_color (CLUTTER_STAGE (stage), &stage_color);
+  clutter_actor_get_geometry (stage, &state.stage_geom);
+
+  group = clutter_group_new ();
+  clutter_container_add_actor (CLUTTER_CONTAINER (stage), group);
+
+  /* We force continuous redrawing of the stage, since we need to skip
+   * the first few frames, and we wont be doing anything else that
+   * will trigger redrawing. */
+  idle_source = g_idle_add (queue_redraw, stage);
+
+  g_signal_connect (group, "paint", G_CALLBACK (on_paint), &state);
+
+  clutter_actor_show_all (stage);
+
+  clutter_main ();
+
+  g_source_remove (idle_source);
+
+  if (g_test_verbose ())
+    g_print ("OK\n");
+}
+