<div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 28, 2012 at 4:47 PM,  <span dir="ltr"><<a href="mailto:otaylor@redhat.com" target="_blank">otaylor@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
From: "Owen W. Taylor" <<a href="mailto:otaylor@fishsoup.net">otaylor@fishsoup.net</a>><br>
<br>
The CoglOutput object represents one output such as a monitor or<br>
laptop panel, with information about attributes of the output such as<br>
the position of the output within the global coordinate space, and<br>
the refresh rate.<br>
<br>
We don't yet publically export the ability to get output information but<br>
we track it for the GLX backend, where we'll use it to track the refresh<br>
rate.<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
---<br>
 cogl/Makefile.am                  |   3 +<br>
 cogl/cogl-output-private.h        |  50 ++++++<br>
 cogl/cogl-output.h                |  87 ++++++++++<br>
 cogl/cogl-xlib-renderer-private.h |  11 ++<br>
 cogl/cogl-xlib-renderer.c         | 327 ++++++++++++++++++++++++++++++++++++++<br>
 cogl/winsys/cogl-winsys-glx.c     | 106 +++++++++++-<br>
 cogl/winsys/cogl-winsys-private.h |   3 +<br>
 <a href="http://configure.ac" target="_blank">configure.ac</a>                      |   3 +-<br>
 8 files changed, 581 insertions(+), 9 deletions(-)<br>
 create mode 100644 cogl/cogl-output-private.h<br>
 create mode 100644 cogl/cogl-output.h<br>
<br>
diff --git a/cogl/Makefile.am b/cogl/Makefile.am<br>
index a73d3f8..022caf5 100644<br>
--- a/cogl/Makefile.am<br>
+++ b/cogl/Makefile.am<br>
@@ -106,6 +106,7 @@ cogl_experimental_h = \<br>
        $(srcdir)/cogl-clip-state.h             \<br>
        $(srcdir)/cogl-framebuffer.h            \<br>
        $(srcdir)/cogl-onscreen.h               \<br>
+       $(srcdir)/cogl-output.h                 \<br>
        $(srcdir)/cogl-vector.h                 \<br>
        $(srcdir)/cogl-euler.h                  \<br>
        $(srcdir)/cogl-quaternion.h             \<br>
@@ -345,6 +346,8 @@ cogl_sources_c = \<br>
        $(srcdir)/cogl-framebuffer.c                    \<br>
        $(srcdir)/cogl-onscreen-private.h               \<br>
        $(srcdir)/cogl-onscreen.c                       \<br>
+       $(srcdir)/cogl-output-private.h                 \<br>
+       $(srcdir)/cogl-output.c                         \<br>
        $(srcdir)/cogl-profile.h                        \<br>
        $(srcdir)/cogl-profile.c                        \<br>
        $(srcdir)/cogl-flags.h                          \<br>
diff --git a/cogl/cogl-output-private.h b/cogl/cogl-output-private.h<br>
new file mode 100644<br>
index 0000000..05803de<br>
--- /dev/null<br>
+++ b/cogl/cogl-output-private.h<br>
@@ -0,0 +1,50 @@<br>
+/*<br>
+ * Cogl<br>
+ *<br>
+ * An object oriented GL/GLES Abstraction/Utility Layer<br>
+ *<br>
+ * Copyright (C) 2012 Red Hat, Inc.<br>
+ *<br>
+ * This library is free software; you can redistribute it and/or<br>
+ * modify it under the terms of the GNU Lesser General Public<br>
+ * License as published by the Free Software Foundation; either<br>
+ * version 2 of the License, or (at your option) any later version.<br>
+ *<br>
+ * This library is distributed in the hope that it will be useful,<br>
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU<br>
+ * Lesser General Public License for more details.<br>
+ *<br>
+ * You should have received a copy of the GNU Lesser General Public<br>
+ * License along with this library. If not, see <<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>>.<br>
+ *<br>
+ *<br>
+ */<br>
+<br>
+#ifndef __COGL_OUTPUT_PRIVATE_H<br>
+#define __COGL_OUTPUT_PRIVATE_H<br>
+<br>
+#include "cogl-output.h"<br>
+#include "cogl-object-private.h"<br>
+<br>
+struct _CoglOutput<br>
+{<br>
+  CoglObject _parent;<br>
+<br>
+  char *name;<br>
+<br>
+  int x; /* Must be first field for _cogl_output_values_equal() */<br>
+  int y;<br>
+  int width;<br>
+  int height;<br>
+  int mm_width;<br>
+  int mm_height;<br>
+  float refresh_rate;<br>
+  CoglSubpixelOrder subpixel_order;<br>
+};<br>
+<br>
+CoglOutput *_cogl_output_new (const char *name);<br>
+gboolean _cogl_output_values_equal (CoglOutput *output,<br>
+                                    CoglOutput *other);<br></blockquote><div>Just a minor style note, this should probably use CoglBool for consistency</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

+<br>
+#endif /* __COGL_OUTPUT_PRIVATE_H */<br>
diff --git a/cogl/cogl-output.h b/cogl/cogl-output.h<br>
new file mode 100644<br>
index 0000000..48da41f<br>
--- /dev/null<br>
+++ b/cogl/cogl-output.h<br>
@@ -0,0 +1,87 @@<br>
+/*<br>
+ * Cogl<br>
+ *<br>
+ * An object oriented GL/GLES Abstraction/Utility Layer<br>
+ *<br>
+ * Copyright (C) 2012 Red Hat, Inc.<br>
+ *<br>
+ * This library is free software; you can redistribute it and/or<br>
+ * modify it under the terms of the GNU Lesser General Public<br>
+ * License as published by the Free Software Foundation; either<br>
+ * version 2 of the License, or (at your option) any later version.<br>
+ *<br>
+ * This library is distributed in the hope that it will be useful,<br>
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU<br>
+ * Lesser General Public License for more details.<br>
+ *<br>
+ * You should have received a copy of the GNU Lesser General Public<br>
+ * License along with this library. If not, see<br>
+ * <<a href="http://www.gnu.org/licenses/" target="_blank">http://www.gnu.org/licenses/</a>>.<br>
+ *<br>
+ *<br>
+ *<br>
+ * Authors:<br>
+ *   Owen Taylor <<a href="mailto:otaylor@redhat.com">otaylor@redhat.com</a>><br>
+ */<br>
+#if !defined(__COGL_H_INSIDE__) && !defined(COGL_COMPILATION)<br>
+#error "Only <cogl/cogl.h> can be included directly."<br>
+#endif<br>
+<br>
+#ifndef __COGL_OUTPUT_H<br>
+#define __COGL_OUTPUT_H<br>
+<br>
+#include <cogl/cogl-types.h><br>
+#include <glib.h><br>
+<br>
+G_BEGIN_DECLS<br>
+<br>
+typedef struct _CoglOutput CoglOutput;<br>
+#define COGL_OUTPUT(X) ((CoglOutput *)(X))<br>
+<br>
+typedef enum {<br>
+  COGL_SUBPIXEL_ORDER_UNKNOWN,<br>
+  COGL_SUBPIXEL_ORDER_NONE,<br>
+  COGL_SUBPIXEL_ORDER_HORIZONTAL_RGB,<br>
+  COGL_SUBPIXEL_ORDER_HORIZONTAL_BGR,<br>
+  COGL_SUBPIXEL_ORDER_VERTICAL_RGB,<br>
+  COGL_SUBPIXEL_ORDER_VERTICAL_BGR<br>
+} CoglSubpixelOrder;<br>
+<br>
+/**<br>
+ * cogl_is_output:<br>
+ * @object: A #CoglObject pointer<br>
+ *<br>
+ * Gets whether the given object references a #CoglOutput.<br>
+ *<br>
+ * Return value: %TRUE if the object references a #CoglOutput<br>
+ *   and %FALSE otherwise.<br>
+ * Since: 2.0<br>
+ * Stability: unstable<br>
+ */<br>
+CoglBool<br>
+cogl_is_output (void *object);<br>
+<br>
+int<br>
+cogl_output_get_x (CoglOutput *output);<br>
+int<br>
+cogl_output_get_y (CoglOutput *output);<br>
+int<br>
+cogl_output_get_width (CoglOutput *output);<br>
+int<br>
+cogl_output_get_height (CoglOutput *output);<br>
+int<br>
+cogl_output_get_mm_width (CoglOutput *output);<br>
+int<br>
+cogl_output_get_mm_height (CoglOutput *output);<br>
+CoglSubpixelOrder<br>
+cogl_output_get_subpixel_order (CoglOutput *output);<br>
+float<br>
+cogl_output_get_refresh_rate (CoglOutput *output);<br>
+<br></blockquote><div>This api seems like a good start, so it would nice to write some good documentation for these.</div><div><br></div><div>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...</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+G_END_DECLS<br>
+<br>
+#endif /* __COGL_OUTPUT_H */<br>
+<br>
+<br>
+<br>
diff --git a/cogl/cogl-xlib-renderer-private.h b/cogl/cogl-xlib-renderer-private.h<br>
index 0c73b5c..cb3b60e 100644<br>
--- a/cogl/cogl-xlib-renderer-private.h<br>
+++ b/cogl/cogl-xlib-renderer-private.h<br>
@@ -28,6 +28,7 @@<br>
 #include "cogl-xlib-private.h"<br>
 #include "cogl-x11-renderer-private.h"<br>
 #include "cogl-context.h"<br>
+#include "cogl-output.h"<br>
<br>
 typedef struct _CoglXlibRenderer<br>
 {<br>
@@ -41,6 +42,9 @@ typedef struct _CoglXlibRenderer<br>
<br>
   /* A poll FD for handling event retrieval within Cogl */<br>
   CoglPollFD poll_fd;<br>
+<br>
+  unsigned long outputs_update_serial;<br>
+  GList *outputs;<br>
 } CoglXlibRenderer;<br></blockquote><div><br></div><div>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.</div><div>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
 CoglBool<br>
@@ -92,4 +96,11 @@ _cogl_xlib_renderer_poll_dispatch (CoglRenderer *renderer,<br>
                                    const CoglPollFD *poll_fds,<br>
                                    int n_poll_fds);<br>
<br>
+CoglOutput *<br>
+_cogl_xlib_renderer_output_for_rectangle (CoglRenderer *renderer,<br>
+                                          int           x,<br>
+                                          int           y,<br>
+                                          int           width,<br>
+                                          int           height);<br></blockquote><div><br></div><div>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.</div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
@@ -259,6 +310,38 @@ _cogl_winsys_renderer_disconnect (CoglRenderer *renderer)<br>
   g_slice_free (CoglGLXRenderer, renderer->winsys);<br>
 }<br>
<br>
+static gboolean<br>
+update_all_outputs (CoglRenderer *renderer)<br>
+{<br>
+  GList *l;<br>
+<br>
+  _COGL_GET_CONTEXT (context, FALSE);<br></blockquote><div><br></div><div>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.<br>
</div><div><br></div><div>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.</div>
<div><br></div><div>kind regards,</div><div>- Robert</div></div></div>