[Cogl] [PATCH 1/3] Add CoglOutput and track for the GLX backend

Robert Bragg robert at sixbynine.org
Tue Dec 4 02:53:31 PST 2012


On Wed, Nov 28, 2012 at 4:47 PM, <otaylor at redhat.com> wrote:

> From: "Owen W. Taylor" <otaylor at fishsoup.net>
>
> The CoglOutput object represents one output such as a monitor or
> laptop panel, with information about attributes of the output such as
> the position of the output within the global coordinate space, and
> the refresh rate.
>
> We don't yet publically export the ability to get output information but
> we track it for the GLX backend, where we'll use it to track the refresh
> rate.
>

Yeah, conceptually this all sounds good to me, we've also been thinking
we'd like to expose CoglOutputs at some point to enable control over the
overlay hardware so this could be a step in the right direction for that
too I think.


> ---
>  cogl/Makefile.am                  |   3 +
>  cogl/cogl-output-private.h        |  50 ++++++
>  cogl/cogl-output.h                |  87 ++++++++++
>  cogl/cogl-xlib-renderer-private.h |  11 ++
>  cogl/cogl-xlib-renderer.c         | 327
> ++++++++++++++++++++++++++++++++++++++
>  cogl/winsys/cogl-winsys-glx.c     | 106 +++++++++++-
>  cogl/winsys/cogl-winsys-private.h |   3 +
>  configure.ac                      |   3 +-
>  8 files changed, 581 insertions(+), 9 deletions(-)
>  create mode 100644 cogl/cogl-output-private.h
>  create mode 100644 cogl/cogl-output.h
>
> diff --git a/cogl/Makefile.am b/cogl/Makefile.am
> index a73d3f8..022caf5 100644
> --- a/cogl/Makefile.am
> +++ b/cogl/Makefile.am
> @@ -106,6 +106,7 @@ cogl_experimental_h = \
>         $(srcdir)/cogl-clip-state.h             \
>         $(srcdir)/cogl-framebuffer.h            \
>         $(srcdir)/cogl-onscreen.h               \
> +       $(srcdir)/cogl-output.h                 \
>         $(srcdir)/cogl-vector.h                 \
>         $(srcdir)/cogl-euler.h                  \
>         $(srcdir)/cogl-quaternion.h             \
> @@ -345,6 +346,8 @@ cogl_sources_c = \
>         $(srcdir)/cogl-framebuffer.c                    \
>         $(srcdir)/cogl-onscreen-private.h               \
>         $(srcdir)/cogl-onscreen.c                       \
> +       $(srcdir)/cogl-output-private.h                 \
> +       $(srcdir)/cogl-output.c                         \
>         $(srcdir)/cogl-profile.h                        \
>         $(srcdir)/cogl-profile.c                        \
>         $(srcdir)/cogl-flags.h                          \
> diff --git a/cogl/cogl-output-private.h b/cogl/cogl-output-private.h
> new file mode 100644
> index 0000000..05803de
> --- /dev/null
> +++ b/cogl/cogl-output-private.h
> @@ -0,0 +1,50 @@
> +/*
> + * Cogl
> + *
> + * An object oriented GL/GLES Abstraction/Utility Layer
> + *
> + * Copyright (C) 2012 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see <
> http://www.gnu.org/licenses/>.
> + *
> + *
> + */
> +
> +#ifndef __COGL_OUTPUT_PRIVATE_H
> +#define __COGL_OUTPUT_PRIVATE_H
> +
> +#include "cogl-output.h"
> +#include "cogl-object-private.h"
> +
> +struct _CoglOutput
> +{
> +  CoglObject _parent;
> +
> +  char *name;
> +
> +  int x; /* Must be first field for _cogl_output_values_equal() */
> +  int y;
> +  int width;
> +  int height;
> +  int mm_width;
> +  int mm_height;
> +  float refresh_rate;
> +  CoglSubpixelOrder subpixel_order;
> +};
> +
> +CoglOutput *_cogl_output_new (const char *name);
> +gboolean _cogl_output_values_equal (CoglOutput *output,
> +                                    CoglOutput *other);
>
Just a minor style note, this should probably use CoglBool for consistency


