[Cogl] [PATCH] Rework sdl integration api

[Neil Roberts]

Robert Bragg wrote:

> + * while (!data.quit)
> + *   {
> + *     cogl_sdl_idle (data.ctx);
> + *     if (SDL_WaitEvent (&event))
> + *       do
> + *         {
> + *           handle_event (&data, &event);
> + *           cogl_sdl_handle_event (data.ctx, &event);
> + *         }
> + *       while (SDL_PollEvent (&event));
> + *     else
> + *       {
> + *         g_critical ("Error waiting for SDL events");
> + *         break;
> + *       }
> + *
> + *      render (&data);
> + *   }

I'm not sure if this loop makes much sense. This will make the loop
always block and wait for an event and then render after any event is
received. Was this meant to be an example of an application that renders
continuously? In that case the application effectively never goes idle,
so it should never call cogl_sdl_idle, right?

For the cogl-sdl-hello example where it really does block and only does
work in response to events, I think the loop should be more like this so
that it only calls cogl_sdl_idle when it knows it will block:

  while (!data.quit)
    {
      /* If there are currently no events then we are going to block
       * so we should give Cogl a chance to do any idle work and then
       * use SDL_WaitEvent to do the actual block */
      if (!SDL_PollEvent (&event))
        {
          cogl_sdl_idle (ctx);
          if (!SDL_WaitEvent (&event))
            {
              fprintf (stderr, "Error waiting for SDL events");
              return 1;
            }
        }

      handle_event (&data, &event);
      cogl_sdl_handle_event (ctx, &event);
    }

> +void
> +cogl_sdl_handle_event (CoglContext *context, SDL_Event *event)
> +{
...
> +  if (G_UNLIKELY (renderer->sdl_event_type_set == FALSE))
> +    g_critical ("cogl_sdl_renderer_set_event_type() ...")
> +                "must be called during initialization");

I think it would be better if this check was done in the implementation
of CoglRenderer.connect instead because you also need have set the event
type before that point anyway. Otherwise the screen would end up flooded
with critical warnings because cogl_sdl_handle_event should be called
very frequently.

Before, I think we agreed that if you are using the GSource and the
GMainLoop the public API would let you be agnostic of the winsys. For
that to work would the GSource somehow have to internally set the SDL
event? I think it would be reasonable to steal any old SDL event number
in that case because if you are using the GLib main loop then you
probably aren't using any SDL-specific API.

Regards,
- Neil