Re: [PATCH weston 7/8] compositor-drm: Implement clone mode, refactor output into logical ones
On Mon, 21 Nov 2016 19:41:33 + Daniel Stone wrote: > Hi Emmanuel, > > On 2 May 2016 at 22:40, Emmanuel Gil Peyrot > wrote: > > Introduces a “same-as” configuration option for each output, which > > bypasses the rest of the output configuration (mode, scale, transform > > and seat) and instead makes it a clone of the specified output. > > > > This is implemented by splitting the drm_output struct into the > > per-connector drm_output and the per-weston_output drm_logical_output, > > with the latter containing one or more of the former in a wl_list. > > Hmm. Let me stop you there ... > > > +struct drm_logical_output { > > + struct weston_output base; > > + struct wl_list output_list; > > + > > + int page_flip_refcount; > > This to me is a red flag. I think you've built this patch at the wrong level. > > There are really two kinds of clone mode: same-CRTC and different > CRTC. Same-CRTC can share planes, and they share timing as well. I > think same-CRTC clones should be ganged together within the backend, > and different-CRTC clones should appear entirely disjoint from the > backend's point of view. > > With the caveat that my review of Armin's drm_keep_current_mode patch > shows that I comprehensively don't yet understand the new output > configuration API - so I'd like his input on this as well - I'd > suggest the following approach. > > Firstly, keep every connector as a separate weston_output, as it is today. > > When you parse an outputB same-as outputA configuration, call a new > backend vfunc: outputA->chain_output(outputB). If this returns > success, mark output B as chained from output A: not appearing in a > surface's output_mask[0], not having dpms/repaint/... called on it, > etc. If this returns failure, then continue to consider them as > totally separate outputs. In the DRM backend, this would simply mean > that we passed multiple connectors to drmModeSetCrtc and multiple > connector_states bound to the same CRTC. > > This would remove a lot of complexity from the DRM backend, and also > allow planes to work when we can clone multiple connectors on the same > CRTC. It would also mean we avoid a possible drop to 30fps, if the two > CRTCs end up clocked half the repaint cycle apart, and thus didn't > call weston_output_finish_frame until too late. Plus it has the added > benefit of working better when we have a constrained number of CRTCs, > or constrained CRTC capability, or whatever. > > The core already has to deal with surfaces overlapping multiple 'real' > outputs - i.e. running on different paint cycles - today, so I don't > see that punting clones running on disjoint CRTCs makes that problem > worse in terms of complexity for the core. We could certainly do > better with repaint-cycle time reporting[1] in the core, but just > punting down to the backend only makes that worse rather than better, > I think. Hi, about the surface/output overlap here... we certainly do handle a surface overlapping several disjoint outputs, but I still have not convinced myself that we handle mutually overlapping outputs properly. Why do I say that? Because struct weston_plane has a member called 'damage', and drm_output_render() directly subtracts damage from the primary plane. Pretty much every backend does that. Thanks, pq > Doing it this way would make also make the patchset a lot less > invasive, particularly if rebased on top of the > atomic/plane_state/output_state patchset ... ;) > > Sorry to be the bearer of bad news. > > Cheers, > Daniel > > [0]: Well. The client should still see the output for > wl_surface::{enter,leave} events, but the backend should only see the > 'real' outputs, not the chained ones. Maybe we need a > client_output_mask, and a backend_output_mask ... ? > [1]: Have a surface span two weston_outputs with disjoint paint > clocks. Watch the surface's frame-event timing bounce between the two > outputs, with resultant jittery framerate. Presumably we should pick > one 'master' surface to clock from, and stick to that. > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel pgpkGRAzBM4Kw.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
I Daniel, I did this patch because I didn't know when the EGL extension to advertise EGL supported format will be available. Since Varad patch set I understand that it is in a good way to land shortly. So we can abandon my patch. BR Vincent On 11/21/2016 06:49 PM, Daniel Stone wrote: > Hi Vincent, > > On 7 October 2016 at 16:08, Vincent Abriou wrote: >> @@ -72,6 +72,9 @@ install-libweston_moduleLTLIBRARIES >> install-moduleLTLIBRARIES: install-libLTLIBR >> lib_LTLIBRARIES = libweston-@LIBWESTON_MAJOR@.la >> libweston_@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON >> libweston_@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) >> $(LIBUNWIND_CFLAGS) >> +if ENABLE_DRM_COMPOSITOR >> +libweston_@LIBWESTON_MAJOR@_la_CFLAGS += $(DRM_COMPOSITOR_CFLAGS) >> +endif >> libweston_@LIBWESTON_MAJOR@_la_LIBADD = $(COMPOSITOR_LIBS) >> $(LIBUNWIND_LIBS) \ >> $(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \ >> $(LIBINPUT_BACKEND_LIBS) libshared.la > > So, given that the set of formats is defined unconditionally in > gl-renderer, this would break compilation with --disable-drm-backend. > The options that come to mind are either: using wl_shm enums for these > formats, shipping a local copy of drm_fourcc.h, or mandating the DRM > backend for dmabuf. Of those, I'd prefer the first, I think. > > I did merge your YUYV patch, though: thanks! > > Cheers, > Daniel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 7/8] compositor-drm: Implement clone mode, refactor output into logical ones
Hi Pekka, On 22 November 2016 at 08:54, Pekka Paalanen wrote: > On Mon, 21 Nov 2016 19:41:33 + Daniel Stone wrote: >> On 2 May 2016 at 22:40, Emmanuel Gil Peyrot >> wrote: >> The core already has to deal with surfaces overlapping multiple 'real' >> outputs - i.e. running on different paint cycles - today, so I don't >> see that punting clones running on disjoint CRTCs makes that problem >> worse in terms of complexity for the core. We could certainly do >> better with repaint-cycle time reporting[1] in the core, but just >> punting down to the backend only makes that worse rather than better, >> I think. > > about the surface/output overlap here... we certainly do handle a > surface overlapping several disjoint outputs, but I still have not > convinced myself that we handle mutually overlapping outputs properly. > > Why do I say that? Because struct weston_plane has a member called > 'damage', and drm_output_render() directly subtracts damage from the > primary plane. Pretty much every backend does that. Thanks for this - it's a good point. It's strengthened my resolve a little bit though, especially in the context of https://phabricator.freedesktop.org/T7621, where our problem is that repaint is inseparable from, and damaging[0] to, core state. If we pulled primary_plane out of core, and started calculating repaint data per-call instead, we could kill two birds with one stone. And also promote surfaces to planes even if they do span multiple outputs. Cheers, Daniel [0]: Damaging in the sense of buffer damage, i.e. makes changes to. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
Hi Vincent, On 22 November 2016 at 08:54, Vincent ABRIOU wrote: > I did this patch because I didn't know when the EGL extension to > advertise EGL supported format will be available. > Since Varad patch set I understand that it is in a good way to land shortly. > > So we can abandon my patch. No, I think this patch is still good! Many GPUs do not support YUV formats natively, so will not advertise them through this EGL extension; your patch allows us to provide them rather than making clients fall back to conversion on their side. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/4] test: start with the first offset when moving touches
Hi, On 22-11-16 02:32, Peter Hutterer wrote: This doesn't have an effect in our current tests because the touchpad always needs 4 motion events to get moving. But for the future, it simplifies the case of "i want to move between x1/y1 and x2/y2", because it fills in only the events in between rather than re-using the touch down coordinates and thus not causing a motion on the first event. Signed-off-by: Peter Hutterer Series looks good to me and is: Reviewed-by: Hans de Goede Regards, Hans --- test/litest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/litest.c b/test/litest.c index 94da85a..e685d61 100644 --- a/test/litest.c +++ b/test/litest.c @@ -1603,7 +1603,7 @@ litest_touch_move_to(struct litest_device *d, double x_to, double y_to, int steps, int sleep_ms) { - for (int i = 0; i < steps - 1; i++) { + for (int i = 1; i < steps - 1; i++) { litest_touch_move(d, slot, x_from + (x_to - x_from)/steps * i, y_from + (y_to - y_from)/steps * i); @@ -1699,7 +1699,7 @@ litest_touch_move_two_touches(struct litest_device *d, double dx, double dy, int steps, int sleep_ms) { - for (int i = 0; i < steps - 1; i++) { + for (int i = 1; i < steps; i++) { litest_push_event_frame(d); litest_touch_move(d, 0, x0 + dx / steps * i, y0 + dy / steps * i); ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: only use the last two coordinates for delta calculation
Hi, On 22-11-16 02:34, Peter Hutterer wrote: Taking the last 4 points means factoring in a coordinate that may be more than 40ms in the past - or even more when the finger moves slowly and we don't get events for a while. This makes the pointer more sluggish and slower to catch up with what the finger is actually doing. We already have the motion hysteresis as a separate item to prevent jumps (and thus adds some delay to the movement), the calculation over time doesn't provide enough benefit to justify the sluggish pointer. Signed-off-by: Peter Hutterer --- I'm leaving the motion history as-is for now even though it's largely unused now. This can be fixed up later once we know this patch has the desired effect (it does here, but that could be confirmation bias) Patch looks good to me and is: Reviewed-by: Hans de Goede Regards, Hans src/evdev-mt-touchpad.c | 20 +--- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index d72cb19..7b8514c 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -262,29 +262,19 @@ tp_end_sequence(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) tp_end_touch(tp, t, time); } -static double -tp_estimate_delta(int x0, int x1, int x2, int x3) -{ - return (x0 + x1 - x2 - x3) / 4.0; -} - struct normalized_coords tp_get_delta(struct tp_touch *t) { struct device_float_coords delta; const struct normalized_coords zero = { 0.0, 0.0 }; - if (t->history.count < TOUCHPAD_MIN_SAMPLES) + if (t->history.count <= 1) return zero; - delta.x = tp_estimate_delta(tp_motion_history_offset(t, 0)->x, - tp_motion_history_offset(t, 1)->x, - tp_motion_history_offset(t, 2)->x, - tp_motion_history_offset(t, 3)->x); - delta.y = tp_estimate_delta(tp_motion_history_offset(t, 0)->y, - tp_motion_history_offset(t, 1)->y, - tp_motion_history_offset(t, 2)->y, - tp_motion_history_offset(t, 3)->y); + delta.x = tp_motion_history_offset(t, 0)->x - + tp_motion_history_offset(t, 1)->x; + delta.y = tp_motion_history_offset(t, 0)->y - + tp_motion_history_offset(t, 1)->y; return tp_normalize_delta(t->tp, delta); } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons
Hey, On 21 November 2016 at 23:13, Peter Hutterer wrote: > On Mon, Nov 21, 2016 at 12:42:36PM +, Daniel Stone wrote: >> Concretely though, reusing BTN_* codes where possible would make it >> easier for clients to transition between the two. > > I disagree here. The kernel only has BTN_STYLUS and BTN_STYLUS2, after that > we overlap with DOUBLETAP range and later buttons that are completely > different (e.g. BTN_GEAR_DOWN). I think this would only make it worse. > This protocol is still unstable, every client needs updates once we mark it > stable anyway, making the enums *values* mean something is counterproductive > IMO. Shrug, once in an enum they're totally arbitrary values (so which BTN_* they overlap doesn't make a difference), and it does make it a little harder to screw it up, as well as easier to stay compatible between multiple versions. But, your call. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
Hi Daniel, This was not my understanding of the EGL extension. In that case I agree it still make sense. I will then propose a new implementation to take into account your comment. Vincent On 11/22/2016 10:07 AM, Daniel Stone wrote: > Hi Vincent, > > On 22 November 2016 at 08:54, Vincent ABRIOU wrote: >> I did this patch because I didn't know when the EGL extension to >> advertise EGL supported format will be available. >> Since Varad patch set I understand that it is in a good way to land shortly. >> >> So we can abandon my patch. > > No, I think this patch is still good! Many GPUs do not support YUV > formats natively, so will not advertise them through this EGL > extension; your patch allows us to provide them rather than making > clients fall back to conversion on their side. > > Cheers, > Daniel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] doc: start documenting Xwayland
Hi, On 21 November 2016 at 15:06, Pekka Paalanen wrote: > On Mon, 21 Nov 2016 14:31:43 +0200 > Pekka Paalanen wrote: >> + >> + X11 and Wayland are different enough that there is no "simple" way to >> + translate between them. Most of X11 is uninteresting to a Wayland >> + compositor. That combined with the gigantic implementation effort >> needed >> + to support X11 makes it untractable to just write X11 support >> directly in Nit: intractable. >> + >> + Therefore Wayland compositors should use Xwayland, the X11 server that >> + lives in the Xorg server source code repository and shares most of the >> + implementation with the Xorg server. Xwayland is a complete X11 >> server, >> + just like Xorg is, but instead of talking to hardware and device >> drivers, >> + it acts as a Wayland client. The rest of this chapter talks about how >> + Xwayland works. >> + Hm, Xwayland still talks to hardware when running Glamor; it just doesn't drive KMS / display controllers directly. >> + >> + The X11 window manager (XWM) is an integral part of the Wayland >> + compositor. XWM uses the usual X11 window management protocol to >> manage >> + all X11 windows in Xwayland. Most importantly, XWM acts as a bridge >> + between Xwayland window state and the Wayland compositor's window >> manager >> + (WWM). This way WWM can manage all windows, both native Wayland and >> X11 >> + (Xwayland) windows. This is very important for a coherent user >> + experience. >> + Probably worth pointing out explicitly that XWM is, like other windows, a real genuine X11 client. And that this creates a dependency loop which requires careful handling: Xwayland is a Wayland client of the compositor, which is an X11 client of Xwayland. >> + >> + Xwayland uses Wayland core protocol and some extensions for input and >> + output. The used protocol is all generic and public, there are no >> + Xwayland-specific or private Wayland extensions being used. This is true, strictly speaking, but maybe misleading by omission as the WL_SURFACE_ID X11 property is essentially private protocol between Xwayland and XWM / the compositor. Even without that, I'm not sure we want to paint ourselves into a corner by disallowing the use of private protocol in the future. >> + Xwayland is >> + just a regular Wayland client, except it is specially handled by the >> WWM. WWM? >> +
Re: [weston] linux-dmabuf: align DMABUF exposed formats with EGL supported formats
Hi Vincent, On 22 November 2016 at 09:14, Vincent ABRIOU wrote: > This was not my understanding of the EGL extension. > In that case I agree it still make sense. > I will then propose a new implementation to take into account your comment. The EGL extension is to advertise all formats which can be natively imported by the EGL stack, and then bound directly to a sampler. The YUV cases, where we import as a format like R/RG and then do colourspace conversion in a user-provided shader, do not meet this criteria. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 7/8] compositor-drm: Implement clone mode, refactor output into logical ones
On Tue, 22 Nov 2016 09:06:00 + Daniel Stone wrote: > Hi Pekka, > > On 22 November 2016 at 08:54, Pekka Paalanen wrote: > > On Mon, 21 Nov 2016 19:41:33 + Daniel Stone > > wrote: > >> On 2 May 2016 at 22:40, Emmanuel Gil Peyrot > >> wrote: > >> The core already has to deal with surfaces overlapping multiple 'real' > >> outputs - i.e. running on different paint cycles - today, so I don't > >> see that punting clones running on disjoint CRTCs makes that problem > >> worse in terms of complexity for the core. We could certainly do > >> better with repaint-cycle time reporting[1] in the core, but just > >> punting down to the backend only makes that worse rather than better, > >> I think. > > > > about the surface/output overlap here... we certainly do handle a > > surface overlapping several disjoint outputs, but I still have not > > convinced myself that we handle mutually overlapping outputs properly. > > > > Why do I say that? Because struct weston_plane has a member called > > 'damage', and drm_output_render() directly subtracts damage from the > > primary plane. Pretty much every backend does that. > > Thanks for this - it's a good point. It's strengthened my resolve a > little bit though, especially in the context of > https://phabricator.freedesktop.org/T7621, where our problem is that > repaint is inseparable from, and damaging[0] to, core state. If we > pulled primary_plane out of core, and started calculating repaint data > per-call instead, we could kill two birds with one stone. And also > promote surfaces to planes even if they do span multiple outputs. Indeed, and maybe even allow targeting renderers to arbitrary planes rather than the hardcoded primary plane. Then we'd "just" need the planner from hwc2 if it was a good fit... Thanks, pq > Cheers, > Daniel > > [0]: Damaging in the sense of buffer damage, i.e. makes changes to. pgpYAOkReq1Zy.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] doc: start documenting Xwayland
On Tue, 22 Nov 2016 09:20:48 + Daniel Stone wrote: > Hi, > Hi Daniel, nice comments, I'll revise the patch later. Some replies below. > On 21 November 2016 at 15:06, Pekka Paalanen > wrote: > > On Mon, 21 Nov 2016 14:31:43 +0200 > > Pekka Paalanen wrote: > >> + > >> + X11 and Wayland are different enough that there is no "simple" way > >> to > >> + translate between them. Most of X11 is uninteresting to a Wayland > >> + compositor. That combined with the gigantic implementation effort > >> needed > >> + to support X11 makes it untractable to just write X11 support > >> directly in > > Nit: intractable. > > >> + > >> + Therefore Wayland compositors should use Xwayland, the X11 server > >> that > >> + lives in the Xorg server source code repository and shares most of > >> the > >> + implementation with the Xorg server. Xwayland is a complete X11 > >> server, > >> + just like Xorg is, but instead of talking to hardware and device > >> drivers, > >> + it acts as a Wayland client. The rest of this chapter talks about > >> how > >> + Xwayland works. > >> + > > Hm, Xwayland still talks to hardware when running Glamor; it just > doesn't drive KMS / display controllers directly. > > >> + > >> + The X11 window manager (XWM) is an integral part of the Wayland > >> + compositor. XWM uses the usual X11 window management protocol to > >> manage > >> + all X11 windows in Xwayland. Most importantly, XWM acts as a bridge > >> + between Xwayland window state and the Wayland compositor's window > >> manager > >> + (WWM). This way WWM can manage all windows, both native Wayland and > >> X11 > >> + (Xwayland) windows. This is very important for a coherent user > >> + experience. > >> + > > Probably worth pointing out explicitly that XWM is, like other > windows, a real genuine X11 client. And that this creates a dependency > loop which requires careful handling: Xwayland is a Wayland client of > the compositor, which is an X11 client of Xwayland. > > >> + > >> + Xwayland uses Wayland core protocol and some extensions for input > >> and > >> + output. The used protocol is all generic and public, there are no > >> + Xwayland-specific or private Wayland extensions being used. > > This is true, strictly speaking, but maybe misleading by omission as > the WL_SURFACE_ID X11 property is essentially private protocol between > Xwayland and XWM / the compositor. Even without that, I'm not sure we > want to paint ourselves into a corner by disallowing the use of > private protocol in the future. > > >> + Xwayland is > >> + just a regular Wayland client, except it is specially handled by > >> the WWM. > > WWM? Introduced in the previous paragraph: Wayland window manager; by that I mean the proper window manager in a Wayland compositor, to differentiate from the component that acts as a X11 WM, even if you count XWM as a part of WWM. Does any better terminology come to mind? Or is the split between XWM and WWM just a Weston thing? > >> +
Re: [PATCH weston 7/8] compositor-drm: Implement clone mode, refactor output into logical ones
Hi, On 22 November 2016 at 09:41, Pekka Paalanen wrote: > On Tue, 22 Nov 2016 09:06:00 + Daniel Stone wrote: >> Thanks for this - it's a good point. It's strengthened my resolve a >> little bit though, especially in the context of >> https://phabricator.freedesktop.org/T7621, where our problem is that >> repaint is inseparable from, and damaging[0] to, core state. If we >> pulled primary_plane out of core, and started calculating repaint data >> per-call instead, we could kill two birds with one stone. And also >> promote surfaces to planes even if they do span multiple outputs. > > Indeed, and maybe even allow targeting renderers to arbitrary planes > rather than the hardcoded primary plane. Then we'd "just" need the > planner from hwc2 if it was a good fit... I'd probably say these should be the other way around. Allocating new GPU surfaces and populating them with buffers for every plane we want to test, on the off chance that it might work, is a lot of work to do in our repaint loop; I'd rather not be doing it for every view of every frame ... That being said, I have no real data to back this up, so hey, maybe it is actually fine. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
KVM and Xen guest fail with cirrus emulated graphic card
Hello, I've recently tried Fedora 25 inside both a KVM and Xen guest. In both cases, the emulated graphic card selected by default is the "bad old" Cirrus. So, as I reported in my comment to this (already existing) bug: https://bugzilla.redhat.com/show_bug.cgi?id=1227770 - on Wayland, with Cirrus as emulated graphic card, I don't see the GNOME login screen appearing. - if I keep Cirrus and disable Wayland (via /etc/gdm/custom.conf), both Xen and KVM guests works ok. - if I keep wayland and use a different graphic card (vga or qxl, for instance), both Xen and KVM guests works ok. There's not much information online, and what's there suggests that it may be due to the hw characteristics of Cirrus (e.g., the 24bpp thing), which are not supported by Wayland. The issue has been brought up and is being discussed in this thread too: https://lists.xen.org/archives/html/xen-devel/2016-11/msg01579.html Now, what we're interested to know is, if possible: - how we can actually figure out what the issue is (I've poked around, but haven't found much in any log)? - is it something that can be made to work? If yes, is it likely to happen, or will this be a 'won't fix'? This is relevant because, as I said, for both for KVM and Xen, what we're getting _by_default_ is a non working guest, if Wayland is used. :-( Thanks and Regards, Dario PS. please, keep me in Cc, if possible, as I'm not subscribed right now... -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] protocol: Define further the behavior of input on the presence of grabs
Hi Carlos, On 12 November 2015 at 12:31, Carlos Garnacho wrote: > The leave events in the respective device interfaces has been further > documented so those can convey the necessary info when input is being > redirected out of their currently focused surface. > > Only wl_touch is missing something semantically similar, a wl_touch.leave > event has been added so the touch interface can cope with such input > redirection situations. Just over a year later. :\ I think I'd put this message into a tab to review later, then at some point Chrome lost all my tabs and that was one of them ... > @@ -1665,6 +1665,19 @@ > > The leave notification is sent before the enter notification > for the new focus. > + > + Normally, a pointer will not leave a surface while there are > + buttons pressed, there's however circumstances where this event > + may be received in this situation, for example: > + > + - When a popup is shown by this or other client. > + - When a drag-and-drop operation is initiated from this or > + any other surface. > + - Other compositor-specific grabs, like pointer gestures. > + > + In these situations, the pairing button release will not be > + emitted, clients should undo the effect of those pressed > + buttons and forget about their state. The client _must_ either receive a release or a leave event in a timely manner, and strictly before all other input events from that seat. > @@ -1699,6 +1712,14 @@ > enter event. > The time argument is a timestamp with millisecond > granularity, with an undefined base. > + > +Clients should note that pressed/released events may not be > +paired, most commonly due to input being actively taken away > +or being redirected into this surface (eg. popups), either > +state must be expected to be received separately. If no such > +input redirection happened in between, the same surface that > +received the "pressed" state is expected to receive the > +"released" state too. > Hm, to be honest, I'd just go with: 'Clients should note that pressed/released events may not be paired; in some cases, a leave event will be sent in lieu of a released event. See that event's documentation for more details'. > @@ -1798,6 +1819,17 @@ > > The leave notification is sent before the enter notification > for the new focus. > + > + There are circumstances that force a focus switch, possibly > + while there are pressed keys, for example: > + > + - When a popup is shown by this or other client. > + - When a drag-and-drop operation is initiated from this or > + any other surface. > + > + In these situations, the pairing key release will not be > + emitted, clients should undo the effect of those pressed > + pressed and forget about their state. I'd probably avoid specifically mentioning focus switching here. > @@ -1816,6 +1848,14 @@ > A key was pressed or released. > The time argument is a timestamp with millisecond > granularity, with an undefined base. > + > +Clients should note that pressed/released events may not be > +paired, most commonly due to input being actively taken away > +or being redirected into this surface (eg. popups), either > +state must be expected to be received separately. If no such > +input redirection happened in between, the same surface that > +received the "pressed" state is expected to receive the > +"released" state too. > Same comment as with wl_pointer's leave. > @@ -1938,6 +1978,25 @@ > > > > + > + > + > + > + > + Notification that this touch point is no longer focused on a > + certain surface. > + > + Note that, because touch points are always implicitly grabbed, > + this event will only be emitted when the touch point is taken > + away from this surface through explicit means, for example: > + > + - When a popup is shown by this or other client. > + - When a drag-and-drop operation is initiated from this or > + any other surface. > + > + > + > + > This looks fine to me, though is missing its counterpart comment in wl_touch::down. Would you be OK with those changes? Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] xwayland: Create the drag-and-drop window in weston_wm_dnd_init
Hi, On 16 November 2016 at 15:32, Daniel Stone wrote: > On 24 May 2016 at 19:34, Derek Foreman wrote: >> On 22/02/16 12:42 PM, Carlos Garnacho wrote: >>> Just to keep it hidden so far... A lot of the plumbing necessary to >>> handle x11->wayland drag and drop is missing, and the current >>> partial handling gets in the middle for X11 drag-and-drop itself >>> to work. >>> >>> The approach is well directed, but needs some further work, till >>> then, just keep our fake drag-and-drop target hidden. This allows >>> drag-and-drop to work between X11 clients in Xwayland, and avoids >>> a crash with (currently unhandled) wl_resource-less data sources. >>> >>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94218 >> >> This seems to be a pretty important stop-gap patch neglected for quite >> some time. :( > > Yikes, quite some time indeed. > >> It looks superficially ok to me, but I'm really not equipped to provide >> proper review, I'll happily: >> Acked-by: Derek Foreman >> because I trust Carlos to follow up :) > > Carlos, do you still think this is good? > >> And maybe if we can get linkmauve to test, as he was the author of the >> bug ticket, we can land it? > > Emmanuel, can you please test? Actually, I looked further into this, and agree with what Carlos has said in the bug; I guess we need to do a bit of work yet to make this actually work properly. And, having trivially tested before and after: Reviewed-by: Daniel Stone To ssh://git.freedesktop.org/git/wayland/weston 24d306c..11f8fcb upstream -> master Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] compositor: allow using nested parent as a subsurface sibling
Hi Arnaud, On 8 June 2016 at 17:37, Arnaud Vrac wrote: > The parent of a subsurface can be used as a sibling in the place_below > and place_above calls. However this did not work when the parent is > nested, so fix the sibling check and add a test to check this case. It took a few reads over, but after that, and writing a few more tests to validate things (which, disappointingly, passed on both old and new Weston): Reviewed-by: Daniel Stone I've made one very minor change though, which is to change 'struct wl_surface *parent' to 'struct wl_surface *grandchild' in the new tests: after all, the surface we're creating will never be the parent of anything, is the child of one thing, and crucially for the test, is the child of something which already has a child. Hope that's OK. Thanks for the patch! To ssh://git.freedesktop.org/git/wayland/weston 11f8fcb..b8c16c9 upstream -> master Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] tests: Extend subsurface place_{above, below} hierarchy
Following on from b8c16c995b, extend the family tree being tested by place_above and place_below a little, ensuring that subsurfaces can't be placed above or below surfaces which are related to them, but aren't their immediate parent or sibling. Signed-off-by: Daniel Stone Cc: Arnaud Vrac Cc: Pekka Paalanen --- tests/subsurface-test.c | 142 1 file changed, 142 insertions(+) diff --git a/tests/subsurface-test.c b/tests/subsurface-test.c index 03ff539..edb0842 100644 --- a/tests/subsurface-test.c +++ b/tests/subsurface-test.c @@ -272,6 +272,77 @@ TEST(test_subsurface_place_above_nested_parent) client_roundtrip(client); } +TEST(test_subsurface_place_above_grandparent) +{ + struct client *client; + struct compound_surface com; + struct wl_surface *grandchild; + struct wl_subsurface *sub; + struct wl_subcompositor *subco; + + client = create_client_and_test_surface(100, 50, 123, 77); + assert(client); + + populate_compound_surface(&com, client); + + subco = get_subcompositor(client); + grandchild = wl_compositor_create_surface(client->wl_compositor); + sub = wl_subcompositor_get_subsurface(subco, grandchild, com.child[0]); + + /* can't place a subsurface above its grandparent */ + wl_subsurface_place_above(sub, com.parent); + + expect_protocol_error(client, &wl_subsurface_interface, + WL_SUBSURFACE_ERROR_BAD_SURFACE); +} + +TEST(test_subsurface_place_above_great_aunt) +{ + struct client *client; + struct compound_surface com; + struct wl_surface *grandchild; + struct wl_subsurface *sub; + struct wl_subcompositor *subco; + + client = create_client_and_test_surface(100, 50, 123, 77); + assert(client); + + populate_compound_surface(&com, client); + + subco = get_subcompositor(client); + grandchild = wl_compositor_create_surface(client->wl_compositor); + sub = wl_subcompositor_get_subsurface(subco, grandchild, com.child[0]); + + /* can't place a subsurface above its parents' siblings */ + wl_subsurface_place_above(sub, com.child[1]); + + expect_protocol_error(client, &wl_subsurface_interface, + WL_SUBSURFACE_ERROR_BAD_SURFACE); +} + +TEST(test_subsurface_place_above_child) +{ + struct client *client; + struct compound_surface com; + struct wl_surface *grandchild; + struct wl_subcompositor *subco; + + client = create_client_and_test_surface(100, 50, 123, 77); + assert(client); + + populate_compound_surface(&com, client); + + subco = get_subcompositor(client); + grandchild = wl_compositor_create_surface(client->wl_compositor); + wl_subcompositor_get_subsurface(subco, grandchild, com.child[0]); + + /* can't place a subsurface above its own child subsurface */ + wl_subsurface_place_above(com.sub[0], grandchild); + + expect_protocol_error(client, &wl_subsurface_interface, + WL_SUBSURFACE_ERROR_BAD_SURFACE); +} + TEST(test_subsurface_place_below_nested_parent) { struct client *client; @@ -294,6 +365,77 @@ TEST(test_subsurface_place_below_nested_parent) client_roundtrip(client); } +TEST(test_subsurface_place_below_grandparent) +{ + struct client *client; + struct compound_surface com; + struct wl_surface *grandchild; + struct wl_subsurface *sub; + struct wl_subcompositor *subco; + + client = create_client_and_test_surface(100, 50, 123, 77); + assert(client); + + populate_compound_surface(&com, client); + + subco = get_subcompositor(client); + grandchild = wl_compositor_create_surface(client->wl_compositor); + sub = wl_subcompositor_get_subsurface(subco, grandchild, com.child[0]); + + /* can't place a subsurface below its grandparent */ + wl_subsurface_place_below(sub, com.parent); + + expect_protocol_error(client, &wl_subsurface_interface, + WL_SUBSURFACE_ERROR_BAD_SURFACE); +} + +TEST(test_subsurface_place_below_great_aunt) +{ + struct client *client; + struct compound_surface com; + struct wl_surface *grandchild; + struct wl_subsurface *sub; + struct wl_subcompositor *subco; + + client = create_client_and_test_surface(100, 50, 123, 77); + assert(client); + + populate_compound_surface(&com, client); + + subco = get_subcompositor(client); + grandchild = wl_compositor_create_surface(client->wl_compositor); + sub = wl_subcompositor_get_subsurface(subco, grandchild, com.child[0]); + + /* can't place a subsurface below its parents' siblings */ + wl_subsurface_place_below(sub, com.child[1]); + + expect_protocol_error(client, &wl_subsurface_interface, + WL_SUBSURFACE_ERROR_BAD_SURFACE); +} + +TEST(
Re: [PATCH v2] protocol: Define further the behavior of input on the presence of grabs
Hey Daniel :), On Tue, Nov 22, 2016 at 12:43 PM, Daniel Stone wrote: > Hi Carlos, > > On 12 November 2015 at 12:31, Carlos Garnacho wrote: >> The leave events in the respective device interfaces has been further >> documented so those can convey the necessary info when input is being >> redirected out of their currently focused surface. >> >> Only wl_touch is missing something semantically similar, a wl_touch.leave >> event has been added so the touch interface can cope with such input >> redirection situations. > > Just over a year later. :\ I think I'd put this message into a tab to > review later, then at some point Chrome lost all my tabs and that was > one of them ... Oh well... you're not alone, it's been in my backlog for a while too :). > >> @@ -1665,6 +1665,19 @@ >> >> The leave notification is sent before the enter notification >> for the new focus. >> + >> + Normally, a pointer will not leave a surface while there are >> + buttons pressed, there's however circumstances where this event >> + may be received in this situation, for example: >> + >> + - When a popup is shown by this or other client. >> + - When a drag-and-drop operation is initiated from this or >> + any other surface. >> + - Other compositor-specific grabs, like pointer gestures. >> + >> + In these situations, the pairing button release will not be >> + emitted, clients should undo the effect of those pressed >> + buttons and forget about their state. > > The client _must_ either receive a release or a leave event in a > timely manner, and strictly before all other input events from that > seat. > >> @@ -1699,6 +1712,14 @@ >> enter event. >> The time argument is a timestamp with millisecond >> granularity, with an undefined base. >> + >> +Clients should note that pressed/released events may not be >> +paired, most commonly due to input being actively taken away >> +or being redirected into this surface (eg. popups), either >> +state must be expected to be received separately. If no such >> +input redirection happened in between, the same surface that >> +received the "pressed" state is expected to receive the >> +"released" state too. >> > > Hm, to be honest, I'd just go with: 'Clients should note that > pressed/released events may not be paired; in some cases, a leave > event will be sent in lieu of a released event. See that event's > documentation for more details'. Indeed, just explaining it thoroughly in one of the 2 events make sense. I actually wonder if it could be explained generically for all keyboard/pointer/touch (eg. a separate section), but it's probably better anyway to state the per-interface specifics. > >> @@ -1798,6 +1819,17 @@ >> >> The leave notification is sent before the enter notification >> for the new focus. >> + >> + There are circumstances that force a focus switch, possibly >> + while there are pressed keys, for example: >> + >> + - When a popup is shown by this or other client. >> + - When a drag-and-drop operation is initiated from this or >> + any other surface. >> + >> + In these situations, the pairing key release will not be >> + emitted, clients should undo the effect of those pressed >> + pressed and forget about their state. > > I'd probably avoid specifically mentioning focus switching here. Yup, sounds wise, after all the client doesn't care that the focus goes elsewhere, just about losing the focus itself. > >> @@ -1816,6 +1848,14 @@ >> A key was pressed or released. >> The time argument is a timestamp with millisecond >> granularity, with an undefined base. >> + >> +Clients should note that pressed/released events may not be >> +paired, most commonly due to input being actively taken away >> +or being redirected into this surface (eg. popups), either >> +state must be expected to be received separately. If no such >> +input redirection happened in between, the same surface that >> +received the "pressed" state is expected to receive the >> +"released" state too. >> > > Same comment as with wl_pointer's leave. > >> @@ -1938,6 +1978,25 @@ >> >> >> >> + >> + >> + >> + >> + >> + Notification that this touch point is no longer focused on a >> + certain surface. >> + >> + Note that, because touch points are always implicitly grabbed, >> + this event will only be emitted when the touch point is taken >> + away from this surface through explicit means, for example: >> + >> + - When a popup is shown by this or other client. >> + - When a drag-and-drop operation is initiated from this or >> + any other surface. >> + >> + >> + >> + >> > > This looks fine to me, thoug
Re: [PATCH wayland v2 3/3] tests: add scanner tests
Hi Pekka, > On Nov 16, 2016, at 3:38 AM, Pekka Paalanen wrote: > > From: Pekka Paalanen > > Add tests that ensure that wayland-scanner output for a given input does > not change unexpectedly. This makes it very easy to review > wayland-scanner patches. > > Before, when patches were proposed for wayland-scanner, I had to > build wayland without the patches, save the generated files into a > temporary directory, apply the patches, build again, and diff the old > vs. new generated file. > > No more. Now whenever someone makes intentional changes to > wayland-scanner's output, he will also have to patch the example output > files to match. That means that reviewers see the diff of the generated > files straight from the patch itself. Verifying the diff is true is as > easy as 'make check'. > > The tests use separate example XML files instead of wayland.xml > directly, so that wayland.xml can be updated without fixing scanner > tests, avoiding the churn. > > example.xml starts as a copy of wayland.xml. If wayland.xml starts using > new wayland-scanner features, they should be copied into example.xml > again to be covered by the tests. > > This patch relies on the previous patch to actually add all the data > files for input and reference output. > > The scanner output is fed through sed to remove parts that are allowed > to vary: the scanner version string. > > v2: no need for scanner-test.sh to depend on the test data > > Task: https://phabricator.freedesktop.org/T3313 > Cc: Emilio Pozuelo Monfort > Cc: Yong Bakos > Signed-off-by: Pekka Paalanen This is an awesome addition to the tests for scanner. I modified different aspects of the scanner output and the expected workflow of needing to also change the corresponding tests/data files seems to work well. I could not find any issues with this, so, from where I sit, this is: Reviewed-by: Yong Bakos Tested-by: Yong Bakos Thank you, yong > --- > .gitignore| 1 + > Makefile.am | 29 - > tests/scanner-test.sh | 51 +++ > 3 files changed, 80 insertions(+), 1 deletion(-) > create mode 100755 tests/scanner-test.sh > > diff --git a/.gitignore b/.gitignore > index 33e809c..8da9861 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -38,6 +38,7 @@ ctags > /missing > /stamp-h1 > /test-driver > +/tests/output/ > Makefile > Makefile.in > exec-fd-leak-checker > diff --git a/Makefile.am b/Makefile.am > index d35231c..be372da 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -168,7 +168,15 @@ if ENABLE_CPP_TEST > built_test_programs += cpp-compile-test > endif > > -TESTS = $(built_test_programs) > +AM_TESTS_ENVIRONMENT = > \ > + export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner'\ > + TEST_DATA_DIR='$(top_srcdir)/tests/data'\ > + TEST_OUTPUT_DIR='$(top_builddir)/tests/output' \ > + SED=$(SED) \ > + ; > + > +TESTS = $(built_test_programs) \ > + tests/scanner-test.sh > > check_PROGRAMS = \ > $(built_test_programs) \ > @@ -245,4 +253,23 @@ os_wrappers_test_LDADD = libtest-runner.la > > exec_fd_leak_checker_SOURCES = tests/exec-fd-leak-checker.c > exec_fd_leak_checker_LDADD = libtest-runner.la > + > +EXTRA_DIST += tests/scanner-test.sh \ > + tests/data/example.xml \ > + tests/data/example-client.h \ > + tests/data/example-server.h \ > + tests/data/example-code.c \ > + tests/data/small.xml\ > + tests/data/small-code.c \ > + tests/data/small-client.h \ > + tests/data/small-server.h \ > + tests/data/small-code-core.c\ > + tests/data/small-client-core.h \ > + tests/data/small-server-core.h > + > +tests/scanner-test.sh: $(top_builddir)/wayland-scanner > + > +clean-local: > + -rm -rf tests/output > + > endif #ENABLE_LIBRARIES > diff --git a/tests/scanner-test.sh b/tests/scanner-test.sh > new file mode 100755 > index 000..7854b86 > --- /dev/null > +++ b/tests/scanner-test.sh > @@ -0,0 +1,51 @@ > +#!/bin/sh > + > +echo "srcdir: $srcdir" > +echo "scanner: $WAYLAND_SCANNER" > +echo "test_data_dir: $TEST_DATA_DIR" > +echo "test_output_dir: $TEST_OUTPUT_DIR" > +echo "pwd: $PWD" > +echo "sed: $SED" > + > +RETCODE=0 > + > +hard_fail() { > + echo "$@" "ERROR" > + exit 99 > +} > + > +fail() { > + echo "$@" "FAIL" > + RETCODE=1 > +} > + > +mkdir -p "$TEST_OUTPUT_DIR" || hard_fail "setup" > + > +generate_and_compare() { > + echo > + echo "Testing $1 generation: $2 -> $3" > + > + "$WAYLAND_SCANNER" $1 < "$TEST_DATA
[PATCH wayland] doc: Remove display of WL_PRINTF attribute
From: Yong Bakos Doxygen truncates a WL_PRINTF function attribute, and there does not seem to be any workaround[1]. When using the attribute like this: typedef void (*wl_log_func_t)(const char *, va_list) WL_PRINTF(1, 0); Doxygen generates something that looks like this: typedef void (*wl_log_func_t)(const char *, va_list) WL_PRINTF(1, Configure doxygen to consider WL_PRINTF(x,y) as predefined, so it does not display the attribute at all in the generated documentation. [1] https://bugzilla.gnome.org/show_bug.cgi?id=774741 Signed-off-by: Yong Bakos --- doc/doxygen/wayland.doxygen.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/doxygen/wayland.doxygen.in b/doc/doxygen/wayland.doxygen.in index 9d7fa0c..3913a13 100644 --- a/doc/doxygen/wayland.doxygen.in +++ b/doc/doxygen/wayland.doxygen.in @@ -8,7 +8,8 @@ QUIET = YES HTML_TIMESTAMP = YES GENERATE_LATEX = NO MAN_LINKS = YES -PREDEFINED = WL_EXPORT= +PREDEFINED = WL_EXPORT= \ + WL_PRINTF(x,y)= MACRO_EXPANSION= YES EXPAND_ONLY_PREDEF = YES DOT_MULTI_TARGETS = YES -- 2.7.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons
On Tuesday, November 22, 2016, Daniel Stone wrote: > Hey, > > On 21 November 2016 at 23:13, Peter Hutterer > wrote: > > On Mon, Nov 21, 2016 at 12:42:36PM +, Daniel Stone wrote: > >> Concretely though, reusing BTN_* codes where possible would make it > >> easier for clients to transition between the two. > > > > I disagree here. The kernel only has BTN_STYLUS and BTN_STYLUS2, after > that > > we overlap with DOUBLETAP range and later buttons that are completely > > different (e.g. BTN_GEAR_DOWN). I think this would only make it worse. > > This protocol is still unstable, every client needs updates once we mark > it > > stable anyway, making the enums *values* mean something is > counterproductive > > IMO. > > Shrug, once in an enum they're totally arbitrary values (so which > BTN_* they overlap doesn't make a difference), and it does make it a > little harder to screw it up, as well as easier to stay compatible I see pros and cons for both suggestions. I was into Peter's idea of generic numbering since it is easier to implement and offers some flexibility for client to decide how to translate those events. However, I am kinda convinced by Daniel's point now. If the BTN_ has a preferred default action/feature, kernel should report that information to userland. Client should translate that default setting accordingly. That's just my 2 cents. It's still your call, Peter ;-). Cheers, Ping between multiple versions. But, your call. > > Cheers, > Daniel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] weston-terminal: Fix crash on first keystroke
Since 894b3rcc634 weston-terminal will crash on first keystroke if you fail to create an xkb compose state. This can happen if you don't have a Compose file. Instead, no we just return uncomposed symbols. Signed-off-by: Derek Foreman --- clients/window.c | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/window.c b/clients/window.c index a82f05c..7341aed 100644 --- a/clients/window.c +++ b/clients/window.c @@ -3108,6 +3108,7 @@ static xkb_keysym_t process_key_press(xkb_keysym_t sym, struct input *input) { #ifdef HAVE_XKBCOMMON_COMPOSE + if (!input->xkb.compose_state) return sym; if (sym == XKB_KEY_NoSymbol) return sym; if (xkb_compose_state_feed(input->xkb.compose_state, -- 2.10.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] weston-terminal: Fix crash on first keystroke
Hi, On 22 November 2016 at 19:00, Derek Foreman wrote: > Since 894b3rcc634 weston-terminal will crash on first keystroke if you > fail to create an xkb compose state. This can happen if you don't have > a Compose file. > > Instead, no we just return uncomposed symbols. Obviously correct, apart from the style/indentation, and the typo in the commit message. ;) Both fixed up locally, reviewed and pushed - thanks! To ssh://git.freedesktop.org/git/wayland/weston b8c16c9..2b685d9 push -> master Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] doc: Remove display of WL_PRINTF attribute
On Tue, Nov 22, 2016 at 08:13:43AM -0800, Yong Bakos wrote: > From: Yong Bakos > > Doxygen truncates a WL_PRINTF function attribute, and there does not > seem to be any workaround[1]. When using the attribute like this: > > typedef void (*wl_log_func_t)(const char *, va_list) WL_PRINTF(1, 0); > > Doxygen generates something that looks like this: > > typedef void (*wl_log_func_t)(const char *, va_list) WL_PRINTF(1, > > Configure doxygen to consider WL_PRINTF(x,y) as predefined, so it does > not display the attribute at all in the generated documentation. > > [1] https://bugzilla.gnome.org/show_bug.cgi?id=774741 > > Signed-off-by: Yong Bakos Reviewed-by: Peter Hutterer Cheers, Peter > --- > doc/doxygen/wayland.doxygen.in | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/doc/doxygen/wayland.doxygen.in b/doc/doxygen/wayland.doxygen.in > index 9d7fa0c..3913a13 100644 > --- a/doc/doxygen/wayland.doxygen.in > +++ b/doc/doxygen/wayland.doxygen.in > @@ -8,7 +8,8 @@ QUIET = YES > HTML_TIMESTAMP = YES > GENERATE_LATEX = NO > MAN_LINKS = YES > -PREDEFINED = WL_EXPORT= > +PREDEFINED = WL_EXPORT= \ > + WL_PRINTF(x,y)= > MACRO_EXPANSION= YES > EXPAND_ONLY_PREDEF = YES > DOT_MULTI_TARGETS = YES > -- > 2.7.2 > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons
On Tue, Nov 22, 2016 at 09:03:52AM -0800, Ping Cheng wrote: > On Tuesday, November 22, 2016, Daniel Stone wrote: > > > Hey, > > > > On 21 November 2016 at 23:13, Peter Hutterer > > wrote: > > > On Mon, Nov 21, 2016 at 12:42:36PM +, Daniel Stone wrote: > > >> Concretely though, reusing BTN_* codes where possible would make it > > >> easier for clients to transition between the two. > > > > > > I disagree here. The kernel only has BTN_STYLUS and BTN_STYLUS2, after > > that > > > we overlap with DOUBLETAP range and later buttons that are completely > > > different (e.g. BTN_GEAR_DOWN). I think this would only make it worse. > > > This protocol is still unstable, every client needs updates once we mark > > it > > > stable anyway, making the enums *values* mean something is > > counterproductive > > > IMO. > > > > Shrug, once in an enum they're totally arbitrary values (so which > > BTN_* they overlap doesn't make a difference), and it does make it a > > little harder to screw it up, as well as easier to stay compatible > > > I see pros and cons for both suggestions. I was into Peter's idea of > generic numbering since it is easier to implement and > offers some flexibility for client to decide how to translate those events. > > However, I am kinda convinced by Daniel's point now. If the BTN_ has a > preferred default action/feature, kernel should report that information to > userland. Client should translate that default setting accordingly. I'm not sure I understand your point here. The only change would be that compositors need to have a switch statement to convert from BTN_STYLUS into the wayland enum. Beyond that, no conversation should be done. The benefit Daniel mentions would be that clients don't have to be switched over immediately because the ABI stays the same for BTN_STYLUS(2) and BTN_LEFT-RIGHT. Cheers, Peter > > That's just my 2 cents. It's still your call, Peter ;-). > > Cheers, > Ping > > between multiple versions. But, your call. > > > > Cheers, > > Daniel > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2 wayland-protocols] tablet: define our own enum for tablet tool buttons
On Tuesday, November 22, 2016, Peter Hutterer wrote: > On Tue, Nov 22, 2016 at 09:03:52AM -0800, Ping Cheng wrote: > > On Tuesday, November 22, 2016, Daniel Stone > wrote: > > > > > Hey, > > > > > > On 21 November 2016 at 23:13, Peter Hutterer > > > > wrote: > > > > On Mon, Nov 21, 2016 at 12:42:36PM +, Daniel Stone wrote: > > > >> Concretely though, reusing BTN_* codes where possible would make it > > > >> easier for clients to transition between the two. > > > > > > > > I disagree here. The kernel only has BTN_STYLUS and BTN_STYLUS2, > after > > > that > > > > we overlap with DOUBLETAP range and later buttons that are completely > > > > different (e.g. BTN_GEAR_DOWN). I think this would only make it > worse. > > > > This protocol is still unstable, every client needs updates once we > mark > > > it > > > > stable anyway, making the enums *values* mean something is > > > counterproductive > > > > IMO. > > > > > > Shrug, once in an enum they're totally arbitrary values (so which > > > BTN_* they overlap doesn't make a difference), and it does make it a > > > little harder to screw it up, as well as easier to stay compatible > > > > > > I see pros and cons for both suggestions. I was into Peter's idea of > > generic numbering since it is easier to implement and > > offers some flexibility for client to decide how to translate those > events. > > > > However, I am kinda convinced by Daniel's point now. If the BTN_ has a > > preferred default action/feature, kernel should report that information > to > > userland. Client should translate that default setting accordingly. > > I'm not sure I understand your point here. The only change would be that > compositors need to have a switch statement to convert from BTN_STYLUS > into the wayland enum. Beyond that, no conversation should be done. > > The benefit Daniel mentions would be that clients don't have to be switched > over immediately because the ABI stays the same for BTN_STYLUS(2) and > BTN_LEFT-RIGHT. I guess I meant to make BTN_STYLUS and BTN_STYLUS2 as well as BTN_LEFT/MIDDLE/RIGHT... into the new protocol since they were in input-event-codes.h already. And anything (tbh I don't know if there is anything relied on them) used those event types would not have to change. I'm thinking in terms of the kernel. I am also assuming there are existing stuff relied on those input-event-codes. Plus, Daniel's comments trigger my reply ;). But, I am not familiar with Wayland. Sorry for my ignorance. Cheers, Ping > > Cheers, >Peter > > > > > That's just my 2 cents. It's still your call, Peter ;-). > > > > Cheers, > > Ping > > > > between multiple versions. But, your call. > > > > > > Cheers, > > > Daniel > > > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel