Re: [PATCH] doc: Removed redundant xslt output elements.
On Thu, Nov 27, 2014 at 12:30:52PM -0800, Jon A. Cruz wrote: > Removed elements that were duplicated but with attributes in > a different order. Standard tools are required to ignore the order of > attributes in an element. > > Signed-off-by: Jon A. Cruz Reviewed-by: Peter Hutterer Cheers, Peter > --- > doc/publican/protocol-interfaces-to-docbook.xsl | 1 - > doc/publican/protocol-to-docbook.xsl| 1 - > 2 files changed, 2 deletions(-) > > diff --git a/doc/publican/protocol-interfaces-to-docbook.xsl > b/doc/publican/protocol-interfaces-to-docbook.xsl > index ad6bdda..9cf0695 100644 > --- a/doc/publican/protocol-interfaces-to-docbook.xsl > +++ b/doc/publican/protocol-interfaces-to-docbook.xsl > @@ -1,6 +1,5 @@ > > xmlns:xsl="http://www.w3.org/1999/XSL/Transform";> > - > > > > diff --git a/doc/publican/protocol-to-docbook.xsl > b/doc/publican/protocol-to-docbook.xsl > index 939ba40..443228d 100644 > --- a/doc/publican/protocol-to-docbook.xsl > +++ b/doc/publican/protocol-to-docbook.xsl > @@ -1,6 +1,5 @@ > > xmlns:xsl="http://www.w3.org/1999/XSL/Transform";> > - > > > > -- > 1.9.1 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 6/9] doc: Added \code tags around sample code in doxygen comments
More experiments, this time with Doxygen 1.8.6. Apparently the ~~~
markers (called "Markdown support" or "Fenced code blocks") were
introduced in 1.8. Maybe config needs to check for this? Indeed ~~~
produces code samples now.
It appears aliases (but not other commands) work inside ~~~. They do not
work inside \code.
There is a bug that causes asterisks: if a \code is anywhere earlier in
the source code (ie it can be in a different comment block) then when a
\comment is encountered in ~~~ it puts the asterisks on the start of
every remaining line. I would assume this is a Doxygen bug.
I think the fix is to remove \code everywhere and use the (nicer-looking
imho) ~~~ delimiters. It may also be useful to require the new Doxygen
in config.
On 11/27/2014 02:53 AM, Pekka Paalanen wrote:
On Wed, 26 Nov 2014 10:32:56 -0800
Bill Spitzak wrote:
On 11/25/2014 11:51 PM, Pekka Paalanen wrote:
What \comment-line command are you looking for?
I see you reformatted
- * wl_list_insert(&foo_list, &item1.link); \comment{Pushes item1 at the head}
but I assume you mean something else?
No I meant that. I thought somebody had made the comment command to
produce "/* text */". Apparently however this is a holdover from javadoc
or something and doxygen does not do it.
Sorry?
doc/doxygen/wayland.doxygen.in has:
ALIASES+= comment{1}="/* \1 */"
What's wrong with that?
I did some web searching, and everybody says the only way to do a
comment is to use a different comment style that can be nested. Since
C-style comments don't nest you have to use something else if the
doxygen comment is C-style. The doxygen code tag appears to make it
entirely literal, the only thing it recognizes is \endcode.
Not sure if it would be acceptable to use C++ comments in Wayland
source, but that would be a solution. It was how I got around this
problem before.
Another solution is to get Doxygen to produce code-like output but where
commands can be interpreted. Using and replacing every space with
and every newline with works but is really ugly.
FWIW, with the Doxygen version I have, that \comment{foo} did
produce /* foo */ in the output, just like one would expect, and it
worked nicely where it was used.
Our style is to use traditional C comments, not C++.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] doc: Removed redundant xslt output elements.
Removed elements that were duplicated but with attributes in a different order. Standard tools are required to ignore the order of attributes in an element. Signed-off-by: Jon A. Cruz --- doc/publican/protocol-interfaces-to-docbook.xsl | 1 - doc/publican/protocol-to-docbook.xsl| 1 - 2 files changed, 2 deletions(-) diff --git a/doc/publican/protocol-interfaces-to-docbook.xsl b/doc/publican/protocol-interfaces-to-docbook.xsl index ad6bdda..9cf0695 100644 --- a/doc/publican/protocol-interfaces-to-docbook.xsl +++ b/doc/publican/protocol-interfaces-to-docbook.xsl @@ -1,6 +1,5 @@ http://www.w3.org/1999/XSL/Transform";> - diff --git a/doc/publican/protocol-to-docbook.xsl b/doc/publican/protocol-to-docbook.xsl index 939ba40..443228d 100644 --- a/doc/publican/protocol-to-docbook.xsl +++ b/doc/publican/protocol-to-docbook.xsl @@ -1,6 +1,5 @@ http://www.w3.org/1999/XSL/Transform";> - -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/6] Attach input profiles and build corresponding LUTs
Am 27.11.2014 um 15:58 schrieb Niels Ole Salscheider:
> #define WL_HIDE_DEPRECATED
> #include
>
> @@ -40,6 +41,10 @@ extern "C" {
>
> #include "config-parser.h"
> #include "zalloc.h"
>
> +#ifdef HAVE_LCMS
> +#include
> +#endif
> +
>
> #ifndef MIN
> #define MIN(x,y) (((x) < (y)) ? (x) : (y))
> #endif
>
> @@ -179,6 +184,24 @@ enum weston_mode_switch_op {
>
> WESTON_MODE_SWITCH_RESTORE_NATIVE
>
> };
>
> +struct weston_clut {
> + unsigned points;
> + char *data;
> +};
> +
> +struct weston_colorspace {
> +#ifdef HAVE_LCMS
> + cmsHPROFILE lcms_handle;
> +#endif
> + struct weston_clut clut;
> +
> + int destroyable;
> + int refcount;
> + int input;
> +
> + struct weston_compositor *compositor;
> +};
>> Note, that compositor.h is a public, installed header. You cannot use
>> HAVE_LCMS, because any external project using this header would then
>> receive the definition based on its own configuration, not how the
>> Weston that is already installed was configured.
Agreed, lcms internals are not nice to get exposed at this level.
Generally speaking, lcms provides a great API for a CMM. On the other
side, lcms is just one CMM and people in the open source world decided
many times to define and implement different API's. E.g. ArgyllCMS
(features, speed), SampleICC (spec playground) and Mozilla/Chrome with
qcms (secuirity, speed). So it is not wise to stick to especially one
CMM, be it lcms or whatever.
A CMM module can register its own functions for their handle type. You
could either wrap the CMM functions or use initial dummy functions
instead for function pointers. The type of the handle is not really
needed to get exposed. As long as the CMM registers its API at startup,
that procedure is simple.
A different approach is to store the data blob of the profile. The lcms
expanded profile handle needs certainly more memory, than the plain
memory block. And loading the profile on the fly is really fast. You
need the CMM profile handle mostly in the weston_build_clut() function
and for the header ID computation in other places. So profile memory
blob and profile header ID would suffice?
Then following might work out for the later approach:
+struct weston_colorspace {
+
+ void * profile_data;
+ int profile_data_size;
+ unsigned char profile_id[16];
+
+ struct weston_clut clut;
+
+ int destroyable;
+ int refcount;
+ int input;
+
+ struct weston_compositor *compositor;
+};
This opens the path to exchange the CMM without much fuzz on start time
depending on the system requirements.
Thanks
Kai-Uwe
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland]scanner.c: Use WL_PRINTF instead of __attribute__((format(printf)))
Signed-off-by: Seedo Eldho Paul --- src/scanner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanner.c b/src/scanner.c index fa8e0c0..ca03c57 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -199,7 +199,7 @@ static const char *indent(int n) } static void -desc_dump(char *desc, const char *fmt, ...) __attribute__((format(printf,2,3))); +desc_dump(char *desc, const char *fmt, ...) WL_PRINTF(2, 3); static void desc_dump(char *desc, const char *fmt, ...) -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/6] Attach input profiles and build corresponding LUTs
Hi, > first a general question, since I'm at loss here on the big picture: > How does all this relate to the cms-static and cms-colord modules > already in Weston? > > Are those modules only about configuring the output's color space? > And then this work simply leverages that to have some output color > spaces to work with? > > Hmm, cms-static/colord set the gamma ramps..? So the hardware LUT? Yes, cms-static and cms-colord allow to get a color space for each output. This is done by querying colord or by reading a static configuration from weston.ini. Without my patches, only the hardware LUT is programmed with the gamma ramp from the corresponding profile. My patches reuse the existing modules to get the profile data for each output but then to do full color correction. > On Mon, 27 Oct 2014 18:54:06 +0100 > > Niels Ole Salscheider wrote: > > On Wednesday 15 October 2014, 21:54:37, Bryce Harrington wrote: > > > On Mon, Oct 13, 2014 at 07:40:47PM +0200, Niels Ole Salscheider wrote: > > > > This implements the functionality to attach a profile to a surface > > > > in weston. An LUT is built from the profile that can be used to > > > > transform colors from the input color space to the blending color > > > > space. > > > > > > > > An sRGB color space is assumed for all newly created outputs > > > > > > > > This patch uses the sRGB color space as blending space because it > > > > uses 8 bit LUTs for now and I want to avoid additional banding. In > > > > the long term we should use a linear blending space though to get > > > > correct results. > > > > > > > > Signed-off-by: Niels Ole Salscheider > > > > --- > > > > > > > > Makefile.am | 3 + > > > > configure.ac | 2 +- > > > > src/compositor.c | 411 > > > > ++- > > > > src/compositor.h > > > > > > > > | 41 ++ > > > > > > > > 4 files changed, 453 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Makefile.am b/Makefile.am > > > > index 3af3b46..1ecab56 100644 > > > > --- a/Makefile.am > > > > +++ b/Makefile.am > > > > @@ -45,6 +45,9 @@ weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON > > > > > > > > weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) > > > > $(LIBUNWIND_CFLAGS) > > > > weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ > > > > > > > > $(DLOPEN_LIBS) -lm libshared.la > > > > > > > > +if HAVE_LCMS > > > > +weston_LDADD += $(LCMS_LIBS) > > > > +endif > > > > > > > > weston_SOURCES = \ > > > > > > > > src/git-version.h \ > > > > > > > > diff --git a/configure.ac b/configure.ac > > > > index 1c133bd..00b7cca 100644 > > > > --- a/configure.ac > > > > +++ b/configure.ac > > > > @@ -59,7 +59,7 @@ AC_CHECK_HEADERS([execinfo.h]) > > > > > > > > AC_CHECK_FUNCS([mkostemp strchrnul initgroups posix_fallocate]) > > > > > > > > -COMPOSITOR_MODULES="wayland-server >= 1.5.91 pixman-1 >= 0.25.2" > > > > +COMPOSITOR_MODULES="wayland-server >= 1.5.91 pixman-1 >= 0.25.2 > > > > glib-2.0" > > > > > > Depending on glib seems a pretty big step. I see it's needed for > > > GHashTable, anything else? > > > > > > Wayland has a wl_map data structure; could that be used instead of > > > GHashTable, in order to avoid the glib dependency? > > Yeah, we can't have an unconditional dependency to glib. > > > Yes, it is only needed for GHashTable. "cms-colord.c" already uses > > GHashTable and therefore I used it in my code, too. But the use in > > "cms-colord.c" only adds a dependency to glib if weston is build with > > colord support, so this might be more acceptable. > > I could use another map implementation, e. g. the one from wayland - but > > then we would have to move it from "wayland-private.h" to some header > > that gets installed. > > wl_map from wayland-private.h is not going to happen either. libwayland > doesn't need a wl_map in its public API, so it has no place to export > it either, even if it was generic. > > I think you need to copy in a hash table implementation, if it is > needed regardless of... of... where is the switch to disable color > management support? It is enabled if weston is compiled with LCMS and disabled otherwise (or do you see any reason why you would want to have LCMS but not full color management?). > Or well, just do what Jasper said. Ok, I'll do so. > > > > AC_ARG_ENABLE(egl, [ --disable-egl],, > > > > > > > >enable_egl=yes) > > > > > > > > diff --git a/src/compositor.c b/src/compositor.c > > > > index 29731c7..4f959a4 100644 > > > > --- a/src/compositor.c > > > > +++ b/src/compositor.c > > > > @@ -56,6 +56,7 @@ > > > > > > > > #include "compositor.h" > > > > #include "scaler-server-protocol.h" > > > > #include "presentation_timing-server-protocol.h" > > > > > > > > +#include "cms-server-protocol.h" > > > > > > > > #include "../shared/os-compatibility.h" > > > > #include "git-version.h" > > > > #i
Re: [PATCH wayland] doc: fixed grammar and a typo
On Mon, 17 Nov 2014 14:59:14 -0600 Derek Foreman wrote: > Signed-off-by: Derek Foreman > --- > src/wayland-client.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/wayland-client.c b/src/wayland-client.c > index 01629e0..4067cfd 100644 > --- a/src/wayland-client.c > +++ b/src/wayland-client.c > @@ -427,8 +427,8 @@ wl_proxy_add_listener(struct wl_proxy *proxy, > * Gets the address to the proxy's listener; which is the listener set with > * \ref wl_proxy_add_listener. > * > - * This function is useful in client with multiple listeners on the same > - * interface to allow the identification of which code to eexecute. > + * This function is useful in clients with multiple listeners on the same > + * interface to allow the identification of which code to execute. > * > * \memberof wl_proxy > */ Pushed. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] input: Use slightly less obscure logic in evdev_notify_keyboard_focus()
On Wed, 19 Nov 2014 11:04:12 -0600
Derek Foreman wrote:
> While the test is actually correct (for non-negative numbers), it's not
> at all clear and seems to be an accidental order of operations mistake.
>
> Also, add an assert() to make sure this number is never negative.
>
> Closes bug 86346 - https://bugs.freedeskto.org/show_bug.cgi?id=86346
>
> Signed-off-by: Derek Foreman
> ---
>
> This version just moves the assert directly after the decrement...
>
> src/input.c | 1 +
> src/libinput-device.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/input.c b/src/input.c
> index 5f19875..cd5e10b 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -2203,6 +2203,7 @@ WL_EXPORT void
> weston_seat_release_keyboard(struct weston_seat *seat)
> {
> seat->keyboard_device_count--;
> + assert(seat->keyboard_device_count >= 0);
> if (seat->keyboard_device_count == 0) {
> weston_keyboard_set_focus(seat->keyboard, NULL);
> weston_keyboard_cancel_grab(seat->keyboard);
> diff --git a/src/libinput-device.c b/src/libinput-device.c
> index 0e3f46d..8a48905 100644
> --- a/src/libinput-device.c
> +++ b/src/libinput-device.c
> @@ -470,7 +470,7 @@ evdev_notify_keyboard_focus(struct weston_seat *seat,
> {
> struct wl_array keys;
>
> - if (!seat->keyboard_device_count > 0)
> + if (seat->keyboard_device_count == 0)
> return;
>
> wl_array_init(&keys);
Pushed with Marek's R-b.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/6] Attach input profiles and build corresponding LUTs
Hi,
first a general question, since I'm at loss here on the big picture:
How does all this relate to the cms-static and cms-colord modules
already in Weston?
Are those modules only about configuring the output's color space?
And then this work simply leverages that to have some output color
spaces to work with?
Hmm, cms-static/colord set the gamma ramps..? So the hardware LUT?
On Mon, 27 Oct 2014 18:54:06 +0100
Niels Ole Salscheider wrote:
> On Wednesday 15 October 2014, 21:54:37, Bryce Harrington wrote:
> > On Mon, Oct 13, 2014 at 07:40:47PM +0200, Niels Ole Salscheider wrote:
> > > This implements the functionality to attach a profile to a surface
> > > in weston. An LUT is built from the profile that can be used to
> > > transform colors from the input color space to the blending color
> > > space.
> > >
> > > An sRGB color space is assumed for all newly created outputs
> > >
> > > This patch uses the sRGB color space as blending space because it
> > > uses 8 bit LUTs for now and I want to avoid additional banding. In
> > > the long term we should use a linear blending space though to get
> > > correct results.
> > >
> > > Signed-off-by: Niels Ole Salscheider
> > > ---
> > >
> > > Makefile.am | 3 +
> > > configure.ac | 2 +-
> > > src/compositor.c | 411
> > > ++- src/compositor.h
> > > | 41 ++
> > > 4 files changed, 453 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 3af3b46..1ecab56 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -45,6 +45,9 @@ weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
> > >
> > > weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> > > weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> > >
> > > $(DLOPEN_LIBS) -lm libshared.la
> > >
> > > +if HAVE_LCMS
> > > +weston_LDADD += $(LCMS_LIBS)
> > > +endif
> > >
> > > weston_SOURCES = \
> > >
> > > src/git-version.h \
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 1c133bd..00b7cca 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -59,7 +59,7 @@ AC_CHECK_HEADERS([execinfo.h])
> > >
> > > AC_CHECK_FUNCS([mkostemp strchrnul initgroups posix_fallocate])
> > >
> > > -COMPOSITOR_MODULES="wayland-server >= 1.5.91 pixman-1 >= 0.25.2"
> > > +COMPOSITOR_MODULES="wayland-server >= 1.5.91 pixman-1 >= 0.25.2 glib-2.0"
> >
> > Depending on glib seems a pretty big step. I see it's needed for
> > GHashTable, anything else?
> >
> > Wayland has a wl_map data structure; could that be used instead of
> > GHashTable, in order to avoid the glib dependency?
Yeah, we can't have an unconditional dependency to glib.
> Yes, it is only needed for GHashTable. "cms-colord.c" already uses GHashTable
> and therefore I used it in my code, too. But the use in "cms-colord.c" only
> adds a dependency to glib if weston is build with colord support, so this
> might be more acceptable.
> I could use another map implementation, e. g. the one from wayland - but then
> we would have to move it from "wayland-private.h" to some header that gets
> installed.
wl_map from wayland-private.h is not going to happen either. libwayland
doesn't need a wl_map in its public API, so it has no place to export
it either, even if it was generic.
I think you need to copy in a hash table implementation, if it is
needed regardless of... of... where is the switch to disable color
management support?
Or well, just do what Jasper said.
> > > AC_ARG_ENABLE(egl, [ --disable-egl],,
> > >
> > >enable_egl=yes)
> > >
> > > diff --git a/src/compositor.c b/src/compositor.c
> > > index 29731c7..4f959a4 100644
> > > --- a/src/compositor.c
> > > +++ b/src/compositor.c
> > > @@ -56,6 +56,7 @@
> > >
> > > #include "compositor.h"
> > > #include "scaler-server-protocol.h"
> > > #include "presentation_timing-server-protocol.h"
> > >
> > > +#include "cms-server-protocol.h"
> > >
> > > #include "../shared/os-compatibility.h"
> > > #include "git-version.h"
> > > #include "version.h"
> > >
> > > @@ -518,7 +519,8 @@ surface_state_handle_buffer_destroy(struct wl_listener
> > > *listener, void *data)>
> > > }
> > >
> > > static void
> > >
> > > -weston_surface_state_init(struct weston_surface_state *state)
> > > +weston_surface_state_init(struct weston_surface_state *state,
> > > + struct weston_compositor *compositor)
> > >
> > > {
> > >
> > > state->newly_attached = 0;
> > > state->buffer = NULL;
> > >
> > > @@ -539,6 +541,8 @@ weston_surface_state_init(struct weston_surface_state
> > > *state)>
> > > state->buffer_viewport.buffer.src_width = wl_fixed_from_int(-1);
> > > state->buffer_viewport.surface.width = -1;
> > > state->buffer_viewport.changed = 0;
> > >
> > > +
> > > + state->colorspace = &compositor->srgb_colorspace;
> > >
> > > }
> > >
> > >
Re: [PATCH 1/6] Add cms protocol
Hi, I had a quick look of our previous discussion to refresh my mind. It is very good that you moved to using a protocol object for a color profile, and fd passing for the profile data. Here's some comments. On Mon, 13 Oct 2014 19:40:46 +0200 Niels Ole Salscheider wrote: > The cms protocol allows to attach an ICC profile to a surface. It also tells > the client about the blending color space and the color spaces of all outputs. > > Signed-off-by: Niels Ole Salscheider > --- > Makefile.am | 15 +-- > protocol/cms.xml | 132 > +++ > 2 files changed, 143 insertions(+), 4 deletions(-) > create mode 100644 protocol/cms.xml > > diff --git a/protocol/cms.xml b/protocol/cms.xml > new file mode 100644 > index 000..34c1b16 > --- /dev/null > +++ b/protocol/cms.xml > @@ -0,0 +1,132 @@ > + > + > + > + > +Copyright © 2014 Niels Ole Salscheider > + > + > + > + So techincally speaking, wl_cms is a global singleton. That should be mentioned here, so it's obvious that creating a wl_cms object just allows you to access the interface, but has no state or other effects. > + This interface allows to attach a color space to a wl_surface. This is > + used by the compositor to display the colors correctly. For this, the > + compositor converts any attached surfaces to the blending color space > + before the blending operations. After blending, the output surface is > + converted to the color space of the output device. > + This interface also provides requests for the sRGB and the blending > color > + space. It further allows to create a color space from an ICC profile. > + The client is informed by an event if the color space of one of the > + outputs changes. > + > + > + > + > +With this request, the color space of a wl_surface can be set. > +Initially, the sRGB colorspace as returned by the srgb_colorspace > +request is attached to a surface. Need to tell about the double-buffering and interaction with wl_surface.commit. > + > + > + > + > + > + > + > +This request allows to create a wl_cms_colorspace object from an ICC > +profile. The fd argument is the file descriptor to the ICC profile. > + > + > + No need for a size argument? You assume the whole file from zero to end is the profile? How does one get the size? You should also carefully specify, what the data behind the fd is. Do you need an extra argument to specify a type? Maybe in the future one would like to add another type as an alternative? For reference, see wl_keyboard.keymap event. > + > + > + > + > +This request returns a wl_cms_colorspace object for the requested It does not "return" anything. This request creates a wl_cms_colorspace object that refers to the color profile of the given output. (I want to avoid the term "return" in protocol language, because "return" implies a roundtrip. Requests are semantically not function calls either, a common misconception.) This applies to most requests below also. > +output. A client can use this when it does not want its surfaces to > be > +color-corrected. In this case it can attach the color space of its > main > +output to its surfaces. > + > + > + > + > + > + > + I think the summary is not right, is it? > +This request returns a wl_cms_colorspace object for the sRGB color > +space. The sRGB color space is initially attached to all surfaces. I like this very much: a shorthand for getting an object for a standard color space. > + > + > + > + > + > + > +This request returns a wl_cms_colorspace object for the blending > color > +space of the compositor. All surfaces are converted by the compositor > +to the blending color space before the blending operations. A client > +should render in this color space if it does any color conversion on > +its own. > + > + > + > + > + > + > +This event will be sent when the color space of an output is changed. Oh, so output color space can change? Ok, I assume the correct reaction from a client is to get the output color space again. > + > + > + > + > + > + + summary="the passed icc data is invalid" /> You forgot to document this in the request that may raise this error. Protocol errors are always fatal. Is invalid ICC data reason enough to disconnect the whole client? It's certainly easy and clear. However, I saw Bryce mentioning different ICC versions. If the compositor understands only version 2, but a client provides a correct version 4 profile, does it get killed for it? I think you may want an explicit profile format argument, that unambiguously describes what the format is. Actually, you are
Re: [PATCH 3/9] doc: preserve links produced by Doxygen
On Wed, 26 Nov 2014 11:24:28 -0800 "Jon A. Cruz" wrote: > > > On 11/26/2014 10:22 AM, Bill Spitzak wrote: > > On 11/25/2014 11:52 PM, Pekka Paalanen wrote: > > > >> Or are duplicates perhaps due to us having, say, struct wl_display a > >> different thing on server vs. client? > > > > The duplicates are due mostly to wl_list (and similar things) being > > documented twice. But I think doxygen does generate some identical tags > > for the server and client documentation. The solution may be to edit one > > of these and add an extra letter to all the tags so there are no > > conflicts. Telling doxygen to document both in one pass caused it to mix > > all the calls together, probably a bad idea. > > I definitely think this sounds like an issue I should look into. One of > the projects I worked on was to figure out how to create design > documents from doxygen, and to coordinate design docs from multiple > teams working on multiple projects/features. > > One key to getting overall documentation to work was to coordinate > identifiers to either end up with different ones or unified ones on a > case by case basis. > > The *ideal* would be to get a single unified Doxygen pass working. > However, it does warrant investigation as to how close to that a > pragmatic solution appropriate to Wayland would get us. Two passes is > not that bad. Once I start to get details and options figured out it > definitely looks like Bill will be able to give feedback and guidance. Please do. If you need it, it is perfectly ok to have server and client side passes separate. I'd like them to be separate HTML pages, too. It doesn't hurt, if we happen to get duplicate documentation for things like wl_list, one for server and one for client. The important thing is to never mix up server stuff with client stuff. Btw. libwayland-cursor is client side stuff and a separate library. For example: struct wl_display is very different between server and client. Struct wl_buffer has three completely different meanings: - client side public API: it is a syntactic type without an actual definition: the real struct behind it is wl_proxy. - server side public API: it is a deprecated real type with public members. - libwayland internal API: a ring byte buffer, completely contained in src/connection.c, used by both server and client side internal code. Luckily connection.c probably should not be included in the generated docs, so we can overlook the third meaning. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 6/9] doc: Added \code tags around sample code in doxygen comments
On Wed, 26 Nov 2014 10:32:56 -0800
Bill Spitzak wrote:
> On 11/25/2014 11:51 PM, Pekka Paalanen wrote:
>
> > What \comment-line command are you looking for?
> >
> > I see you reformatted
> >
> - * wl_list_insert(&foo_list, &item1.link); \comment{Pushes item1 at
> the head}
> >
> > but I assume you mean something else?
>
> No I meant that. I thought somebody had made the comment command to
> produce "/* text */". Apparently however this is a holdover from javadoc
> or something and doxygen does not do it.
Sorry?
doc/doxygen/wayland.doxygen.in has:
ALIASES+= comment{1}="/* \1 */"
What's wrong with that?
> I did some web searching, and everybody says the only way to do a
> comment is to use a different comment style that can be nested. Since
> C-style comments don't nest you have to use something else if the
> doxygen comment is C-style. The doxygen code tag appears to make it
> entirely literal, the only thing it recognizes is \endcode.
>
> Not sure if it would be acceptable to use C++ comments in Wayland
> source, but that would be a solution. It was how I got around this
> problem before.
>
> Another solution is to get Doxygen to produce code-like output but where
> commands can be interpreted. Using and replacing every space with
> and every newline with works but is really ugly.
>
FWIW, with the Doxygen version I have, that \comment{foo} did
produce /* foo */ in the output, just like one would expect, and it
worked nicely where it was used.
Our style is to use traditional C comments, not C++.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] compositor: add weston_surface_set_description()
On Thu, 27 Nov 2014 10:17:21 +0200
Giulio Camuffo wrote:
> Just a quick comment below, I haven't looked at it carefully yet.
>
> --
> Giulio
>
> 2014-11-27 9:07 GMT+02:00 Pekka Paalanen :
> > From: Pekka Paalanen
> >
> > When printing out logs from Weston's actions, mainly for debugging, it
> > can be very difficult to identify the different surfaces. Inspecting
> > the configure function pointer is not useful, as the configure functions
> > may live in modules.
> >
> > Add vfunc get_description to weston_surface, which will produce a short,
> > human-readable description of the surface, which allows identifying it
> > better, rather than just looking at the surface size, for instance.
> >
> > Set the description from most parts of Weston, to identify cursors and
> > drag icons, and panels, backgrounds, screensavers and lock surfaces, and
> > the desktop shell's application surfaces.
> >
> > Signed-off-by: Pekka Paalanen
> > ---
> > desktop-shell/input-panel.c | 9
> > desktop-shell/shell.c | 101
> >
> > src/compositor.c| 17
> > src/compositor.h| 7 +++
> > src/data-device.c | 19 +
> > src/input.c | 10 +
> > 6 files changed, 163 insertions(+)
...
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 17e930c..ea5cc14 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -2736,6 +2736,13 @@ weston_subsurface_parent_commit(struct
> > weston_subsurface *sub,
> > weston_subsurface_synchronized_commit(sub);
> > }
> >
> > +static int
> > +subsurface_get_description(struct weston_surface *surface,
> > + char *buf, size_t len)
> > +{
> > + return snprintf(buf, len, "sub-surface");
> > +}
> > +
> > static void
> > subsurface_configure(struct weston_surface *surface, int32_t dx, int32_t
> > dy)
> > {
> > @@ -2819,6 +2826,14 @@ weston_surface_set_role(struct weston_surface
> > *surface,
> > return -1;
> > }
> >
> > +WL_EXPORT void
> > +weston_surface_set_description(struct weston_surface *surface,
>
> I find the naming a bit confusing, I'd expect this to set the
> description of the surface, that is a char*. The get_description vfunc
> on the other hand doesn't return the function passed here, but the
> char*. I'd call it something like
> weston_surface_set_description_func(), or maybe
> weston_surface_set_descriptor().
Yes, weston_surface_set_get_description_func() it would be indeed, but
it was getting a bit long, especially when the argument is usually
a fairly long word too.
Descriptor smells a bit like an fd thing, though.
weston_surface_set_... eh... umm
What if I did s/description/label/g?
So we'd have e.g.:
weston_surface_set_label_func(surface, shell_surface_get_label)
and weston_surface::get_label field.
How's that?
Thanks,
pq
>
> > + int (*desc)(struct weston_surface *,
> > + char *, size_t))
> > +{
> > + surface->get_description = desc;
> > +}
> > +
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] compositor: add weston_surface_set_description()
Just a quick comment below, I haven't looked at it carefully yet.
--
Giulio
2014-11-27 9:07 GMT+02:00 Pekka Paalanen :
> From: Pekka Paalanen
>
> When printing out logs from Weston's actions, mainly for debugging, it
> can be very difficult to identify the different surfaces. Inspecting
> the configure function pointer is not useful, as the configure functions
> may live in modules.
>
> Add vfunc get_description to weston_surface, which will produce a short,
> human-readable description of the surface, which allows identifying it
> better, rather than just looking at the surface size, for instance.
>
> Set the description from most parts of Weston, to identify cursors and
> drag icons, and panels, backgrounds, screensavers and lock surfaces, and
> the desktop shell's application surfaces.
>
> Signed-off-by: Pekka Paalanen
> ---
> desktop-shell/input-panel.c | 9
> desktop-shell/shell.c | 101
>
> src/compositor.c| 17
> src/compositor.h| 7 +++
> src/data-device.c | 19 +
> src/input.c | 10 +
> 6 files changed, 163 insertions(+)
>
> diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
> index 0b42c2f..d699c47 100644
> --- a/desktop-shell/input-panel.c
> +++ b/desktop-shell/input-panel.c
> @@ -150,6 +150,13 @@ update_input_panels(struct wl_listener *listener, void
> *data)
> memcpy(&shell->text_input.cursor_rectangle, data,
> sizeof(pixman_box32_t));
> }
>
> +static int
> +input_panel_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "input panel");
> +}
> +
> static void
> input_panel_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
> {
> @@ -187,6 +194,7 @@ destroy_input_panel_surface(struct input_panel_surface
> *input_panel_surface)
> wl_list_remove(&input_panel_surface->link);
>
> input_panel_surface->surface->configure = NULL;
> + weston_surface_set_description(input_panel_surface->surface, NULL);
> weston_view_destroy(input_panel_surface->view);
>
> free(input_panel_surface);
> @@ -228,6 +236,7 @@ create_input_panel_surface(struct desktop_shell *shell,
>
> surface->configure = input_panel_configure;
> surface->configure_private = input_panel_surface;
> + weston_surface_set_description(surface, input_panel_get_description);
>
> input_panel_surface->shell = shell;
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 9e8d45a..22e1b25 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -274,6 +274,32 @@ static void
> shell_surface_set_parent(struct shell_surface *shsurf,
> struct weston_surface *parent);
>
> +static int
> +shell_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + struct shell_surface *shsurf;
> + const char *typestr[] = {
> + [SHELL_SURFACE_NONE] = "unidentified",
> + [SHELL_SURFACE_TOPLEVEL] = "top-level",
> + [SHELL_SURFACE_POPUP] = "popup",
> + [SHELL_SURFACE_XWAYLAND] = "Xwayland",
> + };
> + const char *t, *c;
> +
> + shsurf = get_shell_surface(surface);
> + if (!shsurf)
> + return snprintf(buf, len, "unidentified window");
> +
> + t = shsurf->title;
> + c = shsurf->class;
> +
> + return snprintf(buf, len, "%s window%s%s%s%s%s",
> + typestr[shsurf->type],
> + t ? " '" : "", t ?: "", t ? "'" : "",
> + c ? " of " : "", c ?: "");
> +}
> +
> static bool
> shell_surface_is_top_fullscreen(struct shell_surface *shsurf)
> {
> @@ -634,6 +660,13 @@ get_default_output(struct weston_compositor *compositor)
> struct weston_output, link);
> }
>
> +static int
> +focus_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + return snprintf(buf, len, "focus highlight effect for output %s",
> + surface->output->name);
> +}
>
> /* no-op func for checking focus surface */
> static void
> @@ -683,6 +716,7 @@ create_focus_surface(struct weston_compositor *ec,
> surface->configure = focus_surface_configure;
> surface->output = output;
> surface->configure_private = fsurf;
> + weston_surface_set_description(surface,
> focus_surface_get_description);
>
> fsurf->view = weston_view_create(surface);
> if (fsurf->view == NULL) {
> @@ -2741,6 +2775,34 @@ shell_surface_get_shell(struct shell_surface *shsurf)
> return shsurf->shell;
> }
>
> +static int
> +black_surface_get_description(struct weston_surface *surface,
> + char *buf, size_t len)
> +{
> + struct westo