> +
> +#endif /* __COGL_OUTPUT_PRIVATE_H */
> diff --git a/cogl/cogl-output.h b/cogl/cogl-output.h
> new file mode 100644
> index 0000000..48da41f
> --- /dev/null
> +++ b/cogl/cogl-output.h
> @@ -0,0 +1,87 @@
> +/*
> + * Cogl
> + *
> + * An object oriented GL/GLES Abstraction/Utility Layer
> + *
> + * Copyright (C) 2012 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + *
> + *
> + * Authors:
> + *   Owen Taylor <otaylor at redhat.com>
> + */
> +#if !defined(__COGL_H_INSIDE__) && !defined(COGL_COMPILATION)
> +#error "Only <cogl/cogl.h> can be included directly."
> +#endif
> +
> +#ifndef __COGL_OUTPUT_H
> +#define __COGL_OUTPUT_H
> +
> +#include <cogl/cogl-types.h>
> +#include <glib.h>
> +
> +G_BEGIN_DECLS
> +
> +typedef struct _CoglOutput CoglOutput;
> +#define COGL_OUTPUT(X) ((CoglOutput *)(X))
> +
> +typedef enum {
> +  COGL_SUBPIXEL_ORDER_UNKNOWN,
> +  COGL_SUBPIXEL_ORDER_NONE,
> +  COGL_SUBPIXEL_ORDER_HORIZONTAL_RGB,
> +  COGL_SUBPIXEL_ORDER_HORIZONTAL_BGR,
> +  COGL_SUBPIXEL_ORDER_VERTICAL_RGB,
> +  COGL_SUBPIXEL_ORDER_VERTICAL_BGR
> +} CoglSubpixelOrder;
> +
> +/**
> + * cogl_is_output:
> + * @object: A #CoglObject pointer
> + *
> + * Gets whether the given object references a #CoglOutput.
> + *
> + * Return value: %TRUE if the object references a #CoglOutput
> + *   and %FALSE otherwise.
> + * Since: 2.0
> + * Stability: unstable
> + */
> +CoglBool
> +cogl_is_output (void *object);
> +
> +int
> +cogl_output_get_x (CoglOutput *output);
> +int
> +cogl_output_get_y (CoglOutput *output);
> +int
> +cogl_output_get_width (CoglOutput *output);
> +int
> +cogl_output_get_height (CoglOutput *output);
> +int
> +cogl_output_get_mm_width (CoglOutput *output);
> +int
> +cogl_output_get_mm_height (CoglOutput *output);
> +CoglSubpixelOrder
> +cogl_output_get_subpixel_order (CoglOutput *output);
> +float
> +cogl_output_get_refresh_rate (CoglOutput *output);
> +
>
This api seems like a good start, so it would nice to write some good
documentation for these.

It looks like you accidentally forgot to add cogl-output.c to this patch so
I can't review the implementations of this api yet...

+G_END_DECLS
> +
> +#endif /* __COGL_OUTPUT_H */
> +
> +
> +
> diff --git a/cogl/cogl-xlib-renderer-private.h
> b/cogl/cogl-xlib-renderer-private.h
> index 0c73b5c..cb3b60e 100644
> --- a/cogl/cogl-xlib-renderer-private.h
> +++ b/cogl/cogl-xlib-renderer-private.h
> @@ -28,6 +28,7 @@
>  #include "cogl-xlib-private.h"
>  #include "cogl-x11-renderer-private.h"
>  #include "cogl-context.h"
> +#include "cogl-output.h"
>
>  typedef struct _CoglXlibRenderer
>  {
> @@ -41,6 +42,9 @@ typedef struct _CoglXlibRenderer
>
>    /* A poll FD for handling event retrieval within Cogl */
>    CoglPollFD poll_fd;
> +
> +  unsigned long outputs_update_serial;
> +  GList *outputs;
>  } CoglXlibRenderer;
>

I think outputs should probably move up into CoglRenderer directly since
the intention is for other window systems to also be able to access these
later.


>  CoglBool
> @@ -92,4 +96,11 @@ _cogl_xlib_renderer_poll_dispatch (CoglRenderer
> *renderer,
>                                     const CoglPollFD *poll_fds,
>                                     int n_poll_fds);
>
> +CoglOutput *
> +_cogl_xlib_renderer_output_for_rectangle (CoglRenderer *renderer,
> +                                          int           x,
> +                                          int           y,
> +                                          int           width,
> +                                          int           height);
>

As a minor style note, we try and consistently avoid the temptation to
align type and argument names like these. Although it can look quite pretty
we've found numerous times now that it makes automatic refactoring of code
more awkward and also causes noise in patches when you're forced to
re-indent unrelated things.


@@ -259,6 +310,38 @@ _cogl_winsys_renderer_disconnect (CoglRenderer
> *renderer)
>    g_slice_free (CoglGLXRenderer, renderer->winsys);
>  }
>
> +static gboolean
> +update_all_outputs (CoglRenderer *renderer)
> +{
> +  GList *l;
> +
> +  _COGL_GET_CONTEXT (context, FALSE);
>

Instead of using _COGL_GET_CONTEXT, which we're incrementally trying to
eradicate, we should add a back reference from the renderer to the context
after creating a context in cogl_context_new(). For now we only allow
creating a single context and although we want to keep options open for
whether we allow sharing renderers/displays between multiple contexts in
the future its not something we need to worry about now. Adding a single
back reference for now shouldn't really limit us later later.

Generally it looks like this patch is taking the right direction, so
hopefully it shouldn't take much to revise this and we can get it landed.
Mainly I think it would be good to see cogl-output.c and some
documentation. I already made some patches for style fixes and moving the
outputs member to CoglRenderer which I can forward to the list and if
you're happy with those then please just squash them back into your patch
before sending out any revisions.

kind regards,
- Robert
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/cogl/attachments/20121204/ba4df7e3/attachment.html>


More information about the Cogl mailing list