Right mailing list for mutter/gnome-remote-desktop question?

2024-01-17 Thread Matt Hoosier
Hi,

Does anybody know whether there’s a dedicated mailing list suitable for
asking questions about the hardware acceleration in the remote desktop
use-case for those two?

I did a quick look through both repos’ README and CONTRIBUTING files, but
didn’t find anything.


“Rootless” Wayland-on-x11 compositor?

2023-11-02 Thread Matt Hoosier
XWayland gets used in rootless mode, such that each separate X11 window is
exposed as an independent surface to the Wayland compositor.

Does anybody happen to know of a Wayland compositor that acts analogously,
forwarding each client surface to the underlying x11 server as a toplevel
window?


Extension protocols to support keyboard and mouse sharing?

2023-07-20 Thread Matt Hoosier
Hi,

For a while now, I’ve been hoping to see some commercial solutions like
https://symless.com/synergy that implement keyboard and mouse sharing
finally add support for running on DEs that use Wayland.

It seems to be forever on their feature roadmaps but never really getting
closer.

I assume the problem is lack of a good way to snoop on the input events and
(maybe; not sure how these commercial solutions implement it) rewrite or
suppress certain input events when they’re talking to a typical DE
compositor like Mutter.

I had a quick look through the current set of things in wayland-protocols,
and nothing jumped out at me as work in that direction.

Does anybody know of something underway in the upstream compositors that
might not have filtered down to wayland-protocols yet, which would be
useful for securely implementing mouse/keyboard sharing across separate
machines? Maybe I could point these vendors to it.


Re: Refresh rates with multiple monitors

2023-06-15 Thread Matt Hoosier
On Wed, Jun 14, 2023 at 7:13 PM Daniel Stone  wrote:

> Hi Joe,
>
> On Wed, 14 Jun 2023 at 21:33, Joe M  wrote:
>
>> Thanks Daniel. Do you know if wl_output instances are decoupled from each
>> other, when it comes to display refresh?
>>
>
> Yep, absolutely.
>
>
>> The wl_output geometry info hints that each output can be thought of as a
>> region in a larger compositor canvas, given the logical x/y fields in the
>> geometry. Is the compositor able to handle the repaint scheduling in a
>> refresh-aware way?
>>
>
> Yes.
>
>
>> I'm trying to get a better understanding of how these pieces interact to
>> maximize draw time but still hit the glass every frame. The various blog
>> posts and documentation out there are pretty clear when it comes to drawing
>> to a single physical display, but less so when multiple displays are in use.
>>
>
> Per-output repaint cycles are taken as a given. You can assume that every
> compositor does this, and any compositor which doesn't do this is so
> hopelessly broken as to not be worth considering.
>

You can use the wp_presentation extension API to get real-time measurements
about how long elapsed between the moment you submit an updated buffer and
when it hits the glass. If you work backward from that number, you can
figure out how long beforehand to start your drawing so that you can get
minimally stale rendered contends but not drop any frames.


> Cheers,
> Daniel
>


Re: can subsurface and shell surface be used together to manage surfaces

2020-04-11 Thread Matt Hoosier
If you’re asking whether it’s possible to migrate a given wl_surface from
being a subsurface (initially) to later being a shell surface, this isn’t
allowed. This has to do with the immutability of a surface’s so-called
“role.” (Surfaces are only ever allowed to be registered with one given
role. )

-Matt

On Fri, Apr 10, 2020 at 5:43 AM zou lan  wrote:

> Hi pekka & all
>
> I want to use subsurface to manage the initial position of each surface on
> screen, then I want to create shell surface for each wl surface to manage
> them seperately, such as response touch event, ivi-shell can modify the
> z-order of each surface and move each ivi surface to different displays.
>
> Is it ok to use subsurface like this? If the shell surfaces' z-order have
> been modified by desktop shell or ivi-shell,  will the z-order of
> subsurfaces be modified too?
>
> thank you!
>
> Best Regards
> Nancy
> ___
> 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: linux-dmabuf and eglBindWaylandDisplayWl

2020-04-09 Thread Matt Hoosier
On Thu, Apr 9, 2020 at 5:51 PM Simon Ser  wrote:

> On Friday, April 10, 2020 12:48 AM, Matt Hoosier 
> wrote:
>
> > As a compositor author, do I have to account for the possibility that
> the EGL implementation may provide (squat on) the
> “zwp_linux_linux_dmabuf_v1” buffer factory itself, it is that strictly the
> prerogative of the individual compositor?
>
> No, this is the compositor's responsibility only.
>

Thanks, I think that answers the original question pretty clearly.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: linux-dmabuf and eglBindWaylandDisplayWl

2020-04-09 Thread Matt Hoosier
+ list

On Thu, Apr 9, 2020 at 5:48 PM Matt Hoosier  wrote:

> On Thu, Apr 9, 2020 at 5:42 PM Scott Anderson  wrote:
>
>> On 10/04/20 10:34 am, Matt Hoosier wrote:
>> > On Thu, Apr 9, 2020 at 2:51 PM Simon Ser > > <mailto:cont...@emersion.fr>> wrote:
>> >
>> > On Thursday, April 9, 2020 9:41 PM, Matt Hoosier
>> > mailto:matt.hoos...@gmail.com>> wrote:
>> >
>> >  > If I remember correctly, I've read various messages on the list
>> > here saying that eventually it would be preferred for EGL
>> > implementations would just use linux-dmabuf as their buffer factory.
>> > That would mean that EGL backends would now also want to register a
>> > zwp_linux_dmabuf_v1 global.
>> >
>> > Mesa already binds to zwp_linux_dmabuf_v1 and uses it if possible.
>> >
>> >  > I'm wondering how the logistics on that would work. Normally, you
>> > can't register two globals having the same name in the compositor.
>> > Does this lead to a conflict between the EGL implementation's
>> > ability to enumerate a linux-dmabuf API for its own use, and the
>> > linux-dmabuf API that the overall compositor might offer?
>> >
>> > Just to be clear: we're talking about the client side right?
>> >
>> > It's fine to bind twice to the same global. This creates two
>> > independent objects.
>> >
>> >
>> > No, I mean from the standpoint of a compositor implementer.
>> >
>> > -Matt
>>
>> Mesa does not implement zwp_linux_dmabuf_v1 on the compositor side. It's
>> the compositor's job to do that, and they can use EGL or Vulkan
>> extensions [1] to import them for rendering with, or otherwise use them
>> with anything capable of consuming dmabufs.
>>
>> eglBindWaylandDisplayWL still only handles wl_drm.
>>
>> - Scott
>>
>> [1]:
>>
>> https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import.txt
>>
>> https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#VK_EXT_external_memory_dma_buf
>
>
> Okay, interesting that Mesa’s client will use Linux-dmabuf if the
> compositor happens to provide it, but the server-side EGL of Mesa offers
> wl_drm as a fallback.
>
> As a compositor author, do I have to account for the possibility that the
> EGL implementation may provide (squat on) the “zwp_linux_linux_dmabuf_v1”
> buffer factory itself, it is that strictly the prerogative of the
> individual compositor?
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: linux-dmabuf and eglBindWaylandDisplayWl

2020-04-09 Thread Matt Hoosier
On Thu, Apr 9, 2020 at 2:51 PM Simon Ser  wrote:

> On Thursday, April 9, 2020 9:41 PM, Matt Hoosier 
> wrote:
>
> > If I remember correctly, I've read various messages on the list here
> saying that eventually it would be preferred for EGL implementations
> would just use linux-dmabuf as their buffer factory. That would mean that
> EGL backends would now also want to register a zwp_linux_dmabuf_v1 global.
>
> Mesa already binds to zwp_linux_dmabuf_v1 and uses it if possible.
>
> > I'm wondering how the logistics on that would work. Normally, you can't
> register two globals having the same name in the compositor. Does this lead
> to a conflict between the EGL implementation's ability to enumerate a
> linux-dmabuf API for its own use, and the linux-dmabuf API that the overall
> compositor might offer?
>
> Just to be clear: we're talking about the client side right?
>
> It's fine to bind twice to the same global. This creates two
> independent objects.


No, I mean from the standpoint of a compositor implementer.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


linux-dmabuf and eglBindWaylandDisplayWl

2020-04-09 Thread Matt Hoosier
Hi,

Historically, many EGL implementations have registered a private wl_drm
buffer factory for use internally by the implementation's Wayland backend,
as part of the setup work done go eglBindWaylandDisplayWl().

Separately, compositors have started to implement explicit support for the
zwp_linux_dmabuf_v1 buffer factory, independent of the GPU backend.

If I remember correctly, I've read various messages on the list here saying
that eventually it would be preferred for EGL implementations would just
use linux-dmabuf as their buffer factory. That would mean that EGL backends
would now also want to register a zwp_linux_dmabuf_v1 global.

I'm wondering how the logistics on that would work. Normally, you can't
register two globals having the same name in the compositor. Does this lead
to a conflict between the EGL implementation's ability to enumerate a
linux-dmabuf API for its own use, and the linux-dmabuf API that the overall
compositor might offer?

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Aliases for DRI connectors?

2020-04-01 Thread Matt Hoosier
On Wed, Apr 1, 2020 at 3:17 PM Simon Ser  wrote:

> On Wednesday, April 1, 2020 9:38 PM, Matt Hoosier 
> wrote:
>
> > I'm searching for some sort of scheme that will let my DRI master query
> the set of available connectors and select the one carrying a prearranged
> designation. The problem I'm trying to solve is to allow deploying one
> standardized userspace component across a fleet of devices that have
> varying numbers of displays, with the use cases not always driven on the
> same connector topologically.
> >
> > I had a look around the properties available on my system's DRI
> connectors, and nothing immediate jumped out at me. Is there a standard
> connector property that (assuming I can find the right place in DeviceTree
> or similar to) that would be a good fit for this?
>
> Maybe information in the EDID would help? You'll find the manufacturer,
> the model number and the serial number.
>

That might be a possibility. The trouble there is that there can be more
than one instance of the same display model connected to the system, so the
EDID blob wouldn't be particularly helpful to distinguish them. The
connectors in question are generally something like LVDS or DSI that don't
typically expose EDID properties, too.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Aliases for DRI connectors?

2020-04-01 Thread Matt Hoosier
I'm searching for some sort of scheme that will let my DRI master query the
set of available connectors and select the one carrying a prearranged
designation. The problem I'm trying to solve is to allow deploying one
standardized userspace component across a fleet of devices that have
varying numbers of displays, with the use cases not always driven on the
same connector topologically.

I had a look around the properties available on my system's DRI connectors,
and nothing immediate jumped out at me. Is there a standard connector
property that (assuming I can find the right place in DeviceTree or similar
to) that would be a good fit for this?

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: backend-drm and scanning really large resolutions

2020-01-21 Thread Matt Hoosier
On Mon, Jan 20, 2020 at 2:58 AM Pekka Paalanen  wrote:

> On Fri, 17 Jan 2020 10:51:45 -0600
> Matt Hoosier  wrote:
>
> > Hi all,
> >
> > I'm confronting a situation where the hardware with which I work is
> capable
> > of driving connectors at 4K or 8K, but doing so requires bonding the
> > scanning of multiple planes together.
> >
> > The scenario is that you'd have a big primary framebuffer whose size is
> too
> > large for an individual hardware scanning pipeline on the display
> > controller to traverse within its maximum allowed clock rate.
> >
> > The hardware supplier's approach is to assign multiple planes, which in
> the
> > KMS driver map to hardware scanning pipelines, to each be responsible for
> > scanning a smaller section of the framebuffer. The planes are all
> assigned
> > to the same CRTC, and in concert with each other they cover the whole
> area
> > of the framebuffer and CRTC.
> >
> > This sounds a little bit wild to me. I hadn't been aware it's even legal
> to
> > have more than one plane treated a the source of scanout for a single
> > framebuffer. Maybe that distinction isn't really relevant nowadays with
> > universal plane support.
> >
> > I'm wondering if anybody here knows whether this a legit approach for a
> > compositor's DRM backend to take?
>

Hi Pekka; thanks for the reply.


>
> Hi,
>
> I was aware of tiled monitors that need two connectors driven by two
> CRTCs to cover the whole display, but that sounds new to me.
> Libweston/DRM still doesn't support tiled monitors.
>
> What a compositor's DRM-backend can or should do must be generic. It
> cannot be driver or hardware dependent, so handling your case specially
> in userspace would need KMS UAPI to communicate the need in the first
> place. (There is no shared library for "KMS userspace drivers", yet at
> least.)
>
> I am not aware of any KMS UAPI that would indicate the need to use two
> primary planes in a specific configuration for a specific video mode.
> I'm saying two primary planes, because that is the only way I can see
> this situation even hinted at userspace with the current UAPI. I also
> don't know if multiple primary planes is allowed, but it certainly is
> not expected by userspace, so userspace can't make use of it as is.
>

Just to double-check: I think we're still talking here about
universal-plane mode, so we only mean "primary plane" in an informal sense?
This problem would crop up on any attempt to attach a huge framebuffer to a
single plane (whether it happened to be the bottom z-sorted one or a
something used as an overlay).


>
> The idea that comes to my mind is to hide all the details in the
> driver. Expose just one primary plane as usual, and if the video mode
> and FB actually need two scanout units, then steal one under the hood
> in the driver. If that makes another KMS plane (exposed to userspace)
> unusable, that is fine. Userspace with atomic modesetting needs to be
> checking with TEST_ONLY to see if a configuration is possible, and will
> fall back to something else.
>

I think that's about the only approach that would make sense. Who would be
a good person to sanity-check that with? Daniel V? Daniel S?

I suppose in principle that if this is a valid corner-case of the KMS api,
then maybe it wouldn't be wrong to enhance compositors DRM backends to
progressively attempt attaching more and more planes to scan a framebuffer
if the drmModeAtomicCommit(DRM_MODE_ATOMIC_TEST_ONLY) fails for the base
case. But whether anybody in the Weston world would want that patch is
probably another story...


>
> For legacy modesetting I guess you would need to pick between
> supporting the really large video modes vs. exposing all planes. But
> that's a no-brainer, since the legacy API for planes is practically
> unusable. Then again, I don't know if the kernel DRM core allows you to
> make such distinction.
>
> Btw. AFAIK there is nothing wrong with using the exact same FB on
> multiple planes simultaneously.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


backend-drm and scanning really large resolutions

2020-01-17 Thread Matt Hoosier
Hi all,

I'm confronting a situation where the hardware with which I work is capable
of driving connectors at 4K or 8K, but doing so requires bonding the
scanning of multiple planes together.

The scenario is that you'd have a big primary framebuffer whose size is too
large for an individual hardware scanning pipeline on the display
controller to traverse within its maximum allowed clock rate.

The hardware supplier's approach is to assign multiple planes, which in the
KMS driver map to hardware scanning pipelines, to each be responsible for
scanning a smaller section of the framebuffer. The planes are all assigned
to the same CRTC, and in concert with each other they cover the whole area
of the framebuffer and CRTC.

This sounds a little bit wild to me. I hadn't been aware it's even legal to
have more than one plane treated a the source of scanout for a single
framebuffer. Maybe that distinction isn't really relevant nowadays with
universal plane support.

I'm wondering if anybody here knows whether this a legit approach for a
compositor's DRM backend to take?

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Source code to Wayland backend of Android HWComposer?

2019-01-17 Thread Matt Hoosier
Hi all,

A while ago, LWN.net ran an article (https://lwn.net/Articles/702075/) that
overviewed the Wayland-based display architecture used by ChromeOS to allow
its containerized Android runtimes to forward graphics surfaces along to
the ChromeOS compositor.

Does anybody know where to find the sources for that HWComposer
implementation?

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Following all PR's on GitLab

2018-09-26 Thread Matt Hoosier
(Fair warning, I've never used GitLab other than anonymously before.)

I'd like to be able to get email notifications about all the PR's (their
initial posting, subsequent code reviews, acceptance, etc) as a way to
simulate the former bird's eye view that was possible when all patches
passed through wayland-devel as explicit emails.

Is there a way to do this without being part of the core 'wayland' GitLab
group (reserved for the maintainers, I think)? The GitLab UI seems only to
allow configuring notifications on a group-by-group basis. Maybe I'm
missing something though -- I admit to being a novice user of GitLab's web
UI.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Favorite external display/touch combo panels?

2018-08-14 Thread Matt Hoosier
Hi;

I'm interested in getting an external display to make testing the
touch-centric changes in Weston easier when working from a PC. Does anybody
have recommendations for a good model that attaches over HDMI+USB that
supports a reasonably good resolution and reports a standard slotted
multitouch event source under Linux/libinput?

In the past, I was mostly happy with Lilliput's earlier line of 10-inch
external displays that had resistive touch panels, but they're pretty well
aged past usefulness now.

Cheers,
Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Users of libweston?

2018-07-02 Thread Matt Hoosier
On Mon, Jul 2, 2018 at 4:10 AM Pekka Paalanen  wrote:

> On Fri, 29 Jun 2018 10:05:58 -0500
> Matt Hoosier  wrote:
>
> > Hi all,
> >
> > Pekka's recent comments about wanting to enable set-top boxes built with
> > libweston to do DRM content got me to wondering:
> >
> > Who all is using libweston directly (as opposed to running
> /usr/bin/weston
> > possibly with custom shells plugins or similar)? For my own purposes, I
> > just use the full compositor because it's pretty lean and mean anyway,
> and
> > I can do what I need by loading plugins.
>
> Hi Matt,
>
> I wouldn't be surprised if there weren't many users yet. There is a
> huge amount of things I'd like to do before I could comfortably propose
> using libweston. I still think it needs to be a goal in mind all the
> time though, otherwise we'll never get there. :-)
>
> IMO the major point of using libweston instead of weston is to be able
> to customize the UX any way you want - all the stuff and policy that is
> currecntly hardcoded in main.c and the desktop-shell plugin. Making all
> configurable is probably not feasible.
>
> My hope with gaining set-top box etc. use cases is to gather more people
> developing for Weston, people who could be dedicated in the long run.
> Maybe that could gain us more upstream maintainers.
>
>
> Thanks,
> pq
>

Hi --

Yeah, that all makes sense. I certainly didn't mean to imply any criticism
with the question. I wasn't yet following the conversations on
wayland-devel when Weston got overhauled to split out its core compositor
features as libweston, so I wasn't positive exactly who the intended users
are. I suppose that maybe there's little chance that Mutter or Kwin would
ever adopt this (although that would be an amazing proof of generality).

The question mainly came from my observation that the modularity of the
plugin system in Weston seems _so effective_ that it almost completely
subsumes the need for using libweston. I run several out-of-tree plugins
(one of them providing an alternative to desktop-shell, and a few others
doing random runtime things such as integrating with systemd  -- yes, I
know there's an official Weston plugin for that ;) ), and the
/usr/bin/weston entrypoint still seems to hold up very well for all this.

The bit about being able to work around policy choices made in main.c does
make sense.

On the topic of modularity: what was the reason why the libweston overhaul
nixed the ability for out-of-tree plugins to contribute new backends? (This
is just a curiosity question -- I'm don't have any particular opposition.)

Thanks!
Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Users of libweston?

2018-06-29 Thread Matt Hoosier
Hi all,

Pekka's recent comments about wanting to enable set-top boxes built with
libweston to do DRM content got me to wondering:

Who all is using libweston directly (as opposed to running /usr/bin/weston
possibly with custom shells plugins or similar)? For my own purposes, I
just use the full compositor because it's pretty lean and mean anyway, and
I can do what I need by loading plugins.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Wayland content-protection extension

2018-06-18 Thread Matt Hoosier
On Mon, Jun 18, 2018 at 8:54 AM Pekka Paalanen  wrote:

> Hi Matt,
>
> did you intend to reply on list? Please CC if you did.
>
> On Mon, 18 Jun 2018 08:35:56 -0500
> Matt Hoosier  wrote:
>
> > On Mon, Jun 18, 2018 at 3:59 AM Pekka Paalanen 
> wrote:
> >
> > > On Mon, 18 Jun 2018 13:38:09 +0530
> > > Ramalingam C  wrote:
> > >
> > > > On Monday 18 June 2018 01:34 PM, Pekka Paalanen wrote:
> > > > > On Sat, 16 Jun 2018 12:50:52 +0530
> > > > > Ramalingam C  wrote:
> > > > >
> > >
> > > > > How does the kernel signal to userspace that the HDCP status has
> > > > > changed? Do you piggyback on the hotplug event?
> > > > >
> > > > > Anything that would require userspace to repeatedly re-read
> properties
> > > > > without any events triggering it is bad design. If nothing is
> > > > > happening, the compositor needs to be able to stay asleep.
> > > > Pekka,
> > > >
> > > > We proposed a uevent from kernel for indicating the HDCP status
> change.
> > > > But that didn't fly. Right now the merged interface expects
> compositor
> > > > to poll
> > > > the property state for the runtime failures.
> > >
> > > Ugh. :-(
> > >
> >
> > I get what you mean here, but maybe it's not actually that bad. The HDCP
> > runtime failure polling would really only be needed during times when at
> > least one video stream is actually using it, right? If that's true, then
> > the compositor is regularly waking up as the clients submit successive
> > buffers anyway. Can the HDCP connector status polling get folded into
> that
> > wakeup cycle?
>
> Sure, but you are assuming the protected content is video. I'm thinking
> of still images.
>

Yeah, that's a fair point. Still probably covers the dominant use-case
though? Just curious, are you referring to the still images that result
from pausing a video stream, or first-class static images (maybe photos or
something) covered by content protection too?


>
> I'd ask for more details on why the kernel community thinks that there
> must not be an event to signal HDCP state changes, I suspect it's a
> problem with the implementation and not the idea, but I won't have time
> to go there anyway.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] log: improve handling of use-before-init

2018-05-25 Thread Matt Hoosier
Any takers to accept this one?

On Fri, May 4, 2018 at 9:26 AM, Matt Hoosier  wrote:

> Rather than segfaulting by attempting to traverse an initially
> null log handler pointer, explicitly print a message and abort.
>
> Signed-off-by: Matt Hoosier 
> ---
>  libweston/log.c | 22 --
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/libweston/log.c b/libweston/log.c
> index 7d99a95d..c21f25e9 100644
> --- a/libweston/log.c
> +++ b/libweston/log.c
> @@ -36,8 +36,26 @@
>
>  #include "compositor.h"
>
> -static log_func_t log_handler = 0;
> -static log_func_t log_continue_handler = 0;
> +static int default_log_handler(const char *fmt, va_list ap);
> +
> +static log_func_t log_handler = default_log_handler;
> +static log_func_t log_continue_handler = default_log_handler;
> +
> +/** Sentinel log message handler
> + *
> + * This function is used as the default handler for log messages. It
> + * exists only to issue a noisy reminder to the user that a real handler
> + * must be installed prior to issuing logging calls. The process is
> + * immediately aborted after the reminder is printed.
> + *
> + * \param fmt The format string. Ignored.
> + * \param va The variadic argument list. Ignored.
> + */
> +static int default_log_handler(const char *fmt, va_list ap)
> +{
> +fprintf(stderr, "weston_log_set_handler() must be called before
> using of weston_log().\n");
> +abort();
> +}
>
>  /** Install the log handler
>   *
> --
> 2.13.6
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Matt Hoosier
On Fri, May 4, 2018 at 8:46 AM, Pekka Paalanen  wrote:

> On Fri, 4 May 2018 07:55:10 -0500
> Matt Hoosier  wrote:
>
> > On Fri, May 4, 2018 at 2:13 AM, Pekka Paalanen 
> wrote:
> >
> > > The crucial call to make is weston_log_set_handler() with non-NULL
> > > arguments before any other libweston calls.
> > >
> >
> > I wonder if this could be made clearer by adding an assert in
> weston_log()
> > if no handler has been installed yet.
>
> Definitely. :-)
> A shout to stderr and abort() would be fine. Or even have the handlers
> initilized to ones that shout and abort().
>
>
> Thanks,
> pq
>


Done and published on the list for comments.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] log: improve handling of use-before-init

2018-05-04 Thread Matt Hoosier
Rather than segfaulting by attempting to traverse an initially
null log handler pointer, explicitly print a message and abort.

Signed-off-by: Matt Hoosier 
---
 libweston/log.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/libweston/log.c b/libweston/log.c
index 7d99a95d..c21f25e9 100644
--- a/libweston/log.c
+++ b/libweston/log.c
@@ -36,8 +36,26 @@
 
 #include "compositor.h"
 
-static log_func_t log_handler = 0;
-static log_func_t log_continue_handler = 0;
+static int default_log_handler(const char *fmt, va_list ap);
+
+static log_func_t log_handler = default_log_handler;
+static log_func_t log_continue_handler = default_log_handler;
+
+/** Sentinel log message handler
+ *
+ * This function is used as the default handler for log messages. It
+ * exists only to issue a noisy reminder to the user that a real handler
+ * must be installed prior to issuing logging calls. The process is
+ * immediately aborted after the reminder is printed.
+ *
+ * \param fmt The format string. Ignored.
+ * \param va The variadic argument list. Ignored.
+ */
+static int default_log_handler(const char *fmt, va_list ap)
+{
+fprintf(stderr, "weston_log_set_handler() must be called before using 
of weston_log().\n");
+abort();
+}
 
 /** Install the log handler
  *
-- 
2.13.6

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: libweston program segfaults on weston_compositor_load_backend

2018-05-04 Thread Matt Hoosier
On Fri, May 4, 2018 at 2:13 AM, Pekka Paalanen  wrote:

> On Thu, 03 May 2018 23:38:55 +0100
> adlo  wrote:
>
> > On Thu, 2018-05-03 at 14:12 +0300, Pekka Paalanen wrote:
> > >
> > > what's the backtrace from gdb for that?
> > >
> >
> > Is this any help?
> >
> > #0  0x in  ()
> > #1  0x7799c88c in weston_log (fmt=fmt@entry=0x779b6a7e
> > "Loading module '%s'\n") at libweston/log.c:75
> > #2  0x779a603c in weston_load_module (name=name@entry=0x779
> > b6b24 "wayland-backend.so", entrypoint=entrypoint@entry=0x779b6aae
> > "weston_backend_init") at libweston/compositor.c:6375
> > #3  0x779a6264 in weston_compositor_load_backend
> > (compositor=0x611090, backend=backend@entry=
> > WESTON_BACKEND_WAYLAND, config_base=config_base@entry=0x7fffdc5
> > 0)
> > at libweston/compositor.c:6485
> > #4  0x00400704 in main (argc=, argv= > out>)
> > at ../main.c:42
>
> Is that perhaps the very first call to weston_log()?
>
> You need to initialize the logging mechanism before anything can call
> weston_log(). Yeah, it's awkward, could probably be improved, but OTOH
> one would always want all log messages going to the same place, so you
> really do want to initialize the logging before any logging happens to
> avoid losing messages.
>
> The crucial call to make is weston_log_set_handler() with non-NULL
> arguments before any other libweston calls.
>

I wonder if this could be made clearer by adding an assert in weston_log()
if no handler has been installed yet.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH libinput 0/1] Initialization race against udev

2018-04-24 Thread Matt Hoosier
On Sun, Apr 22, 2018 at 11:01 PM, Peter Hutterer 
wrote:

> On Sun, Apr 22, 2018 at 06:07:36PM +, Friedrich, Eugen (ADITG/ESB)
> wrote:
> > Hi, sorry for late replay
> >
> > Best regards
> >
> > Eugen Friedrich
> > Engineering Software Base (ADITG/ESB)
> >
> > Tel. +49 5121 49 6921
> > > -Original Message-
> > > From: Peter Hutterer [mailto:peter.hutte...@who-t.net]
> > > Sent: Freitag, 6. April 2018 04:29
> > > To: Friedrich, Eugen (ADITG/ESB)
> > > Cc: Matt Hoosier; Pekka Paalanen; wayland mailing list
> > > Subject: Re: [PATCH libinput 0/1] Initialization race against udev
> > >
> > > On Thu, Apr 05, 2018 at 01:20:51PM +, Friedrich, Eugen (ADITG/ESB)
> wrote:
> > > > Hi Peter
> > > >
> > > > Best regards
> > > >
> > > > Eugen Friedrich
> > > > Engineering Software Base (ADITG/ESB)
> > > >
> > > > Tel. +49 5121 49 6921
> > > >
> > > > > -Original Message-
> > > > > From: wayland-devel [mailto:wayland-devel-
> > > > > boun...@lists.freedesktop.org] On Behalf Of Peter Hutterer
> > > > > Sent: Donnerstag, 5. April 2018 06:27
> > > > > To: Matt Hoosier
> > > > > Cc: Pekka Paalanen; wayland mailing list
> > > > > Subject: Re: [PATCH libinput 0/1] Initialization race against udev
> > > > >
> > > > > On Wed, Apr 04, 2018 at 09:04:32AM -0500, Matt Hoosier wrote:
> > > > > > I encounter another "bad" behavior related to multiple firings
> of the UDev
> > > > > > 'add' event for a given input node.
> > > > > >
> > > > > > Typically on modest embedded systems, you do not want to run the
> > > > > exhaustive
> > > > > > 'udevadm trigger' during the main booting sequence. That causes
> hundreds
> > > > > or
> > > > > > thousands of UDev nodes to be crawled and processed by udevd all
> during
> > > > > the
> > > > > > first moments of userspace. These overall bulk of these nodes
> are pretty
> > > > > > much irrelevant to the use-case of the device. The time spent
> processing
> > > > > > these non-essential /sys devices can easily slow down the
> device's
> > > > > progress
> > > > > > toward starting a UI with basic touch support and loading
> drivers for the
> > > > > > handful of essential peripherals by 10 or 15 seconds.
> > > > > >
> > > > > > Instead, it works better to manually command the same UDev
> coldplugging
> > > > > > that would have been done by 'udevadm trigger', but for a very
> small
> > > > > > hand-picked set of devices rather than everything in
> /sys/devices. The time
> > > > > > savings are large. Of course, for completeness you do eventually
> have to
> > > > > > run 'udevadm trigger' so that the full set of hardware and
> kernel software
> > > > > > features are activated, but that can wait until after the main
> KPIs are
> > > > > > achieved.
> > > > > >
> > > > > > This scheme generally works just fine. Manually stimulating
> udevd to
> > > > > > coldplug the specific devices you need keeps everything general:
> > > > > > applications that find their hardware with UDev (such as with
> libinput or
> > > > > > Weston's DRM backend) all get to rely on their usual well-tested
> > > > > codepaths.
> > > > > >
> > > > > > But there is a snag: if a device like /dev/input/event0 has been
> > > > > > coldplugged once with the hands-on technique, then all the
> daemons that
> > > > > > care about it have already seen one UDev ACTION=add event for
> it. When
> > > > > the
> > > > > > late-running 'udevadm trigger' does its exhaustive sweep across
> > > > > > /sys/devices, this will cause a second ACTION=add event to be
> triggered
> > > > > for
> > > > > > /dev/input/event0. Currently (well, with libinput 1.1.1) this
> causes
> > > > > > libinput -- and consequently Weston -- to open a second
> filedescriptor
> > > > > > against /dev/input/event0, so that all input events are received
> in
>

Re: [PATCH libinput 0/1] Initialization race against udev

2018-04-06 Thread Matt Hoosier
On Thu, Apr 5, 2018 at 9:28 PM, Peter Hutterer 
wrote:

> On Thu, Apr 05, 2018 at 01:20:51PM +, Friedrich, Eugen (ADITG/ESB)
> wrote:
> > Hi Peter
> >
> > Best regards
> >
> > Eugen Friedrich
> > Engineering Software Base (ADITG/ESB)
> >
> > Tel. +49 5121 49 6921
> >
> > > -Original Message-
> > > From: wayland-devel [mailto:wayland-devel-
> > > boun...@lists.freedesktop.org] On Behalf Of Peter Hutterer
> > > Sent: Donnerstag, 5. April 2018 06:27
> > > To: Matt Hoosier
> > > Cc: Pekka Paalanen; wayland mailing list
> > > Subject: Re: [PATCH libinput 0/1] Initialization race against udev
> > >
> > > On Wed, Apr 04, 2018 at 09:04:32AM -0500, Matt Hoosier wrote:
> > > > I encounter another "bad" behavior related to multiple firings of
> the UDev
> > > > 'add' event for a given input node.
> > > >
> > > > Typically on modest embedded systems, you do not want to run the
> > > exhaustive
> > > > 'udevadm trigger' during the main booting sequence. That causes
> hundreds
> > > or
> > > > thousands of UDev nodes to be crawled and processed by udevd all
> during
> > > the
> > > > first moments of userspace. These overall bulk of these nodes are
> pretty
> > > > much irrelevant to the use-case of the device. The time spent
> processing
> > > > these non-essential /sys devices can easily slow down the device's
> > > progress
> > > > toward starting a UI with basic touch support and loading drivers
> for the
> > > > handful of essential peripherals by 10 or 15 seconds.
> > > >
> > > > Instead, it works better to manually command the same UDev
> coldplugging
> > > > that would have been done by 'udevadm trigger', but for a very small
> > > > hand-picked set of devices rather than everything in /sys/devices.
> The time
> > > > savings are large. Of course, for completeness you do eventually
> have to
> > > > run 'udevadm trigger' so that the full set of hardware and kernel
> software
> > > > features are activated, but that can wait until after the main KPIs
> are
> > > > achieved.
> > > >
> > > > This scheme generally works just fine. Manually stimulating udevd to
> > > > coldplug the specific devices you need keeps everything general:
> > > > applications that find their hardware with UDev (such as with
> libinput or
> > > > Weston's DRM backend) all get to rely on their usual well-tested
> > > codepaths.
> > > >
> > > > But there is a snag: if a device like /dev/input/event0 has been
> > > > coldplugged once with the hands-on technique, then all the daemons
> that
> > > > care about it have already seen one UDev ACTION=add event for it.
> When
> > > the
> > > > late-running 'udevadm trigger' does its exhaustive sweep across
> > > > /sys/devices, this will cause a second ACTION=add event to be
> triggered
> > > for
> > > > /dev/input/event0. Currently (well, with libinput 1.1.1) this causes
> > > > libinput -- and consequently Weston -- to open a second
> filedescriptor
> > > > against /dev/input/event0, so that all input events are received in
> > > > duplicate. That confuses the compositor's and applications' input
> event
> > > > handling.
> > > >
> > > > Would it be tolerable to put a patch into either libinput or Weston
> to
> > > > guard against double-opening the same input device? I realize that
> the
> > > > scheme outlined above is probably technically in violation of the
> expected
> > > > UDev initialization scheme, but I'm not aware of any way to suppress
> > > udevd
> > > > from broadcasting the 'add' action to all udev clients even though
> the
> > > > device has already been coldplugged once. It seems to me at least
> plausible
> > > > that the Weston stack would benefit from guarding against getting
> into this
> > > > bad state from receiving unexpected UDev events.
> > >
> > >
> > > I don't think a weston patch is likely to help you here since
> libinput's
> > > udev handling is independent of weston. but tbh, I'd rather not do
> this in
> > > libinput either. as you write, you're not really behaving like udev is
> > > 

Re: [PATCH libinput 0/1] Initialization race against udev

2018-04-04 Thread Matt Hoosier
I encounter another "bad" behavior related to multiple firings of the UDev
'add' event for a given input node.

Typically on modest embedded systems, you do not want to run the exhaustive
'udevadm trigger' during the main booting sequence. That causes hundreds or
thousands of UDev nodes to be crawled and processed by udevd all during the
first moments of userspace. These overall bulk of these nodes are pretty
much irrelevant to the use-case of the device. The time spent processing
these non-essential /sys devices can easily slow down the device's progress
toward starting a UI with basic touch support and loading drivers for the
handful of essential peripherals by 10 or 15 seconds.

Instead, it works better to manually command the same UDev coldplugging
that would have been done by 'udevadm trigger', but for a very small
hand-picked set of devices rather than everything in /sys/devices. The time
savings are large. Of course, for completeness you do eventually have to
run 'udevadm trigger' so that the full set of hardware and kernel software
features are activated, but that can wait until after the main KPIs are
achieved.

This scheme generally works just fine. Manually stimulating udevd to
coldplug the specific devices you need keeps everything general:
applications that find their hardware with UDev (such as with libinput or
Weston's DRM backend) all get to rely on their usual well-tested codepaths.

But there is a snag: if a device like /dev/input/event0 has been
coldplugged once with the hands-on technique, then all the daemons that
care about it have already seen one UDev ACTION=add event for it. When the
late-running 'udevadm trigger' does its exhaustive sweep across
/sys/devices, this will cause a second ACTION=add event to be triggered for
/dev/input/event0. Currently (well, with libinput 1.1.1) this causes
libinput -- and consequently Weston -- to open a second filedescriptor
against /dev/input/event0, so that all input events are received in
duplicate. That confuses the compositor's and applications' input event
handling.

Would it be tolerable to put a patch into either libinput or Weston to
guard against double-opening the same input device? I realize that the
scheme outlined above is probably technically in violation of the expected
UDev initialization scheme, but I'm not aware of any way to suppress udevd
from broadcasting the 'add' action to all udev clients even though the
device has already been coldplugged once. It seems to me at least plausible
that the Weston stack would benefit from guarding against getting into this
bad state from receiving unexpected UDev events.

-Matt

On Wed, Apr 4, 2018 at 6:51 AM, Pekka Paalanen  wrote:

> From: Pekka Paalanen 
>
> Hi,
>
> here is a patch for a race that was troubling a distribution for an
> embedded device. The original form of this patch has been in developer
> use for months now, and it seems to work there. For this version I have
> only changed the logging and fixed a leak.
>
> I originally discussed this issue on #wayland with Peter Hutterer on
> September 22, 2017. The discussion is quoted below to refresh our
> memories.
>
> In the distribution, Weston is started by systemd, the system is not
> very CPU-powerful, and lots of things are happening during the boot,
> which may all contribute to making this race possible to lose.
>
> I've only tested briefly on my work desktop to see that Weston still
> appears to find the input devices I expect. Obviously my desktop would
> never lose the race, because there are no input devices being hotplugged
> at the same time as Weston is starting up.
>
> The timer/idle callback idea is not implemented here, and neither is the
> double-add filtering. Let me know if you require those.
>
>
> Thanks,
> pq
>
>
> < pq> whot, btw. we've looking into some fun libinput vs. udev
> device initialization race here. Apparently the device enumeration
> on libinput start-up may race with udev preparing devices at the
> same time, so the initial device enumeration for weston may see
> devices that have not had all their udev properties set yet. Do you
> recall ever fighting such issues?
>
> < pq> whot, another issue is that we may see a double-add of a
> device, first from the initial enumeration (with possibly missing
> properties) and then a second time as a hotplug event because
> libinput (correctly) listens for events before it enume existing
> devices.
>
> < whot> pq: yes, I've seen this a lot with the test suite, but never
> in real life
>
> < whot> I'm pretty sure in my case it's always a lingering udev
> device or some lingering properties
>
> < whot> e.g. a tablet has some keyboard properties set because the
> kernel re-uses the event node
>
> < whot> but that's triggered by the test suite using the path
> interface and there's bound to be a window where we can race. it
> shouldn't happen with the udev device, I think
>
> < pq> specifically, in the target device/system with touchscreens
> and weston, there are

Re: [PATCH weston 00/25] A new touchscreen calibrator

2018-04-04 Thread Matt Hoosier
On Wed, Apr 4, 2018 at 2:27 AM, Pekka Paalanen  wrote:

> On Wed, 04 Apr 2018 00:37:58 +
> Matt Hoosier  wrote:
>
> > On Tue, Apr 3, 2018, 08:10 Matt Hoosier  wrote:
> >
> > > On Tue, Apr 3, 2018 at 5:35 AM, Philipp Kerling 
> > > wrote:
> > >
> > >> > Do you happen to know of a good readymade program that fakes the
> > >> > presence of a touch input device with uinput? I'd love to test this
> > >> > series, but current Weston is far ahead of what my embedded devices
> > >> > will do; so I'm in the position of mostly relying on the desktop for
> > >> > testing.
> > >> I'm not sure whether it fits your use case, but you can give mtemu a
> > >> spin.
> > >>
> > > https://gitlab.com/shul/mtemu
> > >
> > >
> > > Thanks. That looks like a really useful program. As it turns out, I
> ended
> > > up being able to find some real touch hardware to use. It was ancient
> stuff
> > > that required writing a uinput program to translate the old
> ABS_X/Y-style
> > > input into modern mtdev representation, but it eventually worked.
> > >
> > > The calibration functionality in Pekka's patch series seems to work for
> > > me. I didn't test out any of the mechanism to spawn an auxiliary
> program
> > > that saves the results to disk, but the hot-installed copy of the
> > > calibration worked.
> > >
> > > I did have a little trouble understanding how I can pick-and-choose
> from
> > > the client side which touch device should be associated with which
> output.
> > > The new weston-touch-calibrator program just prints out a flat list of
> > > input devices and head names, but doesn't seem to let you do
> permutations
> > > of that.
> > >
> > > So I ended up having to use a 'mode=off' directive in weston.ini to
> > > temporarily turn off the main display of my laptop so that the
> touchscreen
> > > got pegged to the correct physical output. I have the feeling that I
> just
> > > overlooked some aspect of configuration about this.
> > >
> >
> > ... which is of course the statically set UDev properties that associate
> an
> > input device with the Wayland output. Nevermind about this question.
>
> Indeed. There is no runtime way to change the touchscreen/output
> associations in Weston, it picks it up from the udev properties on the
> touch device. Other DEs may have their ways, but I considered it out of
> scope for this protocol extension.
>
> Thank you for testing! I wonder if I can out your Tested-by tag in all
> the patches, or maybe some specific patch... like the "clients: add a
> new touchscreen calibrator"?
>

Sure. Let's say that all the patches in this series except "doc: add
example calibration-helper script" are:

Tested-by: Matt Hoosier 

You're welcome to apply the my T-B to the final patch too, but since it's
only documentation, I'm not sure the testing is very relevant there.


>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 00/25] A new touchscreen calibrator

2018-04-03 Thread Matt Hoosier
On Tue, Apr 3, 2018, 08:10 Matt Hoosier  wrote:

> On Tue, Apr 3, 2018 at 5:35 AM, Philipp Kerling 
> wrote:
>
>> > Do you happen to know of a good readymade program that fakes the
>> > presence of a touch input device with uinput? I'd love to test this
>> > series, but current Weston is far ahead of what my embedded devices
>> > will do; so I'm in the position of mostly relying on the desktop for
>> > testing.
>> I'm not sure whether it fits your use case, but you can give mtemu a
>> spin.
>>
> https://gitlab.com/shul/mtemu
>
>
> Thanks. That looks like a really useful program. As it turns out, I ended
> up being able to find some real touch hardware to use. It was ancient stuff
> that required writing a uinput program to translate the old ABS_X/Y-style
> input into modern mtdev representation, but it eventually worked.
>
> The calibration functionality in Pekka's patch series seems to work for
> me. I didn't test out any of the mechanism to spawn an auxiliary program
> that saves the results to disk, but the hot-installed copy of the
> calibration worked.
>
> I did have a little trouble understanding how I can pick-and-choose from
> the client side which touch device should be associated with which output.
> The new weston-touch-calibrator program just prints out a flat list of
> input devices and head names, but doesn't seem to let you do permutations
> of that.
>
> So I ended up having to use a 'mode=off' directive in weston.ini to
> temporarily turn off the main display of my laptop so that the touchscreen
> got pegged to the correct physical output. I have the feeling that I just
> overlooked some aspect of configuration about this.
>

... which is of course the statically set UDev properties that associate an
input device with the Wayland output. Nevermind about this question.


> -Matt
>
>
>>
>>
>>
>> > On Fri, Mar 23, 2018, 09:38 Matt Hoosier 
>> > wrote:
>> > > On Fri, Mar 23, 2018 at 9:30 AM, Pekka Paalanen > > > m> wrote:
>> > > > On Fri, 23 Mar 2018 08:46:46 -0500
>> > > > Matt Hoosier  wrote:
>> > > >
>> > > >> I am very much in favor of the overall approach on this patch
>> > > series.
>> > > >> I've experienced every single one of the problems described in
>> > > this
>> > > >> summary, and my company currently resorts to maintaining a hacky
>> > > >> out-of-tree calibration tool to paper over these problems.
>> > > >
>> > > > Hi Matt,
>> > > >
>> > > > that is very heartwarming to hear. Is your tool specifically for
>> > > Weston
>> > > > too?
>> > >
>> > > Yes and no. It's not phrased as a patch against the Weston source
>> > > code, but it uses heuristics for determining which output the raw
>> > > /dev/input/* events should be correlated against, and those
>> > > heuristics
>> > > probably would fail if some different compositor happened to be
>> > > running.
>> > >
>> > > >
>> > > > I would be very happy if this proposal fits your needs, and
>> > > certainly
>> > > > interested in hearing where it falls short.
>> > > >
>> > > >
>> > > > Thanks,
>> > > > pq
>> > > >
>> > > >> On Fri, Mar 23, 2018 at 7:00 AM, Pekka Paalanen > > > .com> wrote:
>> > > >> > From: Pekka Paalanen 
>> > > >> >
>> > > >> > Hi all,
>> > > >> >
>> > > >> > the existing touchscreen calibrator in Weston has several
>> > > problems. This
>> > > >> > proposal intends to solve them all by introducing a new
>> > > protocol
>> > > >> > extension for touchscreen calibration and a new calibrator
>> > > tool.
>> > > >> >
>> > > >> > The benefits of the new tool, which the old tool lacks, are:
>> > > >> >
>> > > >> > - You can unambiguously pick a physical touch device to
>> > > calibrate.
>> > > >> >
>> > > >> > - You can be sure your touch events come only from that
>> > > particular
>> > > >> >   device, and that you cannot miss touch events even if the
>> > > current
>> > > >> >   calibration is horri

Re: [PATCH weston 00/25] A new touchscreen calibrator

2018-04-03 Thread Matt Hoosier
On Tue, Apr 3, 2018 at 5:35 AM, Philipp Kerling  wrote:

> > Do you happen to know of a good readymade program that fakes the
> > presence of a touch input device with uinput? I'd love to test this
> > series, but current Weston is far ahead of what my embedded devices
> > will do; so I'm in the position of mostly relying on the desktop for
> > testing.
> I'm not sure whether it fits your use case, but you can give mtemu a
> spin.
>
https://gitlab.com/shul/mtemu


Thanks. That looks like a really useful program. As it turns out, I ended
up being able to find some real touch hardware to use. It was ancient stuff
that required writing a uinput program to translate the old ABS_X/Y-style
input into modern mtdev representation, but it eventually worked.

The calibration functionality in Pekka's patch series seems to work for me.
I didn't test out any of the mechanism to spawn an auxiliary program that
saves the results to disk, but the hot-installed copy of the calibration
worked.

I did have a little trouble understanding how I can pick-and-choose from
the client side which touch device should be associated with which output.
The new weston-touch-calibrator program just prints out a flat list of
input devices and head names, but doesn't seem to let you do permutations
of that.

So I ended up having to use a 'mode=off' directive in weston.ini to
temporarily turn off the main display of my laptop so that the touchscreen
got pegged to the correct physical output. I have the feeling that I just
overlooked some aspect of configuration about this.

-Matt


>
>
>
> > On Fri, Mar 23, 2018, 09:38 Matt Hoosier 
> > wrote:
> > > On Fri, Mar 23, 2018 at 9:30 AM, Pekka Paalanen  > > m> wrote:
> > > > On Fri, 23 Mar 2018 08:46:46 -0500
> > > > Matt Hoosier  wrote:
> > > >
> > > >> I am very much in favor of the overall approach on this patch
> > > series.
> > > >> I've experienced every single one of the problems described in
> > > this
> > > >> summary, and my company currently resorts to maintaining a hacky
> > > >> out-of-tree calibration tool to paper over these problems.
> > > >
> > > > Hi Matt,
> > > >
> > > > that is very heartwarming to hear. Is your tool specifically for
> > > Weston
> > > > too?
> > >
> > > Yes and no. It's not phrased as a patch against the Weston source
> > > code, but it uses heuristics for determining which output the raw
> > > /dev/input/* events should be correlated against, and those
> > > heuristics
> > > probably would fail if some different compositor happened to be
> > > running.
> > >
> > > >
> > > > I would be very happy if this proposal fits your needs, and
> > > certainly
> > > > interested in hearing where it falls short.
> > > >
> > > >
> > > > Thanks,
> > > > pq
> > > >
> > > >> On Fri, Mar 23, 2018 at 7:00 AM, Pekka Paalanen  > > .com> wrote:
> > > >> > From: Pekka Paalanen 
> > > >> >
> > > >> > Hi all,
> > > >> >
> > > >> > the existing touchscreen calibrator in Weston has several
> > > problems. This
> > > >> > proposal intends to solve them all by introducing a new
> > > protocol
> > > >> > extension for touchscreen calibration and a new calibrator
> > > tool.
> > > >> >
> > > >> > The benefits of the new tool, which the old tool lacks, are:
> > > >> >
> > > >> > - You can unambiguously pick a physical touch device to
> > > calibrate.
> > > >> >
> > > >> > - You can be sure your touch events come only from that
> > > particular
> > > >> >   device, and that you cannot miss touch events even if the
> > > current
> > > >> >   calibration is horribly wrong.
> > > >> >
> > > >> > - You can be sure the calibration window (pattern) is shown on
> > > the right
> > > >> >   output with the right coordinates.
> > > >> >
> > > >> > - You can unambiguously calibrate even multiple touchscreens
> > > that are
> > > >> >   all cloned (showing the same image).
> > > >> >
> > > >> > - You get a libinput style calibation matrix instead of the
> > > >> >   WL_CALIBRATION format which depends on output resolution.
> > > >> >
> > > >> > - You can load a new calibration into the compositor without
> > > playing
> > > >> >   tricks with udev or restarting the compositor.
> > > >> >
> > > >> > There is more discussion about the topic at:
> > > >> > https://phabricator.freedesktop.org/T7868
> > > >> >
> > > >> > This patch series depends on the clone mode series:
> > > >> > https://patchwork.freedesktop.org/series/32898/
> > > >> >
> > > >> > There is a full branch available at:
> > > >> > https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
> > > >
> >
> > ___
> > 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: [PATCH weston 00/25] A new touchscreen calibrator

2018-04-02 Thread Matt Hoosier
Do you happen to know of a good readymade program that fakes the presence
of a touch input device with uinput? I'd love to test this series, but
current Weston is far ahead of what my embedded devices will do; so I'm in
the position of mostly relying on the desktop for testing.

On Fri, Mar 23, 2018, 09:38 Matt Hoosier  wrote:

> On Fri, Mar 23, 2018 at 9:30 AM, Pekka Paalanen 
> wrote:
> > On Fri, 23 Mar 2018 08:46:46 -0500
> > Matt Hoosier  wrote:
> >
> >> I am very much in favor of the overall approach on this patch series.
> >> I've experienced every single one of the problems described in this
> >> summary, and my company currently resorts to maintaining a hacky
> >> out-of-tree calibration tool to paper over these problems.
> >
> > Hi Matt,
> >
> > that is very heartwarming to hear. Is your tool specifically for Weston
> > too?
>
> Yes and no. It's not phrased as a patch against the Weston source
> code, but it uses heuristics for determining which output the raw
> /dev/input/* events should be correlated against, and those heuristics
> probably would fail if some different compositor happened to be
> running.
>
> >
> > I would be very happy if this proposal fits your needs, and certainly
> > interested in hearing where it falls short.
> >
> >
> > Thanks,
> > pq
> >
> >> On Fri, Mar 23, 2018 at 7:00 AM, Pekka Paalanen 
> wrote:
> >> > From: Pekka Paalanen 
> >> >
> >> > Hi all,
> >> >
> >> > the existing touchscreen calibrator in Weston has several problems.
> This
> >> > proposal intends to solve them all by introducing a new protocol
> >> > extension for touchscreen calibration and a new calibrator tool.
> >> >
> >> > The benefits of the new tool, which the old tool lacks, are:
> >> >
> >> > - You can unambiguously pick a physical touch device to calibrate.
> >> >
> >> > - You can be sure your touch events come only from that particular
> >> >   device, and that you cannot miss touch events even if the current
> >> >   calibration is horribly wrong.
> >> >
> >> > - You can be sure the calibration window (pattern) is shown on the
> right
> >> >   output with the right coordinates.
> >> >
> >> > - You can unambiguously calibrate even multiple touchscreens that are
> >> >   all cloned (showing the same image).
> >> >
> >> > - You get a libinput style calibation matrix instead of the
> >> >   WL_CALIBRATION format which depends on output resolution.
> >> >
> >> > - You can load a new calibration into the compositor without playing
> >> >   tricks with udev or restarting the compositor.
> >> >
> >> > There is more discussion about the topic at:
> >> > https://phabricator.freedesktop.org/T7868
> >> >
> >> > This patch series depends on the clone mode series:
> >> > https://patchwork.freedesktop.org/series/32898/
> >> >
> >> > There is a full branch available at:
> >> > https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
> >
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 00/25] A new touchscreen calibrator

2018-03-23 Thread Matt Hoosier
On Fri, Mar 23, 2018 at 9:30 AM, Pekka Paalanen  wrote:
> On Fri, 23 Mar 2018 08:46:46 -0500
> Matt Hoosier  wrote:
>
>> I am very much in favor of the overall approach on this patch series.
>> I've experienced every single one of the problems described in this
>> summary, and my company currently resorts to maintaining a hacky
>> out-of-tree calibration tool to paper over these problems.
>
> Hi Matt,
>
> that is very heartwarming to hear. Is your tool specifically for Weston
> too?

Yes and no. It's not phrased as a patch against the Weston source
code, but it uses heuristics for determining which output the raw
/dev/input/* events should be correlated against, and those heuristics
probably would fail if some different compositor happened to be
running.

>
> I would be very happy if this proposal fits your needs, and certainly
> interested in hearing where it falls short.
>
>
> Thanks,
> pq
>
>> On Fri, Mar 23, 2018 at 7:00 AM, Pekka Paalanen  wrote:
>> > From: Pekka Paalanen 
>> >
>> > Hi all,
>> >
>> > the existing touchscreen calibrator in Weston has several problems. This
>> > proposal intends to solve them all by introducing a new protocol
>> > extension for touchscreen calibration and a new calibrator tool.
>> >
>> > The benefits of the new tool, which the old tool lacks, are:
>> >
>> > - You can unambiguously pick a physical touch device to calibrate.
>> >
>> > - You can be sure your touch events come only from that particular
>> >   device, and that you cannot miss touch events even if the current
>> >   calibration is horribly wrong.
>> >
>> > - You can be sure the calibration window (pattern) is shown on the right
>> >   output with the right coordinates.
>> >
>> > - You can unambiguously calibrate even multiple touchscreens that are
>> >   all cloned (showing the same image).
>> >
>> > - You get a libinput style calibation matrix instead of the
>> >   WL_CALIBRATION format which depends on output resolution.
>> >
>> > - You can load a new calibration into the compositor without playing
>> >   tricks with udev or restarting the compositor.
>> >
>> > There is more discussion about the topic at:
>> > https://phabricator.freedesktop.org/T7868
>> >
>> > This patch series depends on the clone mode series:
>> > https://patchwork.freedesktop.org/series/32898/
>> >
>> > There is a full branch available at:
>> > https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 00/25] A new touchscreen calibrator

2018-03-23 Thread Matt Hoosier
I am very much in favor of the overall approach on this patch series.
I've experienced every single one of the problems described in this
summary, and my company currently resorts to maintaining a hacky
out-of-tree calibration tool to paper over these problems.

On Fri, Mar 23, 2018 at 7:00 AM, Pekka Paalanen  wrote:
> From: Pekka Paalanen 
>
> Hi all,
>
> the existing touchscreen calibrator in Weston has several problems. This
> proposal intends to solve them all by introducing a new protocol
> extension for touchscreen calibration and a new calibrator tool.
>
> The benefits of the new tool, which the old tool lacks, are:
>
> - You can unambiguously pick a physical touch device to calibrate.
>
> - You can be sure your touch events come only from that particular
>   device, and that you cannot miss touch events even if the current
>   calibration is horribly wrong.
>
> - You can be sure the calibration window (pattern) is shown on the right
>   output with the right coordinates.
>
> - You can unambiguously calibrate even multiple touchscreens that are
>   all cloned (showing the same image).
>
> - You get a libinput style calibation matrix instead of the
>   WL_CALIBRATION format which depends on output resolution.
>
> - You can load a new calibration into the compositor without playing
>   tricks with udev or restarting the compositor.
>
> There is more discussion about the topic at:
> https://phabricator.freedesktop.org/T7868
>
> This patch series depends on the clone mode series:
> https://patchwork.freedesktop.org/series/32898/
>
> There is a full branch available at:
> https://gitlab.collabora.com/pq/weston/commits/touchcalib-1
>
> This series contains many patches that could be landed separately, but I
> have not pulled them out at this point.
>
> A notable single patch is patch 3 which deprecates the udev property
> WL_CALIBRATION. It is completely already superseded by
> LIBINPUT_CALIBRATION_MATRIX property. I would hope to remove
> WL_CALIBRATION support after few releases and the old calibrator tool
> with it.
>
> Patch 21 adds the touchscreen calibration protocol, patch 24 adds the
> new calibrator tool, and patch 25 gives suggestions on how to let the
> server permanently store a new calibration into udev rules.
>
> As LIBINPUT_CALIBRATION_MATRIX is automatically handled by libinput,
> being a description of the physical input device (like e.g. mouse dpi),
> it perfectly possible to run Weston and the calibrator once to calibrate
> your touchscreens, and then run whatever compositor you want.
>
> One major thing this patch series does not address is how a Wayland
> client gets authorized to use the touchscreen calibration interface. The
> interface offers two privileged actions: the ability to grab all touch
> input (but with provision for the server to cancel at will - not
> implemented in weston so far), and the ability to upload a new
> calibration for a touch device. Obviously these should not be free for
> all. The method implemented here is a simple global configuration
> setting to advertise or not the touchscreen calibration interface. If
> advertised, it is free for all. Therefore it is not advertised by
> default.
>
>
> Thanks,
> pq
>
>
> Louis-Francis Ratté-Boulianne (7):
>   input: introduce weston_touch_device
>   libweston: fix weston_touch_start_grab() arg name
>   input: move touchpoint counting up
>   input: introduce touch event mode for calibrator
>   libweston: implement touch calibration protocol
>   weston: add touchscreen_calibrator option
>   clients: add a new touchscreen calibrator
>
> Pekka Paalanen (18):
>   libinput: remove evdev_device::devnode
>   libinput: note if calibrating without an output
>   libinput: deprecate WL_CALIBRATION
>   libinput: log input device to output associations
>   libinput: make setting the same output a no-op
>   libinput: allow evdev_device_set_output(dev, NULL)
>   libinput: use head names for output matching
>   libweston: require connected heads for input devices
>   libinput: do not switch output associations on disable
>   man: document WESTON_LIBINPUT_LOG_PRIORITY env
>   tests: add test_seat_release() for symmetry
>   libinput: move calibration printing into do_set_calibration()
>   libweston: notify_touch API to use weston_touch_device
>   libweston: unexport weston_{pointer,keyboard,touch}_{create,destroy}()
>   libweston: introduce notify_touch_cal() and doc
>   input: do not forward unmatched touch-ups
>   protocol: add weston_touch_calibration
>   doc: add example calibration-helper script
>
>  .gitignore|   1 +
>  Makefile.am   |  21 +-
>  clients/touch-calibrator.c| 774 
> ++
>  clients/window.c  |   4 +-
>  clients/window.h  |   4 +
>  compositor/main.c |  68 +++
>  doc/calibration-helper.bash   |  44 ++
>  libweston/compositor-wayland.c|  31 +-
>  libw

Re: wl_buffer is not released for long time.

2018-03-12 Thread Matt Hoosier
Here's the discussion and patch that addressed the issue I was mentioning:

https://lists.freedesktop.org/archives/wayland-devel/2017-September/035191.html

On Mon, Mar 12, 2018 at 3:51 PM, Matt Hoosier  wrote:
> Hi,
>
> Unless you're using an unreleased version of Weston, I think you're
> probably running into a bug that we fixed a few months ago in which
> wl_buffer::release() events were prone to sit undispatched in the
> server's outgoing queue until some other event happened to need
> transmitted.
>
> -Matt
>
> On Mon, Mar 12, 2018 at 1:23 PM, Sichem Zhou  wrote:
>> Hi all,
>>
>> Dear wayland devs, I have a question regarding to double `wl_buffer`
>> management. I don't seem to have wl_buffer released untill some other events
>> triggered (for example, the  inputs). My current environment is under
>> `X11-backend` and a libweston based compositor.
>>
>> My pipeline follows: (frame, attach, damage, commit) -> buffer switch ->
>> wait until one buffer available -> redraw. The `done` event is handled
>> differently since it only signals if ready to draw.
>>
>> Through my experiments, I found out at I wouldn't get either of the
>> `release` of `frame done` event if I wasn't using the compositor (moving
>> cursor or typing). In this case I guess there is something wrong with my
>> pipeline but I couldn't figure out which. I wish to know if there is obvious
>> mistakes in my pipeline or the problem lies in the compositor.
>>
>>  Very appreciate any answers.
>>
>> SZ
>>
>>
>>
>> ___
>> 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: wl_buffer is not released for long time.

2018-03-12 Thread Matt Hoosier
Hi,

Unless you're using an unreleased version of Weston, I think you're
probably running into a bug that we fixed a few months ago in which
wl_buffer::release() events were prone to sit undispatched in the
server's outgoing queue until some other event happened to need
transmitted.

-Matt

On Mon, Mar 12, 2018 at 1:23 PM, Sichem Zhou  wrote:
> Hi all,
>
> Dear wayland devs, I have a question regarding to double `wl_buffer`
> management. I don't seem to have wl_buffer released untill some other events
> triggered (for example, the  inputs). My current environment is under
> `X11-backend` and a libweston based compositor.
>
> My pipeline follows: (frame, attach, damage, commit) -> buffer switch ->
> wait until one buffer available -> redraw. The `done` event is handled
> differently since it only signals if ready to draw.
>
> Through my experiments, I found out at I wouldn't get either of the
> `release` of `frame done` event if I wasn't using the compositor (moving
> cursor or typing). In this case I guess there is something wrong with my
> pipeline but I couldn't figure out which. I wish to know if there is obvious
> mistakes in my pipeline or the problem lies in the compositor.
>
>  Very appreciate any answers.
>
> SZ
>
>
>
> ___
> 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: [systemd-devel] [PATCH weston] doc/systemd: system service example

2018-01-25 Thread Matt Hoosier
On Mon, Jan 22, 2018 at 7:49 AM, Pekka Paalanen  wrote:
> On Fri, 29 Dec 2017 15:09:28 -0600
> Matt Hoosier  wrote:
>
>> Hi Lennart,
>>
>> On Mon, Dec 4, 2017 at 9:11 AM, Pekka Paalanen  wrote:
>> > On Fri, 1 Dec 2017 18:25:35 +0100
>> > Lennart Poettering  wrote:
>> >
>
>> >> So, as long as weston runs from a PAM enabled service (and its PAM
>> >> snippet pulls in pam_systemd) all should be good and race-free: as the
>> >> PAM session is set up XDG_RUNTIME_DIR will be made available and the
>> >> systemd --user instance is invoked in the background.
>> >
>> > This is exactly what we attempted with the User and PAMName directive,
>> > and it turned out to be racy somehow.
>
>> >> > > > > > +# Set up a full user session for the user, required by Weston.
>> >> > > > > > +PAMName=login
>> >> > > > >
>> >> > > > > Piggy-backing on "login" is a bad idea. "login" is a text tool, 
>> >> > > > > and
>> >> > > > > thus the PAM rules for it usually pull in some TTY specific PAM
>> >> > > > > modules. YOu shoudl really use your own PAM fragment here, and
>> >> > > > > configure only the bits you need.
>> >> > > >
>> >> > >
>> >> > > Oh, so could it just be that we needed something other than
>> >> > > "PAMName=login"?
>> >> >
>> >> > What are they key bits in the PAM configuration we must have, and are
>> >> > there any often used bits we must not have to avoid the race Martyn
>> >> > describes?
>> >>
>> >> well, pam_systemd needs to be pulled in from it, that's the most
>> >> important thing. A PAM snippet that pulls in pam_systemd means you get
>> >> a session allcoated in logind, which in turn sets up XDG_RUNTIME_DIR
>> >> for you.
>
>>
>> The approach that you and Pekka most recently put on record here:
>>
>> * User=foo
>> * PAMName=weston
>>
>> with a /etc/pam.d/weston that just does minimal stuff (enforce the
>> account exists and then execute pam_systemd.so for the session phase)
>> works well for me.
>
> Hi Matt,
>
> that is cool. Could you share your PAM file for 'weston'? When I get to
> re-spinning the example patch, I could use that instead of trying to
> craft my own which I probably could not test in any reasonable time.

Sure, although I'm not certain about its applicability to desktop
distributions. Mine is intended for an OpenEmbedded-style device, and
I basically just adapted the PAM file that some XDM recipe had:

  auth required pam_unix.so null
  account required pam_unix.so
  session required pam_unix.so
  -session optional pam_systemd.so

The entry about pam_systemd.so could probably be promoted to be
strictly required.

>
>> One thing I can't figure out though: using PAMName= causes the service
>> process's journal entries emitted by regular stdout and stderr not to
>> be visible with 'journalctl -u weston.service' anymore. Only the
>> messages coming internally from systemd ("Started Weston." and
>> similar) show in that journal.
>>
>> I've tacked in StandardOutput=journal and StandardError=journal to
>> compensate for the StandardInput=tty-fail. The messages do make it
>> across to journald; you can view them with 'journalctl
>> /usr/bin/weston'. But somehow they're not associated with the system
>> unit weston.service anymore. Does using the PAMName= directive cause
>> the stdout/stderr messages to be reassigned to a user-session unit or
>> something?
>
> That was an interesting point and it's nice to see Mantas' reply
> explaining what happens there. I should make a note of that as well.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [systemd-devel] [PATCH weston] doc/systemd: system service example

2017-12-29 Thread Matt Hoosier
On Dec 29, 2017 15:32, "Mantas Mikulėnas"  wrote:

On Fri, Dec 29, 2017 at 11:09 PM, Matt Hoosier 
wrote:

>
> The approach that you and Pekka most recently put on record here:
>
> * User=foo
> * PAMName=weston
>
> with a /etc/pam.d/weston that just does minimal stuff (enforce the
> account exists and then execute pam_systemd.so for the session phase)
> works well for me.
>
> One thing I can't figure out though: using PAMName= causes the service
> process's journal entries emitted by regular stdout and stderr not to
> be visible with 'journalctl -u weston.service' anymore. Only the
> messages coming internally from systemd ("Started Weston." and
> similar) show in that journal.
>
> I've tacked in StandardOutput=journal and StandardError=journal to
> compensate for the StandardInput=tty-fail. The messages do make it
> across to journald; you can view them with 'journalctl
> /usr/bin/weston'. But somehow they're not associated with the system
> unit weston.service anymore. Does using the PAMName= directive cause
> the stdout/stderr messages to be reassigned to a user-session unit or
> something?
>

No, it's done by pam_systemd specifically.

The main purpose of pam_systemd is to create a user session and move the
process to the session's own cgroup (from the weston.service cgroup). As a
result systemd no longer considers it as belonging to the weston.service
unit, but to session-c123.scope or such.)

(The same happens with all other interactive login types -- e.g. when you
log in at the console, you get moved out of getty@.service and into your
own cgroup.)

-- 
Mantas Mikulėnas


Okay, thanks. So that's just hardwired behavior that we should expect? I
tried looking around the source to pam_systemd a bit, but quickly lost
track of the way that the 'type', 'class', and other parameters to logind's
CreateSession() eventually interact with systemd recordkeeping.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [systemd-devel] [PATCH weston] doc/systemd: system service example

2017-12-29 Thread Matt Hoosier
Hi Lennart,

On Mon, Dec 4, 2017 at 9:11 AM, Pekka Paalanen  wrote:
> On Fri, 1 Dec 2017 18:25:35 +0100
> Lennart Poettering  wrote:
>
>> On Fr, 01.12.17 13:42, Pekka Paalanen (ppaala...@gmail.com) wrote:
>>
>> > > > > This is racy, as the session ID is not really reliably predictable,
>> > > > > and is synthesized in different contexts in different ways, for
>> > > > > example depnding on whether audit is enabled in the kernel it might 
>> > > > > be
>> > > > > session-1.scope rather than session-c1.scope.
>> >
>> > > > If we could determine the bug doesn't exist anymore, that would be
>> > > > awesome and I could just drop this.
>> >
>> > Hi Lennart,
>> >
>> > taking a step back, the session-c1.scope directive is definitely not
>> > wanted and we should drop it. We should also use a custom PAM name
>> > setup. If we do that, is the service file otherwise good for
>> > guaranteeing:
>> >
>> > - a full user session setup (I presume we want it), specifically
>> >   XDG_RUNTIME_DIR being set up
>> >
>> > - running exclusively on a VT that is made current
>>
>> This really depends on how weston sets up a VT. I really don't know
>> Weston and what it does.
>
> Weston doesn't set up the VT, we rely on the systemd unit directives to
> set it up.
>
> Weston calls sd_pid_get_session(getpid()), sd_session_get_seat(), and wants
> sd_session_get_vt() to succeed and give a VT number. Then it connects
> to logind, wants TakeControl to succeed, and calls Activate. It uses
> TakeDevice to open the DRM KMS device and input devices. I think that's
> the start-up sequence, it also listens on signals from logind etc.
>
>> > - access to DRM and input devices via logind
>>
>> So, I can't comment on Weston really.
>
> No worries, that was more of a general question about whether the
> example unit file was making any unwarranted assumptions.
>
>> Here are brief (and possibly slightly out-of-date, but probably not)
>> notes on how to write display managers with logind:
>>
>> https://www.freedesktop.org/wiki/Software/systemd/writing-display-managers/
>
> Thanks, I had a quick read through. We expect the systemd unit to also
> set up PAM, Weston itself does not touch PAM.
>
>> > so that all these are already in place by the time the Weston process
>> > is started?
>> >
>> > As you can see from Martyn below, the first issue that prevented Weston
>> > from running was that XDG_RUNTIME_DIR was not set. Furthermore, this
>> > failure did not occur always, sometimes things just worked as we
>> > expected.
>>
>> So, as long as weston runs from a PAM enabled service (and its PAM
>> snippet pulls in pam_systemd) all should be good and race-free: as the
>> PAM session is set up XDG_RUNTIME_DIR will be made available and the
>> systemd --user instance is invoked in the background.
>
> This is exactly what we attempted with the User and PAMName directive,
> and it turned out to be racy somehow.
>
>> What currently is not supported is to run things independently of any
>> session, i.e. run weston as systemd --user service with nothing that
>> creates a session in the first place. In that case XDG_RUNTIME_DIR
>> will not be set up, and no devices are made available to weston...
>
> Weston never was a --user service.
>
> As far as I know, there was also nothing that would manually attempt to
> start user@.service, the only trigger for that were the User and PAMName
> directives in the system weston.service.
>
>> > > > > > +# Set up a full user session for the user, required by Weston.
>> > > > > > +PAMName=login
>> > > > >
>> > > > > Piggy-backing on "login" is a bad idea. "login" is a text tool, and
>> > > > > thus the PAM rules for it usually pull in some TTY specific PAM
>> > > > > modules. YOu shoudl really use your own PAM fragment here, and
>> > > > > configure only the bits you need.
>> > > >
>> > >
>> > > Oh, so could it just be that we needed something other than
>> > > "PAMName=login"?
>> >
>> > What are they key bits in the PAM configuration we must have, and are
>> > there any often used bits we must not have to avoid the race Martyn
>> > describes?
>>
>> well, pam_systemd needs to be pulled in from it, that's the most
>> important thing. A PAM snippet that pulls in pam_systemd means you get
>> a session allcoated in logind, which in turn sets up XDG_RUNTIME_DIR
>> for you.
>
> Yes, it was, but apparently somewhere in the PAM stack or something it
> calls there was a race, which sometimes let Weston to start before
> XDG_RUNTIME_DIR environment variable was set, causing weston to fail.
>
> We all here were quite baffled on what could even be racing, unless it
> is possible that the weston process gets started in parallel with the
> PAM setup done by the User/PAMName in weston.service. We assumed that
> all the setup described in the systemd unit file would be guaranteed to
> complete before the actual process gets started.
>
> It seems our and your expectations are aligned. Maybe we should just
> forget about that race, remove the h

Re: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell

2017-12-22 Thread Matt Hoosier
On Wed, Nov 8, 2017 at 1:30 AM, Pekka Paalanen  wrote:
> On Tue, 7 Nov 2017 11:01:09 -0600
> Matt Hoosier  wrote:
>
>> Hi Pekka,
>>
>> On Wed, Oct 25, 2017 at 10:09 AM, Ucan, Emre (ADITG/ESB)
>>  wrote:
>> > Actually,  IMO ivi-shell is not a proper wayland compositor.
>> > Because it is violating wayland protocol by not supporting wl_shell
>> > interface.
>> >
>> > Therefore, we have to at least support wl_shell interface in
>> > ivi-shell. Why not support it via libweston-desktop ?
>>
>> I'm wondering if you have any thoughts on this one specific point that
>> Emre made. I know there's a lot of heartburn over the inclusion of
>> wl_shell into the core protocol, and you wouldn't do it that way if
>> that decision were getting made today.
>
> Hi,
>
> I have a long reply brewing for that email and several points
> you raised, but I have not had the time to finish it yet. It is
> very time consuming to write the rebuttal for this proposal.
>
>
> Thanks,
> pq

Hi Pekka,

I'm still interested to hear your position here. If writing the full
response is still prohibitive, could you maybe just confirm whether
your stance is about the technical nuts and bolts of how (or whether)
the IVI shell can make a conforming implementation of the desktop
shell APIs, or whether you just think it's wrong-headed for an IVI
system to attempt to use code that was developed with the nominal
purpose of showing on a desktop system.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v4 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-12-13 Thread Matt Hoosier
On Mon, Dec 11, 2017 at 2:03 AM, Pekka Paalanen  wrote:
> On Mon, 27 Nov 2017 08:54:54 -0600
> Matt Hoosier  wrote:
>
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>
>> v4 changes:
>>
>> * Improved internal comments and some boundary-condition
>>   error checks in test case.
>> * Refer to compositor as "Wayland server" rather than "Wayland
>>   display" in wl_display_connect() doxygen comments.
>> * Remove redundant descriptions of parameter-interpretation
>>   mechanics from wl_display_connect() manpage. Reworked things
>>   to make it clear that 'name' and $WAYLAND_DISLAY are each
>>   capable of encoding absolute server socket paths.
>> * Remove callout to reference implementation behavior in protocol
>>   documented. In its place there is now a simple statement that
>>   implementations can optionally support absolute socket paths.
>>
>> v3 changes:
>>
>> * Added test case.
>> * Clarified documentation to note that 'name' parameter to 
>> wl_display_connect()
>>   can also be an absolute path.
>>
>> v2 changes:
>>
>> * Added backward incompatibility note to wl_display_connect() manpage.
>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>   changed behavior.
>> * Added mention of new absolute path behavior in wl_display_connect()
>>   doxygen comments.
>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>   protocol documentation.
>>
>> Signed-off-by: Matt Hoosier 
>> Acked-by: Pekka Paalanen 
>> Acked-by: Jonas Ådahl 
>> ---
>>  doc/man/wl_display_connect.xml|  32 +--
>>  doc/publican/sources/Protocol.xml |   5 +-
>>  src/wayland-client.c  |  47 
>>  tests/socket-test.c   | 109 
>> ++
>>  4 files changed, 177 insertions(+), 16 deletions(-)
>
> Hi Matt,
>
> this patch is:
>
> Reviewed-by: Pekka Paalanen 
>
> The wording in the man page sounds little like WAYLAND_DISPLAY
> accepting an absolute path is a side-effect rather than an intentional
> feature, but it doesn't matter.
>
> Everyone,
>
> if there are no objections, I will push this patch on Wednesday, that
> is in two days. If you want your R-b or Acks recorded that are not
> already in the above, please send them explicitly.
>
>
> Thanks,
> pq

Hi Pekka,

Did you ever hear any objections to this one?

-Matt

>
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..dab4ddb 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,15 +55,39 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR when 
>> WAYLAND_DISPLAY
>> +   (or name, see below) is a simple name, for this
>> +   function to find it. The server socket is also allowed to exist at an
>> +   arbitrary path; usage details follow. See below for compatibility 
>> issue
>> +   details.
>> +
>> +The name argument specifies the name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>> -  WAYLAND_DISPLAY replaces the default value. If
>> -  WAYLAND_SOCKET is set, this function behaves like
>> +  WAYLAND_DISPLAY replaces the default value.
>> +
>> +  

Re: [PATCH weston v3] libweston-desktop: add signal for title/app-id changes

2017-12-08 Thread Matt Hoosier
On Fri, Dec 8, 2017 at 4:26 AM, Quentin Glidic
 wrote:
> On 12/8/17 11:22 AM, Quentin Glidic wrote:
>>
>> From: Matt Hoosier 
>>
>> As discussed on
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034720.html,
>> it's useful for the shell implementation to know when these change,
>> for example to relay the information on to taskbars or similar.
>>
>> To avoid ABI changes or the need to make the weston_desktop_surface
>> definition public, new functions are introduced for attaching
>> listeners to these signals.
>>
>> Signed-off-by: Matt Hoosier 
>> Reviewed-by: Quentin Glidic 
>> Signed-off-by: Quentin Glidic 
>> ---
>
>
> (Err, wanted to drop the Sob send-email added via command line and I just
> closed vim…)
>
> I made this v3 to rename the signal to "metadata" instead of "title", but
> also to tweak a tiny bit the signal sending. I wanted the shell not to have
> to strdup() these, so now, it can listen to the signal, and compare old and
> new values (and even use both) without extra cost.
> Do you ack that change?
>
> Still not sure I prefer a signal to just adding to the callbacks struct, but
> we can always break API/ABI again later if we need to. :-)

I like the change, and prefer the name "metadata" for the signal
anyway. The v3 changes are:

Acked-by: Matt Hoosier 

(Not sure whether you actually want to put it into the commit message,
since I'm the original author. But I approve.)

Thanks for the review.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Making weston (or maybe libinput) ignore an input device

2017-12-07 Thread Matt Hoosier
On Thu, Dec 7, 2017 at 2:05 AM, Pekka Paalanen  wrote:
> On Tue, 5 Dec 2017 13:57:39 -0600
> Matt Hoosier  wrote:
>
>> Hi all,
>>
>> The thing I'd like to arrange is for a device which is otherwise a
>> valid keyboard, not to get automatically vacuumed up into the Weston
>> DRM backend's usage of libinput.
>>
>> Nowadays, Weston has delegated most of its former firsthand extraction
>> of UDev properties to libinput. Nothing exists (that I found) in
>> Weston's immediate sources to cause an input device reported through
>> the libinput UDev seats to be ignored. I also had a quick stroll
>> through libinput to look for some kind of sensitivity to a UDev
>> property with that effect, but didn't spot anything likely.
>>
>> Is there a best way to do this? The best thing I could think of was to
>> try to add a late-running UDev rule that subtracts out the
>> ID_INPUT_KEYBOARD=1 property, so that libinput won't know what to do
>> with the device. But that seems like a fragile solution -- some
>> package unknown to me could come along and install an even-later
>> running rule that messages that arrangement up.
>
> Hi Matt,
>
> you could try assigning that device to a different physical seat with
> the ID_SEAT property:
> https://wayland.freedesktop.org/libinput/doc/latest/udev_config.html

Aaaah, right. Thanks, that did the trick.

>
> There is also LIBINPUT_IGNORE_DEVICE mentioned on that page.
>
> Some talk about physical vs. logical (wl_seat) seats:
> https://wayland.freedesktop.org/libinput/doc/latest/seats.html
>
> A compositor usually handles only one physical seat, and Weston in
> particular does so.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Making weston (or maybe libinput) ignore an input device

2017-12-05 Thread Matt Hoosier
Hi all,

The thing I'd like to arrange is for a device which is otherwise a
valid keyboard, not to get automatically vacuumed up into the Weston
DRM backend's usage of libinput.

Nowadays, Weston has delegated most of its former firsthand extraction
of UDev properties to libinput. Nothing exists (that I found) in
Weston's immediate sources to cause an input device reported through
the libinput UDev seats to be ignored. I also had a quick stroll
through libinput to look for some kind of sensitivity to a UDev
property with that effect, but didn't spot anything likely.

Is there a best way to do this? The best thing I could think of was to
try to add a late-running UDev rule that subtracts out the
ID_INPUT_KEYBOARD=1 property, so that libinput won't know what to do
with the device. But that seems like a fragile solution -- some
package unknown to me could come along and install an even-later
running rule that messages that arrangement up.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v3 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-30 Thread Matt Hoosier
Hi Jonas (and Pekka),

How well does v4 address your requests?

-Matt

On Mon, Nov 27, 2017 at 9:00 AM, Matt Hoosier  wrote:
> On Mon, Nov 27, 2017 at 3:16 AM, Pekka Paalanen  wrote:
>> On Sat, 25 Nov 2017 08:11:43 -0600
>> Matt Hoosier  wrote:
>>
>>> Hi Jonas, Pekka,
>>>
>>> I have no objection to making the tweaks that Jonas mentions, but I'm
>>> wary of messing with the form of this patch that Pekka already
>>> stamped. Pekka, am I going to lose your consent as given in v3 if I
>>> make changes along the lines Jonas requests?
>>
>> Hi Matt,
>>
>> you can downgrade my Reviewed-by to Acked-by in this case. I think
>> doing the changes Jonas suggested are good, but I cannot give R-b
>> without seeing the actual words. I'll just re-review then.
>
> Hi Pekka,
>
> Thanks.
>
> Jonas,
>
> I've overhauled the manpage write-up to try to incorporate your
> suggestions. I wasn't clear about how to interpret a couple of the
> things you wrote, but I think your main thrust was to remove
> wordiness, get rid of redundancy, and always call attention to the
> compatibility notes. I've tried that, mostly be re-ordering things so
> that the natural flow of the documentation on parameter interpretation
> that was in-place before my change now survives. Now it just reads
> like:
>
> - *name* gives the socket to use (it's default value is wayland-0)
> - $WAYLAND_DISPLAY replaces the default
> - both *name* and $WAYLAND_DISPLAY are allowed to be absolute paths
> - implication: if $WAYLAND_DISPLAY is absolute, then the connection
> attempt will be too
> - $WAYLAND_SOCKET overrides all of this, if it's set
>
> -Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Hide surface

2017-11-27 Thread Matt Hoosier
On Sun, Nov 26, 2017 at 2:23 PM, ferreiradaselva
 wrote:
> Hi,
>
> My first time posting here (sorry if I'm posting on the wrong place).

You're in the right place. Welcome!

>
> I'm working on this window creation framework
> (https://github.com/ferreiradaselva/swfw), and for the Wayland backend I
> need to implement a function to hide the "window".
>
> I found that that is done by destroying the surface (I'm assuming the
> wl_shell_surface). This is currently how I'm trying to do:
>
> =
> void swfw_hide_window_wl(struct swfw_window_wl *swfw_win_wl)
> {
> if (swfw_win_wl->shell_surface) {
> wl_shell_surface_destroy(swfw_win_wl->shell_surface);
> swfw_win_wl->shell_surface = NULL;
> wl_surface_commit(swfw_win_wl->surface);
> }
> }
>
> void swfw_show_window_wl(struct swfw_window_wl *swfw_win_wl)
> {
> if (!swfw_win_wl->shell_surface) {
> swfw_win_wl->shell_surface =
> wl_shell_get_shell_surface(swfw_win_wl->swfw_ctx_wl->shell,
> swfw_win_wl->surface);
> wl_shell_surface_add_listener(swfw_win_wl->shell_surface,
> &shell_surface_listener, swfw_win_wl);
> wl_shell_surface_set_toplevel(swfw_win_wl->shell_surface);
> wl_surface_commit(swfw_win_wl->surface);
> }
> }
> =
>
> However, even after destroying the shell surface, the surface is still
> rendered (testing on weston). Calling the function to show the surface again
> will result in the surface being rendered twice on the screen. I'm also
> using EGL, if that matters somehow.
>
> I couldn't find any example on internet on how to properly hide the surface
> (I couldn't get working with GLFW, and SDL doesn't seem to implement the
> hide function on wayland).

There are a couple levels of abstraction you might choose to approach
your need from. If you're always going to be using wl-shell, then you
can minimize the window. Have a look at wl_shell_surface::minimize.
Depending on the desktop environment, that may be the behavior you
were most closely seeking anyway.

If you simply want the application-defined window content to
disappear, however, then just attach a null buffer to the wl_surface.
That is, wl_surface::attach( NULL ). Upon the next
wl_surface::commit(), the prior renderbuffers will no longer get
painted into the composited scene.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v3 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-27 Thread Matt Hoosier
On Mon, Nov 27, 2017 at 3:16 AM, Pekka Paalanen  wrote:
> On Sat, 25 Nov 2017 08:11:43 -0600
> Matt Hoosier  wrote:
>
>> Hi Jonas, Pekka,
>>
>> I have no objection to making the tweaks that Jonas mentions, but I'm
>> wary of messing with the form of this patch that Pekka already
>> stamped. Pekka, am I going to lose your consent as given in v3 if I
>> make changes along the lines Jonas requests?
>
> Hi Matt,
>
> you can downgrade my Reviewed-by to Acked-by in this case. I think
> doing the changes Jonas suggested are good, but I cannot give R-b
> without seeing the actual words. I'll just re-review then.

Hi Pekka,

Thanks.

Jonas,

I've overhauled the manpage write-up to try to incorporate your
suggestions. I wasn't clear about how to interpret a couple of the
things you wrote, but I think your main thrust was to remove
wordiness, get rid of redundancy, and always call attention to the
compatibility notes. I've tried that, mostly be re-ordering things so
that the natural flow of the documentation on parameter interpretation
that was in-place before my change now survives. Now it just reads
like:

- *name* gives the socket to use (it's default value is wayland-0)
- $WAYLAND_DISPLAY replaces the default
- both *name* and $WAYLAND_DISPLAY are allowed to be absolute paths
- implication: if $WAYLAND_DISPLAY is absolute, then the connection
attempt will be too
- $WAYLAND_SOCKET overrides all of this, if it's set

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v4 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-27 Thread Matt Hoosier
From: Matt Hoosier 

In order to support system compositor instances, it is necessary to
allow clients' wl_display_connect() to find the compositor's listening
socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
the discussion beginning here:

https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html

This change adjusts the client-side connection logic so that, if
WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
connection attempt is made to just $WAYLAND_DISPLAY rather than
usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.

This change is based on Davide Bettio's submission of the same concept
at:

https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.

v4 changes:

* Improved internal comments and some boundary-condition
  error checks in test case.
* Refer to compositor as "Wayland server" rather than "Wayland
  display" in wl_display_connect() doxygen comments.
* Remove redundant descriptions of parameter-interpretation
  mechanics from wl_display_connect() manpage. Reworked things
  to make it clear that 'name' and $WAYLAND_DISLAY are each
  capable of encoding absolute server socket paths.
* Remove callout to reference implementation behavior in protocol
  documented. In its place there is now a simple statement that
  implementations can optionally support absolute socket paths.

v3 changes:

* Added test case.
* Clarified documentation to note that 'name' parameter to wl_display_connect()
  can also be an absolute path.

v2 changes:

* Added backward incompatibility note to wl_display_connect() manpage.
* Rephased wl_display_connect() manpage changes to precisely match actual
  changed behavior.
* Added mention of new absolute path behavior in wl_display_connect()
  doxygen comments.
* Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
  protocol documentation.

Signed-off-by: Matt Hoosier 
Acked-by: Pekka Paalanen 
Acked-by: Jonas Ådahl 
---
 doc/man/wl_display_connect.xml|  32 +--
 doc/publican/sources/Protocol.xml |   5 +-
 src/wayland-client.c  |  47 
 tests/socket-test.c   | 109 ++
 4 files changed, 177 insertions(+), 16 deletions(-)

diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
index 7e6e05c..dab4ddb 100644
--- a/doc/man/wl_display_connect.xml
+++ b/doc/man/wl_display_connect.xml
@@ -55,15 +55,39 @@
 Description
 wl_display_connect connects to a Wayland socket
   that was previously opened by a Wayland server. The server socket 
must
-  be placed in XDG_RUNTIME_DIR for this function to
-  find it. The name argument specifies the name of
+  be placed in XDG_RUNTIME_DIR when 
WAYLAND_DISPLAY
+ (or name, see below) is a simple name, for this
+ function to find it. The server socket is also allowed to exist at an
+ arbitrary path; usage details follow. See below for compatibility 
issue
+ details.
+
+The name argument specifies the name of
   the socket or NULL to use the default (which is
   "wayland-0"). The environment variable
-  WAYLAND_DISPLAY replaces the default value. If
-  WAYLAND_SOCKET is set, this function behaves like
+  WAYLAND_DISPLAY replaces the default value.
+
+ If name is an absolute path, then that path is used
+ as the Wayland socket to which the connection is attempted. Note that
+ in combination with the default-value behavior described above, this
+ implies that setting WAYLAND_DISPLAY to an absolute
+ path will implicitly cause name to take on that
+ absolute path if name is NULL.
+
+  If WAYLAND_SOCKET is set, this function behaves like
   wl_display_connect_to_fd with the 
file-descriptor
   number taken from the environment variable.
 
+Support for interpreting WAYLAND_DISPLAY as an
+  absolute path is a change in behavior compared to
+  wl_display_connect's behavior in versions
+  1.14 and older of Wayland. It is no longer guaranteed in versions
+  1.15 and higher that the Wayland socket chosen is equivalent to
+  manually constructing a socket pathname by concatenating
+  XDG_RUNTIME_DIR and WAYLAND_DISPLAY.
+  Manual construction of the socket path must account for the
+  possibility that WAYLAND_DISPLAY contains an absolute
+  path.
+
 wl_display_connect_to_fd connects to a Wayland
   socket with an explicit file-descriptor. The file-descriptor is 
passed
   as argument fd.
diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index ba6b5f1..9fdee9a 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -94,7 +94,10 @@
   The

Re: [PATCH v3 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-25 Thread Matt Hoosier
Hi Jonas, Pekka,

I have no objection to making the tweaks that Jonas mentions, but I'm
wary of messing with the form of this patch that Pekka already
stamped. Pekka, am I going to lose your consent as given in v3 if I
make changes along the lines Jonas requests?

On Tue, Nov 21, 2017 at 10:00 PM, Jonas Ådahl  wrote:
> On Tue, Nov 14, 2017 at 12:23:55PM -0600, Matt Hoosier wrote:
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>
>> v3 changes:
>>
>> * Added test case.
>> * Clarified documentation to note that 'name' parameter to 
>> wl_display_connect()
>>   can also be an absolute path.
>>
>> v2 changes:
>>
>> * Added backward incompatibility note to wl_display_connect() manpage.
>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>   changed behavior.
>> * Added mention of new absolute path behavior in wl_display_connect()
>>   doxygen comments.
>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>   protocol documentation.
>>
>> Signed-off-by: Matt Hoosier 
>
> This feature is
>
> Acked-by: Jonas Ådahl 
>
> but I have a few comments on the documentation below.
>
>> ---
>>  doc/man/wl_display_connect.xml|  22 ++--
>>  doc/publican/sources/Protocol.xml |   5 +-
>>  src/wayland-client.c  |  47 
>>  tests/socket-test.c   | 109 
>> ++
>>  4 files changed, 168 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..9c67612 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,30 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY or 
>> name
>> +  for this function to find it. The name 
>> argument specifies the name of
>
> nit1: This line looks like it should be split.
>
> nit2: It's easy to miss the compatibility issues by not mentioning
> anything about it here, so maybe refer to the section below here?
> Something like
>
> The server socket must be placed in XDG_RUNTIME_DIR
> or, depending on version libwayland, exist at the absolute path
> referred to by WAYLA... ...find it. See below for compatibility
> issue details.
>
> Otherwise, I think the new wording is a bit awkward:
>
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>>be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>>path referred to by WAYLAND_DISPLAY or 
>> name
>>for this function to find it.
>
> This changes the meaning of "name" and WAYLAND_DISPLAY.

Can you be more specific? The implementation does allow 'name' now to
encode an absolute path, so I agree that it changes the meaning in
that sense. What else did you mean? The bit saying 'the server socket
must be placed in XDG_RUNTIME_DIR' is recycled language that already
existed before this change.

>
>>The name argument specifies the name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0").
>
> This refers to the old meaning where "name" is either NULL or the socket
> *name*

Re: [PATCH v3 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-18 Thread Matt Hoosier
On Fri, Nov 17, 2017 at 2:22 AM, Pekka Paalanen  wrote:
> On Tue, 14 Nov 2017 12:23:55 -0600
> Matt Hoosier  wrote:
>
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>
>> v3 changes:
>>
>> * Added test case.
>> * Clarified documentation to note that 'name' parameter to 
>> wl_display_connect()
>>   can also be an absolute path.
>>
>> v2 changes:
>>
>> * Added backward incompatibility note to wl_display_connect() manpage.
>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>   changed behavior.
>> * Added mention of new absolute path behavior in wl_display_connect()
>>   doxygen comments.
>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>   protocol documentation.
>>
>> Signed-off-by: Matt Hoosier 
>
> Hi Matt,
>
> this patch looks very good. I have made some very small comments below,
> but I'm confident that those fixed or discussed this patch will be:
>
> Reviewed-by: Pekka Paalanen 
>
> Everyone else, especially those in the CC, I would like to ask you to
> give your explicit Acked-by for this patch to record the community
> acceptance in the git history.
>
> Matt, could you hold off sending v4 for a while so that you can collect
> all the Reviewed-by's and Acked-by's that will arrive, but I am also
> fine collecting them myself if you want this sooner out of your hands.

Thanks, Pekka.

Yeah, I can wait a while to submit v4. I'll make the small changes you
requested in your comments below, and add your r-b. I'm a little
unclear on how I'll know when enough time has elapsed in your opinion
to judge that all the ack's and r-b's that are likely to eventually
arrive, have done so.

>
>> ---
>>  doc/man/wl_display_connect.xml|  22 ++--
>>  doc/publican/sources/Protocol.xml |   5 +-
>>  src/wayland-client.c  |  47 
>>  tests/socket-test.c   | 109 
>> ++
>>  4 files changed, 168 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..9c67612 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,30 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY or 
>> name
>> +  for this function to find it. The name 
>> argument specifies the name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>>WAYLAND_DISPLAY replaces the default value. If
>>WAYLAND_SOCKET is set, this function behaves like
>>wl_display_connect_to_fd with the 
>> file-descriptor
>> -  number taken from the environment variable.
>> +  number taken from the environment variable. If
>> +  WAYLAND_SOCKET is not set and 
>> name
>> +  is NULL and WAYLAND_DISPLAY
>> +  is an absolute path, then the path stored in 
>> WAYLAND_DISPLAY
>> +  is used as the Wayland socket to which the connection is 
>> attempted.
>> +
>> +Support for interpreting WAYLAND_DISPLAY as an
>> +  absolute path is a change in behavior compared to
>> +  wl_display_connect's behavior in versions
>> + 

Re: [PATCH v2 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-14 Thread Matt Hoosier
On Tue, Nov 14, 2017 at 8:22 AM, Pekka Paalanen  wrote:
> On Fri, 10 Nov 2017 11:20:42 -0600
> Matt Hoosier  wrote:
>
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>
>> v2 changes:
>>
>> * Added backward incompatibility note to wl_display_connect() manpage.
>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>   changed behavior.
>> * Added mention of new absolute path behavior in wl_display_connect()
>>   doxygen comments.
>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>   protocol documentation.
>>
>> Signed-off-by: Matt Hoosier 
>> ---
>>  doc/man/wl_display_connect.xml| 22 +---
>>  doc/publican/sources/Protocol.xml |  5 -
>>  src/wayland-client.c  | 43 
>> +--
>>  3 files changed, 55 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..7bdfc46 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,30 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY for this 
>> function
>> +  to find it. The name argument specifies the 
>> name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>>WAYLAND_DISPLAY replaces the default value. If
>>WAYLAND_SOCKET is set, this function behaves like
>>wl_display_connect_to_fd with the 
>> file-descriptor
>> -  number taken from the environment variable.
>> +  number taken from the environment variable. If
>> +  WAYLAND_SOCKET is not set and 
>> name
>> +  is NULL and WAYLAND_DISPLAY
>> +  is an absolute path, then the path stored in 
>> WAYLAND_DISPLAY
>> +  is used as the Wayland socket to which the connection is 
>> attempted.
>> +
>> +Support for interpreting WAYLAND_DISPLAY as an
>> +  absolute path is a change in behavior compared to
>> +  wl_display_connect's behavior in versions
>> +  1.14 and older of Wayland. It is no longer guaranteed in versions
>> +  1.15 and higher that the Wayland socket chosen is equivalent to
>> +  manually constructing a socket pathname by concatenating
>> +  XDG_RUNTIME_DIR and WAYLAND_DISPLAY.
>> +  Manual construction of the socket path must account for the
>> +  possibility that WAYLAND_DISPLAY contains an 
>> absolute
>> +  path.
>
> Hi Matt,
>
> this is a bit verbose, but it is correct. 'name' argument could now be
> absolute as well.

Done.

>
>>
>>  wl_display_connect_to_fd connects to a 
>> Wayland
>>socket with an explicit file-descriptor. The file-descriptor is 
>> passed
>> diff --git a/doc/publican/sources/Protocol.xml 
>> b/doc/publican/sources/Protocol.xml
>> index ba6b5f1..dbfed06 100644
>> --- a/doc/publican/sources/Protocol.xml
>> +++ b/doc/publican/sources/Protocol.xml
>> @@ -94,7 +94,10 @@
>>The protocol is sent over a UNIX domain stream socket, where the 
>> endpoint
>>usually is named wayland-0
>>(although it can be changed via WAY

Re: [PATCH v2 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-14 Thread Matt Hoosier
On Tue, Nov 14, 2017 at 8:25 AM, Matt Hoosier  wrote:
> On Tue, Nov 14, 2017 at 8:22 AM, Pekka Paalanen  wrote:
>> On Fri, 10 Nov 2017 11:20:42 -0600
>> Matt Hoosier  wrote:
>>
>>> From: Matt Hoosier 
>>>
>>> In order to support system compositor instances, it is necessary to
>>> allow clients' wl_display_connect() to find the compositor's listening
>>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>>> the discussion beginning here:
>>>
>>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>>
>>> This change adjusts the client-side connection logic so that, if
>>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>>
>>> This change is based on Davide Bettio's submission of the same concept
>>> at:
>>>
>>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>>
>>> v2 changes:
>>>
>>> * Added backward incompatibility note to wl_display_connect() manpage.
>>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>>   changed behavior.
>>> * Added mention of new absolute path behavior in wl_display_connect()
>>>   doxygen comments.
>>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>>   protocol documentation.
>>>
>>> Signed-off-by: Matt Hoosier 
>>> ---
>>>  doc/man/wl_display_connect.xml| 22 +---
>>>  doc/publican/sources/Protocol.xml |  5 -
>>>  src/wayland-client.c  | 43 
>>> +--
>>>  3 files changed, 55 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>>> index 7e6e05c..7bdfc46 100644
>>> --- a/doc/man/wl_display_connect.xml
>>> +++ b/doc/man/wl_display_connect.xml
>>> @@ -55,14 +55,30 @@
>>>  Description
>>>  wl_display_connect connects to a Wayland 
>>> socket
>>>that was previously opened by a Wayland server. The server 
>>> socket must
>>> -  be placed in XDG_RUNTIME_DIR for this function to
>>> -  find it. The name argument specifies the name 
>>> of
>>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>>> absolute
>>> +  path referred to by WAYLAND_DISPLAY for this 
>>> function
>>> +  to find it. The name argument specifies the 
>>> name of
>>>the socket or NULL to use the default 
>>> (which is
>>>"wayland-0"). The environment variable
>>>WAYLAND_DISPLAY replaces the default value. If
>>>WAYLAND_SOCKET is set, this function behaves like
>>>wl_display_connect_to_fd with the 
>>> file-descriptor
>>> -  number taken from the environment variable.
>>> +  number taken from the environment variable. If
>>> +  WAYLAND_SOCKET is not set and 
>>> name
>>> +  is NULL and WAYLAND_DISPLAY
>>> +  is an absolute path, then the path stored in 
>>> WAYLAND_DISPLAY
>>> +  is used as the Wayland socket to which the connection is 
>>> attempted.
>>> +
>>> +Support for interpreting WAYLAND_DISPLAY as an
>>> +  absolute path is a change in behavior compared to
>>> +  wl_display_connect's behavior in versions
>>> +  1.14 and older of Wayland. It is no longer guaranteed in versions
>>> +  1.15 and higher that the Wayland socket chosen is equivalent to
>>> +  manually constructing a socket pathname by concatenating
>>> +  XDG_RUNTIME_DIR and 
>>> WAYLAND_DISPLAY.
>>> +  Manual construction of the socket path must account for the
>>> +  possibility that WAYLAND_DISPLAY contains an 
>>> absolute
>>> +  path.
>>
>> Hi Matt,
>>
>> this is a bit verbose, but it is correct. 'name' argument could now be
>> absolute as well.
>>
>>>
>>>  wl_display_connect_to_fd connects to a 
>>> Wayland
>>>socket with an explicit file-descriptor. The file-descriptor is 
>>> passed
>>> d

[PATCH v3 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-14 Thread Matt Hoosier
From: Matt Hoosier 

In order to support system compositor instances, it is necessary to
allow clients' wl_display_connect() to find the compositor's listening
socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
the discussion beginning here:

https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html

This change adjusts the client-side connection logic so that, if
WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
connection attempt is made to just $WAYLAND_DISPLAY rather than
usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.

This change is based on Davide Bettio's submission of the same concept
at:

https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.

v3 changes:

* Added test case.
* Clarified documentation to note that 'name' parameter to wl_display_connect()
  can also be an absolute path.

v2 changes:

* Added backward incompatibility note to wl_display_connect() manpage.
* Rephased wl_display_connect() manpage changes to precisely match actual
  changed behavior.
* Added mention of new absolute path behavior in wl_display_connect()
  doxygen comments.
* Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
  protocol documentation.

Signed-off-by: Matt Hoosier 
---
 doc/man/wl_display_connect.xml|  22 ++--
 doc/publican/sources/Protocol.xml |   5 +-
 src/wayland-client.c  |  47 
 tests/socket-test.c   | 109 ++
 4 files changed, 168 insertions(+), 15 deletions(-)

diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
index 7e6e05c..9c67612 100644
--- a/doc/man/wl_display_connect.xml
+++ b/doc/man/wl_display_connect.xml
@@ -55,14 +55,30 @@
 Description
 wl_display_connect connects to a Wayland socket
   that was previously opened by a Wayland server. The server socket 
must
-  be placed in XDG_RUNTIME_DIR for this function to
-  find it. The name argument specifies the name of
+  be placed in XDG_RUNTIME_DIR or exist at the absolute
+  path referred to by WAYLAND_DISPLAY or 
name
+  for this function to find it. The name argument 
specifies the name of
   the socket or NULL to use the default (which is
   "wayland-0"). The environment variable
   WAYLAND_DISPLAY replaces the default value. If
   WAYLAND_SOCKET is set, this function behaves like
   wl_display_connect_to_fd with the 
file-descriptor
-  number taken from the environment variable.
+  number taken from the environment variable. If
+  WAYLAND_SOCKET is not set and name
+  is NULL and WAYLAND_DISPLAY
+  is an absolute path, then the path stored in 
WAYLAND_DISPLAY
+  is used as the Wayland socket to which the connection is 
attempted.
+
+Support for interpreting WAYLAND_DISPLAY as an
+  absolute path is a change in behavior compared to
+  wl_display_connect's behavior in versions
+  1.14 and older of Wayland. It is no longer guaranteed in versions
+  1.15 and higher that the Wayland socket chosen is equivalent to
+  manually constructing a socket pathname by concatenating
+  XDG_RUNTIME_DIR and WAYLAND_DISPLAY.
+  Manual construction of the socket path must account for the
+  possibility that WAYLAND_DISPLAY contains an absolute
+  path.
 
 wl_display_connect_to_fd connects to a Wayland
   socket with an explicit file-descriptor. The file-descriptor is 
passed
diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index ba6b5f1..dbfed06 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -94,7 +94,10 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named wayland-0
   (although it can be changed via WAYLAND_DISPLAY
-  in the environment).
+  in the environment). In the reference implementation, a client whose
+  WAYLAND_DISPLAY is formatted as an absolute path
+  connects to that path as the endpoint, otherwise the implementation
+  searches in XDG_RUNTIME_DIR for the endpoint.
 
 
   Every message is structured as 32-bit words; values are represented in 
the
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 3d7361e..bcf35a6 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -857,9 +857,17 @@ connect_to_socket(const char *name)
socklen_t size;
const char *runtime_dir;
int name_size, fd;
+   bool path_is_absolute;
+
+   if (name == NULL)
+   name = getenv("WAYLAND_DISPLAY");
+   if (name == NULL)
+   name = "wayland-0";
+
+   path_is_absolute = name[0] == '/';
 
runtime_dir = getenv(&q

Re: [PATCH v2 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-14 Thread Matt Hoosier
On Tue, Nov 14, 2017 at 8:22 AM, Pekka Paalanen  wrote:
> On Fri, 10 Nov 2017 11:20:42 -0600
> Matt Hoosier  wrote:
>
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>>
>> v2 changes:
>>
>> * Added backward incompatibility note to wl_display_connect() manpage.
>> * Rephased wl_display_connect() manpage changes to precisely match actual
>>   changed behavior.
>> * Added mention of new absolute path behavior in wl_display_connect()
>>   doxygen comments.
>> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
>>   protocol documentation.
>>
>> Signed-off-by: Matt Hoosier 
>> ---
>>  doc/man/wl_display_connect.xml| 22 +---
>>  doc/publican/sources/Protocol.xml |  5 -
>>  src/wayland-client.c  | 43 
>> +--
>>  3 files changed, 55 insertions(+), 15 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..7bdfc46 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,30 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY for this 
>> function
>> +  to find it. The name argument specifies the 
>> name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>>WAYLAND_DISPLAY replaces the default value. If
>>WAYLAND_SOCKET is set, this function behaves like
>>wl_display_connect_to_fd with the 
>> file-descriptor
>> -  number taken from the environment variable.
>> +  number taken from the environment variable. If
>> +  WAYLAND_SOCKET is not set and 
>> name
>> +  is NULL and WAYLAND_DISPLAY
>> +  is an absolute path, then the path stored in 
>> WAYLAND_DISPLAY
>> +  is used as the Wayland socket to which the connection is 
>> attempted.
>> +
>> +Support for interpreting WAYLAND_DISPLAY as an
>> +  absolute path is a change in behavior compared to
>> +  wl_display_connect's behavior in versions
>> +  1.14 and older of Wayland. It is no longer guaranteed in versions
>> +  1.15 and higher that the Wayland socket chosen is equivalent to
>> +  manually constructing a socket pathname by concatenating
>> +  XDG_RUNTIME_DIR and WAYLAND_DISPLAY.
>> +  Manual construction of the socket path must account for the
>> +  possibility that WAYLAND_DISPLAY contains an 
>> absolute
>> +  path.
>
> Hi Matt,
>
> this is a bit verbose, but it is correct. 'name' argument could now be
> absolute as well.
>
>>
>>  wl_display_connect_to_fd connects to a 
>> Wayland
>>socket with an explicit file-descriptor. The file-descriptor is 
>> passed
>> diff --git a/doc/publican/sources/Protocol.xml 
>> b/doc/publican/sources/Protocol.xml
>> index ba6b5f1..dbfed06 100644
>> --- a/doc/publican/sources/Protocol.xml
>> +++ b/doc/publican/sources/Protocol.xml
>> @@ -94,7 +94,10 @@
>>The protocol is sent over a UNIX domain stream socket, where the 
>> endpoint
>>usually is named wayland-0
>>(although it can be changed via WAYLAND_DISPLAY
>&

[PATCH v2 wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-10 Thread Matt Hoosier
From: Matt Hoosier 

In order to support system compositor instances, it is necessary to
allow clients' wl_display_connect() to find the compositor's listening
socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
the discussion beginning here:

https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html

This change adjusts the client-side connection logic so that, if
WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
connection attempt is made to just $WAYLAND_DISPLAY rather than
usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.

This change is based on Davide Bettio's submission of the same concept
at:

https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.

v2 changes:

* Added backward incompatibility note to wl_display_connect() manpage.
* Rephased wl_display_connect() manpage changes to precisely match actual
  changed behavior.
* Added mention of new absolute path behavior in wl_display_connect()
  doxygen comments.
* Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
  protocol documentation.

Signed-off-by: Matt Hoosier 
---
 doc/man/wl_display_connect.xml| 22 +---
 doc/publican/sources/Protocol.xml |  5 -
 src/wayland-client.c  | 43 +--
 3 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
index 7e6e05c..7bdfc46 100644
--- a/doc/man/wl_display_connect.xml
+++ b/doc/man/wl_display_connect.xml
@@ -55,14 +55,30 @@
 Description
 wl_display_connect connects to a Wayland socket
   that was previously opened by a Wayland server. The server socket 
must
-  be placed in XDG_RUNTIME_DIR for this function to
-  find it. The name argument specifies the name of
+  be placed in XDG_RUNTIME_DIR or exist at the absolute
+  path referred to by WAYLAND_DISPLAY for this function
+  to find it. The name argument specifies the name 
of
   the socket or NULL to use the default (which is
   "wayland-0"). The environment variable
   WAYLAND_DISPLAY replaces the default value. If
   WAYLAND_SOCKET is set, this function behaves like
   wl_display_connect_to_fd with the 
file-descriptor
-  number taken from the environment variable.
+  number taken from the environment variable. If
+  WAYLAND_SOCKET is not set and name
+  is NULL and WAYLAND_DISPLAY
+  is an absolute path, then the path stored in 
WAYLAND_DISPLAY
+  is used as the Wayland socket to which the connection is 
attempted.
+
+Support for interpreting WAYLAND_DISPLAY as an
+  absolute path is a change in behavior compared to
+  wl_display_connect's behavior in versions
+  1.14 and older of Wayland. It is no longer guaranteed in versions
+  1.15 and higher that the Wayland socket chosen is equivalent to
+  manually constructing a socket pathname by concatenating
+  XDG_RUNTIME_DIR and WAYLAND_DISPLAY.
+  Manual construction of the socket path must account for the
+  possibility that WAYLAND_DISPLAY contains an absolute
+  path.
 
 wl_display_connect_to_fd connects to a Wayland
   socket with an explicit file-descriptor. The file-descriptor is 
passed
diff --git a/doc/publican/sources/Protocol.xml 
b/doc/publican/sources/Protocol.xml
index ba6b5f1..dbfed06 100644
--- a/doc/publican/sources/Protocol.xml
+++ b/doc/publican/sources/Protocol.xml
@@ -94,7 +94,10 @@
   The protocol is sent over a UNIX domain stream socket, where the endpoint
   usually is named wayland-0
   (although it can be changed via WAYLAND_DISPLAY
-  in the environment).
+  in the environment). In the reference implementation, a client whose
+  WAYLAND_DISPLAY is formatted as an absolute path
+  connects to that path as the endpoint, otherwise the implementation
+  searches in XDG_RUNTIME_DIR for the endpoint.
 
 
   Every message is structured as 32-bit words; values are represented in 
the
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 3d7361e..d90cbfb 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -857,9 +857,17 @@ connect_to_socket(const char *name)
socklen_t size;
const char *runtime_dir;
int name_size, fd;
+   bool path_is_absolute;
+
+   if (name == NULL)
+   name = getenv("WAYLAND_DISPLAY");
+   if (name == NULL)
+   name = "wayland-0";
+
+   path_is_absolute = name[0] == '/';
 
runtime_dir = getenv("XDG_RUNTIME_DIR");
-   if (!runtime_dir) {
+   if (!runtime_dir && !path_is_absolute) {
wl_log("error: XDG_RUNTIME_DIR not set in the environment.\n");
 

Re: [PATCH wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-10 Thread Matt Hoosier
Hi,

On Fri, Nov 10, 2017 at 5:37 AM, Pekka Paalanen  wrote:
> On Thu,  9 Nov 2017 09:36:29 -0600
> Matt Hoosier  wrote:
>
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>
> Hi,
>
> Davide, let us know ASAP if you're not happy with the attribution.
>
> Matt, could add your Signed-off-by?
>
> Jonas' comment should be addressed. There are apps that do the
> $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY concatenation themselves and do not
> expect an absolute path. They do that because they need the socket file
> path for e.g. bind-mounting it somewhere else, instead of just
> connecting to it.

Yeah, after a re-reading of Jonas's original message, I agree. That's
now implemented in v2.

>
> FWIW, if we had gone for the additional automatic fallback
> to /run/wayland/socket, those apps would still have the same problem.
> And even if we used a whole different environment variable for absolute
> paths.
>
>> ---
>>  doc/man/wl_display_connect.xml | 11 ---
>>  src/wayland-client.c   | 34 +++---
>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..e86ee26 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,19 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY for this 
>> function
>> +  to find it. The name argument specifies the 
>> name of
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>>WAYLAND_DISPLAY replaces the default value. If
>>WAYLAND_SOCKET is set, this function behaves like
>>wl_display_connect_to_fd with the 
>> file-descriptor
>> -  number taken from the environment variable.
>> +  number taken from the environment variable. If
>> +  WAYLAND_SOCKET is not set and 
>> WAYLAND_DISPLAY
>> +  is an absolute path then name is ignored and 
>> the
>> +  path stored in WAYLAND_DISPLAY is used as the 
>> Wayland
>> +  socket to which the connection is attempted.
>
> WAYLAND_DISPLAY has never before overridden the function argument and I
> believe we should keep it that way. In fact, the code you propose
> already works like this, it's only the documentation here that is
> inaccurate.

Agreed, done.

>
> It would also be good to mention the version, that is, starting from
> libwayland 1.15, WAYLAND_DISPLAY accepts also absolute paths.

Done.

>
>>
>>  wl_display_connect_to_fd connects to a 
>> Wayland
>>socket with an explicit file-descriptor. The file-descriptor is 
>> passed
>> diff --git a/src/wayland-client.c b/src/wayland-client.c
>> index 3d7361e..2263d06 100644
>> --- a/src/wayland-client.c
>> +++ b/src/wayland-client.c
>> @@ -858,8 +858,13 @@ connect_to_socket(const char *name)
>>   const char *runtime_dir;
>>   int name_size, fd;
>>
>> + if (name == NULL)
>> + name = getenv("WAYLAND_DISPLAY");
>> + if (name == NULL)
>> + name = "wayland-0";
>> +
>>   runtime_dir = getenv("XDG_RUNTIME_DIR");
>> - if (!runtime_dir) {
>> + if (!runtime_dir &am

Re: [PATCH wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-10 Thread Matt Hoosier
On Thu, Nov 9, 2017 at 8:53 PM, Jonas Ådahl  wrote:
> On Thu, Nov 09, 2017 at 09:36:29AM -0600, Matt Hoosier wrote:
>> From: Matt Hoosier 
>>
>> In order to support system compositor instances, it is necessary to
>> allow clients' wl_display_connect() to find the compositor's listening
>> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
>> the discussion beginning here:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>>
>> This change adjusts the client-side connection logic so that, if
>> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
>> connection attempt is made to just $WAYLAND_DISPLAY rather than
>> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>>
>> This change is based on Davide Bettio's submission of the same concept
>> at:
>>
>> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>> ---
>>  doc/man/wl_display_connect.xml | 11 ---
>>  src/wayland-client.c   | 34 +++---
>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>
>> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
>> index 7e6e05c..e86ee26 100644
>> --- a/doc/man/wl_display_connect.xml
>> +++ b/doc/man/wl_display_connect.xml
>> @@ -55,14 +55,19 @@
>>  Description
>>  wl_display_connect connects to a Wayland 
>> socket
>>that was previously opened by a Wayland server. The server socket 
>> must
>> -  be placed in XDG_RUNTIME_DIR for this function to
>> -  find it. The name argument specifies the name 
>> of
>> +  be placed in XDG_RUNTIME_DIR or exist at the 
>> absolute
>> +  path referred to by WAYLAND_DISPLAY for this 
>> function
>> +  to find it. The name argument specifies the 
>> name of
>
> As mentioned in another E-mail to the old thread, this change is not
> completely backward compatible and it should be made clear that one
> cannot rely on a previously working valid applications to continue to
> work without adaptations for supporting absolute socket paths.

Thanks. I misunderstood your request earlier. I thought you were just
calling for a very clear description of the situations in which the
absolute path would actually get used. I'm resubmitting momentarily
with the manpage for wl_display_connect() adjusted to note the version
of the Wayland client library this the behavior changes. A warning to
users who do manual construction of the socket path is spelled out.

>
> To repeat, this is for applications (or other things dealing with
> Wayland sockets, such as sandbox runtimes) that do their own processing
> of the content of WAYLAND_DISPLAY.
>
>
> Jonas
>
>>the socket or NULL to use the default (which 
>> is
>>"wayland-0"). The environment variable
>>WAYLAND_DISPLAY replaces the default value. If
>>WAYLAND_SOCKET is set, this function behaves like
>>wl_display_connect_to_fd with the 
>> file-descriptor
>> -  number taken from the environment variable.
>> +  number taken from the environment variable. If
>> +  WAYLAND_SOCKET is not set and 
>> WAYLAND_DISPLAY
>> +  is an absolute path then name is ignored and 
>> the
>> +  path stored in WAYLAND_DISPLAY is used as the 
>> Wayland
>> +  socket to which the connection is attempted.
>>
>>  wl_display_connect_to_fd connects to a 
>> Wayland
>>socket with an explicit file-descriptor. The file-descriptor is 
>> passed
>> diff --git a/src/wayland-client.c b/src/wayland-client.c
>> index 3d7361e..2263d06 100644
>> --- a/src/wayland-client.c
>> +++ b/src/wayland-client.c
>> @@ -858,8 +858,13 @@ connect_to_socket(const char *name)
>>   const char *runtime_dir;
>>   int name_size, fd;
>>
>> + if (name == NULL)
>> + name = getenv("WAYLAND_DISPLAY");
>> + if (name == NULL)
>> + name = "wayland-0";
>> +
>>   runtime_dir = getenv("XDG_RUNTIME_DIR");
>> - if (!runtime_dir) {
>> + if (!runtime_dir && (name[0] != '/')) {
>>   wl_log("error: XDG_RUNTIME_DIR not set in the environment.\n");
>>   /* to prevent programs reporting
>>* "failed to create display: Success" */
>> @@ -867,25 +872,32 @@ connect_to_soc

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-09 Thread Matt Hoosier
On Wed, Nov 8, 2017 at 2:16 AM, Pekka Paalanen  wrote:
> On Tue, 7 Nov 2017 19:42:42 +
> Daniel Stone  wrote:
>
>> Hi,
>>
>> On 3 November 2017 at 07:33, Pekka Paalanen  wrote:
>
>> > - when the current socket search fails, add one more place to look
>> >   in: /run/wayland/$WAYLAND_DISPLAY with the default "wayland-0" if
>> >   WAYLAND_DISPLAY is not set.
>>
>> NAK. It's a surprising change in behaviour which could catch people
>> who could reasonably expect other behaviour. It doesn't play well with
>> each session expecting to be able to create a socket called
>> 'wayland-0', which is separated by $XDG_RUNTIME_DIR being different
>> per session.
>
> Hi all,
>
> obviously the /run/wayland option would be most useful for systems that
> will only ever run a single graphical session at a time, in other words,
> appliances, maybe tablets and phones, but not traditional/desktop
> computers.
>
> For the record, I'd never have libwayland-server automatically create
> the /run/wayland socket.
>
> It seems concensus is turning towards supporting absolute paths in
> WAYLAND_DISPLAY, with the /run/wayland proposal being more
> controversial. As the latter can be trivially realized with the former,
> I think we have a conclusion: absolute path support is good.
>
> I would hope someone will resurrect the patch and send it, I can review
> it for one, I'd like another maintainer Reviewed-by on it as well, and
> please do explicitly post your Acked-by's for the patch.

Done; see 
https://lists.freedesktop.org/archives/wayland-devel/2017-November/035745.html
.

>
> If the /run/wayland proposal is still really wanted, it can be raised
> again after we have the absolute path support in WAYLAND_DISPLAY.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] client: Allow absolute paths in WAYLAND_DISPLAY

2017-11-09 Thread Matt Hoosier
From: Matt Hoosier 

In order to support system compositor instances, it is necessary to
allow clients' wl_display_connect() to find the compositor's listening
socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
the discussion beginning here:

https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html

This change adjusts the client-side connection logic so that, if
WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
connection attempt is made to just $WAYLAND_DISPLAY rather than
usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.

This change is based on Davide Bettio's submission of the same concept
at:

https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
---
 doc/man/wl_display_connect.xml | 11 ---
 src/wayland-client.c   | 34 +++---
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
index 7e6e05c..e86ee26 100644
--- a/doc/man/wl_display_connect.xml
+++ b/doc/man/wl_display_connect.xml
@@ -55,14 +55,19 @@
 Description
 wl_display_connect connects to a Wayland socket
   that was previously opened by a Wayland server. The server socket 
must
-  be placed in XDG_RUNTIME_DIR for this function to
-  find it. The name argument specifies the name of
+  be placed in XDG_RUNTIME_DIR or exist at the absolute
+  path referred to by WAYLAND_DISPLAY for this function
+  to find it. The name argument specifies the name 
of
   the socket or NULL to use the default (which is
   "wayland-0"). The environment variable
   WAYLAND_DISPLAY replaces the default value. If
   WAYLAND_SOCKET is set, this function behaves like
   wl_display_connect_to_fd with the 
file-descriptor
-  number taken from the environment variable.
+  number taken from the environment variable. If
+  WAYLAND_SOCKET is not set and 
WAYLAND_DISPLAY
+  is an absolute path then name is ignored and the
+  path stored in WAYLAND_DISPLAY is used as the Wayland
+  socket to which the connection is attempted.
 
 wl_display_connect_to_fd connects to a Wayland
   socket with an explicit file-descriptor. The file-descriptor is 
passed
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 3d7361e..2263d06 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -858,8 +858,13 @@ connect_to_socket(const char *name)
const char *runtime_dir;
int name_size, fd;
 
+   if (name == NULL)
+   name = getenv("WAYLAND_DISPLAY");
+   if (name == NULL)
+   name = "wayland-0";
+
runtime_dir = getenv("XDG_RUNTIME_DIR");
-   if (!runtime_dir) {
+   if (!runtime_dir && (name[0] != '/')) {
wl_log("error: XDG_RUNTIME_DIR not set in the environment.\n");
/* to prevent programs reporting
 * "failed to create display: Success" */
@@ -867,25 +872,32 @@ connect_to_socket(const char *name)
return -1;
}
 
-   if (name == NULL)
-   name = getenv("WAYLAND_DISPLAY");
-   if (name == NULL)
-   name = "wayland-0";
-
fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
if (fd < 0)
return -1;
 
memset(&addr, 0, sizeof addr);
addr.sun_family = AF_LOCAL;
-   name_size =
-   snprintf(addr.sun_path, sizeof addr.sun_path,
-"%s/%s", runtime_dir, name) + 1;
+   if (name[0] != '/') {
+   name_size =
+   snprintf(addr.sun_path, sizeof addr.sun_path,
+"%s/%s", runtime_dir, name) + 1;
+   } else {
+   /* absolute path */
+   name_size =
+   snprintf(addr.sun_path, sizeof addr.sun_path,
+"%s", name) + 1;
+   }
 
assert(name_size > 0);
if (name_size > (int)sizeof addr.sun_path) {
-   wl_log("error: socket path \"%s/%s\" plus null terminator"
-  " exceeds 108 bytes\n", runtime_dir, name);
+   if (name[0] != '/') {
+   wl_log("error: socket path \"%s/%s\" plus null 
terminator"
+  " exceeds %i bytes\n", runtime_dir, name, (int) 
sizeof(addr.sun_path));
+   } else {
+   wl_log("error: socket path \"%s\" plus null terminator"
+  " exceeds %i bytes\n", name, (int) 
sizeof(addr.sun_path));
+ 

Re: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell

2017-11-07 Thread Matt Hoosier
On Tue, Nov 7, 2017 at 11:58 AM, Quentin Glidic
 wrote:
> On 11/7/17 6:01 PM, Matt Hoosier wrote:
>>
>> Hi Pekka,
>>
>> On Wed, Oct 25, 2017 at 10:09 AM, Ucan, Emre (ADITG/ESB)
>>  wrote:
>>>
>>> Actually,  IMO ivi-shell is not a proper wayland compositor.
>>> Because it is violating wayland protocol by not supporting wl_shell
>>> interface.
>
>
> The wl_shell protocol was designed for the desktop use case. So not
> supporting it is perfectly fine on non-desktop.
>
>
>>> Therefore, we have to at least support wl_shell interface in
>>> ivi-shell. Why not support it via libweston-desktop ?
>>
>>
>> I'm wondering if you have any thoughts on this one specific point
>> that Emre made. I know there's a lot of heartburn over the inclusion
>> of wl_shell into the core protocol, and you wouldn't do it that way
>> if that decision were getting made today.
>>
>> But given the history that actually happened, is there a reason not
>> to go ahead and allow the ivi-shell to implement wl_shell simply on
>> the grounds that it is part of the defined core protocol? I think
>> that some potentially reasonable answers were made above to your
>> concerns that the API offered by wl_shell targeted toward desktops
>> wouldn't be meaningful on an IVI system.  All the mandatory
>> operations seem to be possible to support,
>
>
> Except the current patch doesn’t say that.

I'm unclear whether you're objecting to the verbage in the changeset
frontmatter. Are you just calling for Michael to state this ground in
the commit message?

> xdg_shell allows the compositor
> to ignore the fullscreen/maximize requests. libweston-desktop API was
> designed around xdg_shell, with best effort for wl_shell (and I am in the
> process of fixing that).

I'm trying to figure out whether Pekka is unconditionally opposed to
adding any desktop-ish API support to the IVI compositor. For the
moment (see my "magic wand" comment earlier), I think it's useful to
suppose that using libweston did not commit the shell to supporting
both xdg and wl_shell as a packaged deal. If there's still no consent
toward this patch series even with that liberating assumption, then no
amount of implementation adjustment would probably be found
persuasive.

> With wl_shell, the compositor cannot deny the
> client the fullscreen or maximized state.

Are you pointing to a behavior in Michael's proposed implementation
that dishonors a request from a wl_shell client to enter fullscreen or
maximized state? If that's the case, maybe he's willing to make a
guarantee in the IVI shell a weston_desktop_surface always gets such
states honored.

>
>
>> and the IVI shell just needs to come up with sensible definitions
>> (i.e., documented for its users) about how the anonymous wl_shell
>> clients' surfaces will be integrated with the explicit IVI clients.
>>
>> Note, I'm not for the moment trying to expand this line of reasoning into
>> a grounds for justifying the support of xdg-shell. That
>> protocol is (deliberately) not part of the core, and I understand
>> that. If a magic wand were to be waved and use of libweston-desktop
>> didn't automatically mean that xdg-shell is supported too, would that
>> be tolerable?
>
>
> xdg_shell was not added to core to avoid the wl_shell situation. We cannot
> drop wl_shell support because it’s in core wayland.xml, and it’s hurting us
> already, because wl_shell clients will always prevent a full migration to
> xdg_shell. We are lucky that libweston-desktop (and others) doesn’t need too
> much code to support it.
>
>
> Thanks,
>
> --
>
> Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell

2017-11-07 Thread Matt Hoosier
Hi Pekka,

On Wed, Oct 25, 2017 at 10:09 AM, Ucan, Emre (ADITG/ESB)
 wrote:
> Actually,  IMO ivi-shell is not a proper wayland compositor. Because it is 
> violating wayland protocol by not supporting wl_shell interface.
>
> Therefore, we have to at least support wl_shell interface in ivi-shell. Why 
> not support it via libweston-desktop ?

I'm wondering if you have any thoughts on this one specific point that
Emre made. I know there's a lot of heartburn over the inclusion of
wl_shell into the core protocol, and you wouldn't do it that way if
that decision were getting made today.

But given the history that actually happened, is there a reason not to
go ahead and allow the ivi-shell to implement wl_shell simply on the
grounds that it is part of the defined core protocol? I think that
some potentially reasonable answers were made above to your concerns
that the API offered by wl_shell targeted toward desktops wouldn't be
meaningful on an IVI system.  All the mandatory operations seem to be
possible to support, and the IVI shell just needs to come up with
sensible definitions (i.e., documented for its users) about how the
anonymous wl_shell clients' surfaces will be integrated with the
explicit IVI clients.

Note, I'm not for the moment trying to expand this line of reasoning
into a grounds for justifying the support of xdg-shell. That protocol
is (deliberately) not part of the core, and I understand that. If a
magic wand were to be waved and use of libweston-desktop didn't
automatically mean that xdg-shell is supported too, would that be
tolerable?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-07 Thread Matt Hoosier
Hi Pekka,

What do you think is a good amount of time to allow for people to
respond to your call for acks/nacks?

On Fri, Nov 3, 2017 at 8:33 AM, Carsten Haitzler  wrote:
> On Fri, 3 Nov 2017 12:47:39 +0200 Pekka Paalanen  said:
>
>> On Fri, 3 Nov 2017 11:04:27 +0100 (CET)
>> Jan Engelhardt  wrote:
>>
>> > On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
>> > >
>> > >> Summary of (individual) proposals follows.
>> > >>
>> > >> >- modify WAYLAND_DISPLAY to support absolute paths which overrides
>> > >> >  any search paths
>> > >>
>> > >>  - introduce new WAYLAND_SOCKET
>> > >>  - modify WAYLAND_DISPLAY to reject '/'
>> > >
>> > >What would be the functional difference to WAYLAND_DISPLAY accepting
>> > >absolute paths? Why would a different environment variable make a
>> > >difference?
>> >
>> > Well because you cannot establish for certain that people have or have not
>> > already used WAYLAND_DISPLAY=/newsock in the concatenation sense.
>> >
>> > (Depending on who you ask and how much weight they give to it,
>> > breaking application interfaces is out of the question. That's all.)
>>
>> Ah, that, ok. I thought this was about the security stuff you referred
>> to. But given the same rationale, we cannot forbid / in WAYLAND_DISPLAY
>> either.
>
> security here i think is a red herring. i can effectively trick
> libwayland-client to connect to an abs path by setting XDG_RUNTIME_DIR AND
> WAYLAND_DISPLAY. so ... effectively same thing. it will force all xdg runtime
> stuff to be in that same dir... but i think abs path for wl display
> specifically being a security issue is a red herring, unless there is 
> something
> none of us can think of. then we have the problem already with runtime dir env
> var and wl display too like above.
>
> --
> - Codito, ergo sum - "I code, therefore I am" --
> Carsten Haitzler - ras...@rasterman.com
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v12 27/40] compositor-drm: Add modifiers to GBM dmabuf import

2017-11-03 Thread Matt Hoosier
On Tue, Sep 26, 2017 at 12:16 PM, Daniel Stone  wrote:
> Add support for the GBM_BO_IMPORT_FD_MODIFIER path, which allows us to
> import multi-plane dmabufs, as well as format modifiers.
>
> Signed-off-by: Daniel Stone 
> ---
>  configure.ac   |   3 -
>  libweston/compositor-drm.c | 181 
> +
>  2 files changed, 133 insertions(+), 51 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f09d0e04..3996c80c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -203,9 +203,6 @@ if test x$enable_drm_compositor = xyes; then
>PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
> [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic 
> API])],
> [AC_MSG_WARN([libdrm does not support atomic modesetting, 
> will omit that capability])])
> -  PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 10.2],
> -   [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports dmabuf 
> import])],
> -   [AC_MSG_WARN([gbm does not support dmabuf import, will 
> omit that capability])])
>  fi
>
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7557ef55..65934a01 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -273,6 +273,7 @@ struct drm_mode {
>  enum drm_fb_type {
> BUFFER_INVALID = 0, /**< never used */
> BUFFER_CLIENT, /**< directly sourced from client */
> +   BUFFER_DMABUF, /**< imported from linux_dmabuf client */
> BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */
> BUFFER_GBM_SURFACE, /**< internal EGL rendering */
> BUFFER_CURSOR, /**< internal cursor buffer */
> @@ -928,6 +929,118 @@ drm_fb_ref(struct drm_fb *fb)
> return fb;
>  }
>
> +static void
> +drm_fb_destroy_dmabuf(struct drm_fb *fb)
> +{
> +   /* We deliberately do not close the GEM handles here; GBM manages
> +* their lifetime through the BO. */
> +   gbm_bo_destroy(fb->bo);
> +   drm_fb_destroy(fb);
> +}
> +
> +static struct drm_fb *
> +drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer *dmabuf,
> +  struct drm_backend *backend, bool is_opaque)
> +{
> +   struct drm_fb *fb;
> +   struct gbm_import_fd_data import_legacy = {
> +   .width = dmabuf->attributes.width,
> +   .height = dmabuf->attributes.height,
> +   .format = dmabuf->attributes.format,
> +   .stride = dmabuf->attributes.stride[0],
> +   .fd = dmabuf->attributes.fd[0],
> +   };
> +   struct gbm_import_fd_modifier_data import_mod = {
> +   .width = dmabuf->attributes.width,
> +   .height = dmabuf->attributes.height,
> +   .format = dmabuf->attributes.format,
> +   .num_fds = dmabuf->attributes.n_planes,
> +   .modifier = dmabuf->attributes.modifier[0],
> +   };
> +   int i;
> +
> +/* XXX: TODO:
> + *
> + * Currently the buffer is rejected if any dmabuf attribute
> + * flag is set.  This keeps us from passing an inverted /
> + * interlaced / bottom-first buffer (or any other type that may
> + * be added in the future) through to an overlay.  Ultimately,
> + * these types of buffers should be handled through buffer
> + * transforms and not as spot-checks requiring specific
> + * knowledge. */
> +   if (dmabuf->attributes.flags)
> +   return NULL;
> +
> +   fb = zalloc(sizeof *fb);
> +   if (fb == NULL)
> +   return NULL;
> +
> +   fb->refcnt = 1;
> +   fb->type = BUFFER_DMABUF;
> +
> +   memcpy(import_mod.fds, dmabuf->attributes.fd, sizeof(import_mod.fds));
> +   memcpy(import_mod.strides, dmabuf->attributes.stride,
> +  sizeof(import_mod.fds));
> +   memcpy(import_mod.offsets, dmabuf->attributes.offset,
> +  sizeof(import_mod.fds));
> +
> +   if (dmabuf->attributes.modifier[0] != DRM_FORMAT_MOD_INVALID) {
> +   fb->bo = gbm_bo_import(backend->gbm, 
> GBM_BO_IMPORT_FD_MODIFIER,
> +  &import_mod,
> +  GBM_BO_USE_SCANOUT);
> +   } else {
> +   fb->bo = gbm_bo_import(backend->gbm, GBM_BO_IMPORT_FD,
> +  &import_legacy,
> +  GBM_BO_USE_SCANOUT);
> +   }
> +
> +   if (!fb->bo)
> +   goto err_free;
> +
> +   fb->width = dmabuf->attributes.width;
> +   fb->height = dmabuf->attributes.height;
> +   memcpy(fb->strides, dmabuf->attributes.stride, sizeof(fb->strides));
> +   memcpy(fb->offsets, dmabuf->attributes.offset, sizeof(fb->offsets));
> +   fb->format = pixel_format_get_info(dmabuf->attributes.format);
> +   //fb->modifier = dmabuf->attributes.modifier;
> +   fb->size = 0;
> +   fb->fd = backend->d

Re: [PATCH weston v12 00/40] Atomic modesetting

2017-11-03 Thread Matt Hoosier
On Fri, Nov 3, 2017 at 2:46 AM, Pekka Paalanen  wrote:
>
> On Thu, 2 Nov 2017 14:13:42 -0500
> Matt Hoosier  wrote:
>
> > What sort of testing on this series would be most helpful? I figure that
> > you and pq have most of the code review covered, so the main
contribution I
> > can make with the hardware available to me is to exercise this on
> > consumer-grade systems like Macbooks with Intel display controllers. Are
> > there are specific use-cases that will put the most stress on your new
> > stuff? I assume that doing some sub-fullscreen OpenGL windows will kick
in
> > the overlay selections to test that out.
>
> Hi,
>
> from my point of view, output hotplug is always a pain to test on DRM,
> so I would appreciate help with that. This kind of cases:
>
> - on a running compositor, add an output with hotplug
> - on a running compositor, remove an output with hot-unplug
> - hot-unplug all outputs, the compositor should remain running
> - after hot-unplugging all outputs, plug them back in
> - start the compositor without any outputs, then hotplug some
> - hotplug/unplug tests with MST, so that we have DRM connectors
>   appearing and disappearing
>
> At all times, weston-info should reflect the actual number of outputs,
> 0..N.
>
> All the cases could be taken further by hotplugging and hot-unplugging
> while VT-switched away. It is also possible that some of this is
> already broken in upstream master, so if you encounter failures, it
> would be good to know if it was already broken.
>
> Those are my wishes, Daniel probably has others. Any testing reports
> at all would be awesome.
>
>
> Thanks,
> pq


Okay, I'll see how many of those cases I can manage to exercise.

At the moment, I already noticed what looks like some sort of failure to
handle wl_surface damage regions as submitted by weston-simple-egl. (Only
part of the triangle refreshes while the remainder is stuck at its initial
frame's contents.)

It doesn't look to me like this has anything to do with overlay usage. The
contents of /sys/kernel/debug/dri/0/i915_display_info show:

  CRTC info
  -
  CRTC 31: pipe: A, active=yes, (size=2560x1600), dither=no, bpp=24
  fb: 72, pos: 0x0, size: 2560x1600
  encoder 46: type: DDI A, connectors:
  connector 47: type: eDP-1, status: connected, mode:
  id 0:"2560x1600" freq 60 clock 268500 hdisp 2560 hss 2608
hse 2640 htot 2720 vdisp 1600 vss 1603 vse 1609 vtot 1646 type 0x48 flags
0x9
  cursor visible? no, position (0, 0), size 0x0, addr 0x,
active? no
  No scalers available on this platform
  --Plane id 25: type=PRI, crtc_pos=   0x   0, crtc_size=2560x1600,
src_pos=0.x0., src_size=2560.x1600., format=XR24
little-endian (0x34325258), rotation=0 (0x0001)
  --Plane id 27: type=OVL, crtc_pos=   0x   0, crtc_size=   0x   0,
src_pos=0.x0., src_size=0.x0., format=N/A, rotation=0
(0x0001)
  --Plane id 29: type=CUR, crtc_pos=   0x   0, crtc_size=   0x   0,
src_pos=0.x0., src_size=0.x0., format=N/A, rotation=0
(0x0001)


So I think that everything is going through the primary plane.

(This behavior isn't seen in the last common point between master and
Daniel's wip/2017-10/atomic-v13 branch head.)

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Matt Hoosier
On Fri, Nov 3, 2017 at 2:33 AM, Pekka Paalanen  wrote:

> On Thu, 2 Nov 2017 10:07:28 -0500
> Matt Hoosier  wrote:
>
> > On Thu, Nov 2, 2017 at 3:36 AM, Pekka Paalanen 
> wrote:
> >
> > > On Wed, 1 Nov 2017 13:44:50 -0500
> > > Matt Hoosier  wrote:
> > >
>
> > > >
> > > > I'd like to call attention to another difficult point in using
> Wayland
> > > for
> > > > a device that acts more like a smartphone than a desktop: the main
> logic
> > > in
> > > > wl_display_connect() always attempts to contact a server socket
> living at
> > > > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
>
> Hi,
>
> I rewrote the subject with the hope to raise more interest, being more
> specific to the proposals.
>
> > > I can also imagine that it may not be feasible to create
> > > application-user specific sockets for everything, so there could be a
> > > need for a common socket file somewhere that is not tied to
> > > XDG_RUNTIME_DIR. With a good justification written down, I suspect that
> > > would be fine. We just need to figure out the details on how to do that
> > > exactly.
> > >
> > > Modifying the meaning of WAYLAND_DISPLAY environment variable to
> > > support also absolute paths has been proposed before IIRC. Maybe
> > > resurrecting that work could be a way forward? Can anyone see a problem
> > > with that?
> > >
> >
> > This would definitely work, so I don't object if this is the preference
> of
> > other reviewers. I would prefer (for the reasons coming below) the
> > /run/wayland/$WAYLAND_DISPLAY suggestion though.
> >
> > One note about this: this would contain a subtle change in behavior to
> > existing users of $WAYLAND_DISPLAY. If somebody sets
> > WAYLAND_DISPLAY="/wayland-0" currently, this works okay. The
> concatenation
> > logic in wl_display_connect() results in a string
> > "/run/user///wayland-0", which is valid despite the duplicate '/'.
> If
> > $WAYLAND_DISPLAY is now examined for absolute-pathiness, the logic would
> > probably see "/wayland-0" as an absolute path reference, and the
> connection
> > attempt would fail.
>
> While this is true, I don't think it is a blocker unless we find out
> about tools or compositors doing so.
>
>
> > >
> > > The suggestion to automatically look for a fallback socket
> > > at /run/wayland/$WAYLAND_DISPLAY with WAYLAND_DISPLAY defaulting to
> > > "wayland-0" sounds reasonable to me, but I wouldn't feel comfortable
> > > making that review decision alone. I do know some people are eager to
> > > avoid mandatory environment variables if something can be found by
> > > convention.
> > >
> >
> > This would be my preference. The fewer tweaks to environment variables
> are
> > required, the better in my opinion.
>
> Right, so there is interest to both ideas, and I don't see them as
> mutually exclusive. I believe one can also write a good justification
> for each, so now I'd be looking for reasons why either might be
> unwanted and acks for each, so that we get enough buy-in to "bless" the
> behavioural change in libwayland-client.
>
> People, give your acks/nacks for the two ideas, please:
>
> - modify WAYLAND_DISPLAY to support absolute paths which overrides
>   any search paths
>

Ack


>
> - when the current socket search fails, add one more place to look
>   in: /run/wayland/$WAYLAND_DISPLAY with the default "wayland-0" if
>   WAYLAND_DISPLAY is not set.
>

Ack


>
>
> The last proposed patch for absolute paths is probably
> https://lists.freedesktop.org/archives/wayland-devel/2015-
> August/023838.html
> which looks like it got ignored mostly because of a damaged submission
> and mixed-up (nowadays probably unwanted) server-side changes. The
> patch also lacks the rationale and justification in the commit message.
> This is FYI for anyone wanting to carry on that work.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v12 00/40] Atomic modesetting

2017-11-02 Thread Matt Hoosier
What sort of testing on this series would be most helpful? I figure that
you and pq have most of the code review covered, so the main contribution I
can make with the hardware available to me is to exercise this on
consumer-grade systems like Macbooks with Intel display controllers. Are
there are specific use-cases that will put the most stress on your new
stuff? I assume that doing some sub-fullscreen OpenGL windows will kick in
the overlay selections to test that out.

On Thu, Nov 2, 2017 at 2:05 AM, Ucan, Emre (ADITG/ESB)  wrote:

> Hi Daniel,
>
> Your proposal is exactly what I thought, how it should be.
> I am looking forward for next revision of patches.
>
> Best regards
>
> Emre Ucan
> Engineering Software Base (ADITG/ESB)
>
> Tel. +49 5121 49 6937
>
> > -Original Message-
> > From: Daniel Stone [mailto:dan...@fooishbar.org]
> > Sent: Mittwoch, 1. November 2017 15:14
> > To: Ucan, Emre (ADITG/ESB)
> > Cc: wayland-devel@lists.freedesktop.org
> > Subject: Re: [PATCH weston v12 00/40] Atomic modesetting
> >
> > Hi Emre.
> >
> > On 1 November 2017 at 11:56, Ucan, Emre (ADITG/ESB)
> >  wrote:
> > > Is this the latest WIP branch to test "
> > https://gitlab.collabora.com/daniels/weston/commits/wip/2017-10/atomic-
> > v13" ?
> >
> > Right you are.
> >
> > > In my opinion, it would easier to review/test your patches if you can
> > separate them in multiple patch series.
> > >
> > > For example, you can send at first up to "compositor-drm: Atomic
> > modesetting support". Commit message states that it enables atomic API
> > support for weston.
> > > Other features like GBM_BO_IMPORT_FD_MODIFIER support are nice to
> > have but not a hard requirement of atomic modesetting support.
> > >
> > > What do you think ?
> >
> > It's a reasonable idea, but in practice the two aren't completely
> > independent. The reason GBM_BO_IMPORT_FD_MODIFIER was tied up with
> > this is that it relies quite heavily on changes made to drm_fb which
> > have now been merged, but were previously part of the atomic series.
> > I've been considering pulling those out separately, but on the other
> > hand there are quite large conflicts doing so: before the 'helper'
> > commits, there are two separate GBM import paths for primary/scanout
> > and overlay planes, which only get unified inside the atomic series.
> >
> > My current thinking is:
> >   * everything up to 'atomic modesetting support' is qutie
> > self-contained, largely reviewed, and should hopefully be very very
> > close to landing by the time I can send out a new revision next week
> > (been busy with internal stuff & travel recently)
> >   * once that's landed, everything up to 'Add modifiers to GBM dmabuf
> > import', and possibly including 'Support plane IN_FORMATS' + 'Support
> > modifiers with GBM' can be considered as one independent series
> > (though will need a non-trivial rebase) which should be quite easy to
> > review
> >   * the rest of the code dealing with plane assignments (up to 'Enable
> > planes for atomic') can be considered another separate series; though
> > there are a couple of bugfixes in there, the rest is more complex and
> > difficult
> >
> > I think it makes the most sense to work through like that in order. Of
> > course if you have any other ideas or priorities, I'd be really
> > interested to hear - anything which makes it easier to review is
> > obviously good! :)
> >
> > Cheers,
> > Daniel
> ___
> 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: Enabling Android-style per application user ids

2017-11-02 Thread Matt Hoosier
On Thu, Nov 2, 2017 at 3:36 AM, Pekka Paalanen  wrote:

> On Wed, 1 Nov 2017 13:44:50 -0500
> Matt Hoosier  wrote:
>
> > There has been a surge of recent interest in the details of using Wayland
> > on nontraditional systems that don't have the normal notion of human-user
> > login sessions. Discussions are ongoing about the details of how to
> manage
> > the allocation of TTYs and Weston's vision of the best practices for
> > assigning Unix ownership modes to the hardware devices. Set those aside
> for
> > the moment though.
> >
> > I'd like to call attention to another difficult point in using Wayland
> for
> > a device that acts more like a smartphone than a desktop: the main logic
> in
> > wl_display_connect() always attempts to contact a server socket living at
> > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
> >
> > This creates a problem for a device that isolates each application in a
> > different user ID. XDG_RUNTIME_DIR is expressly defined as a directory
> > which is private to its user. The system must either abusively point
> > numerous users' XDG_RUNTIME_DIR's all at the same path, or use an
> > out-of-band hack such as hardlinking each application uid's
> > /run/user//wayland-0 path to the true server socket
> > /run/user//wayland-0.
> >
> > As a consequence, it is difficult to implement a security model similar
> to
> > Android's, on a device that's adopted Wayland.
> >
> > I can imagine several schemes for allowing a generic Wayland client
> program
> > to transparently find some systemwide compositor instance. The client
> could
> > automatically attempt to open /var/run/${WAYLAND_DISPLAY} if
> > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY} doesn't exist. That approach could
> > also be modified to require the client to explicitly opt-in by setting
> some
> > environment variable such as WAYLAND_RUNTIME_DIR that is treated as a
> > fallback if XDG_RUNTIME_DIR contains no server socket.
> >
> > Thoughts? It's very encouraging to see all the technical capabilities
> that
> > allow Wayland/Weston to be a credible option for making embedded devices
> > with great graphics support, but there are a few rough edges such as
> these
> > that make deployment a challenge.
>
> Hi Matt,
>
> this is all true. There have been suggestions before about adding
> various forms of environment variables to libwayland-client and
> libwayland-server to divert the socket creation and finding, but they
> died in either lack of review, interest, or having a hacky feel to
> them.
>
> Personally I recall being not fond of the server side environment
> variables at least, because libwayland-server has API for creating
> sockets in arbitrary places already, you just need to write your
> compositor to use it.
>
> The API in question is the wl_display_add_socket*() set of functions.
> It is even possible to configure the sockets in a systemd unit file and
> have Weston use those automatically via the systemd-notify.so plugin.
> At least I see the code in there.
>

Agreed. The server side here is not the problem. The existing systemd
integration already makes it very simple to provide as many different
listening-socket locations as needed.


>
> But that was about the server side, while the real problem is in the
> client side, as you say.
>

Exactly.


>
> There is one more possibility in addition to having a single socket
> somewhere findable and usable for all: creating application-user
> specific sockets from the compositor or from systemd before launching
> the compositor.
>
> If you created application-user specific sockets and your
> application-users are properly isolated, you could identify the
> connecting application(-user) by the socket it is coming in through.
> That way the server could authenticate different client groups. The
> catch is that I don't think libwayland-server has the API to query how
> a wl_client got connected, so that's something we would need to work on
> if it seems useful.
>
> I can also imagine that it may not be feasible to create
> application-user specific sockets for everything, so there could be a
> need for a common socket file somewhere that is not tied to
> XDG_RUNTIME_DIR. With a good justification written down, I suspect that
> would be fine. We just need to figure out the details on how to do that
> exactly.
>
> Modifying the meaning of WAYLAND_DISPLAY environment variable to
> support also absolute paths has been proposed before IIRC. Maybe
> resurrecting that work could be a way forward? Can anyone see a problem
> with that?
>

This wo

Enabling Android-style per application user ids

2017-11-01 Thread Matt Hoosier
There has been a surge of recent interest in the details of using Wayland
on nontraditional systems that don't have the normal notion of human-user
login sessions. Discussions are ongoing about the details of how to manage
the allocation of TTYs and Weston's vision of the best practices for
assigning Unix ownership modes to the hardware devices. Set those aside for
the moment though.

I'd like to call attention to another difficult point in using Wayland for
a device that acts more like a smartphone than a desktop: the main logic in
wl_display_connect() always attempts to contact a server socket living at
${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.

This creates a problem for a device that isolates each application in a
different user ID. XDG_RUNTIME_DIR is expressly defined as a directory
which is private to its user. The system must either abusively point
numerous users' XDG_RUNTIME_DIR's all at the same path, or use an
out-of-band hack such as hardlinking each application uid's
/run/user//wayland-0 path to the true server socket
/run/user//wayland-0.

As a consequence, it is difficult to implement a security model similar to
Android's, on a device that's adopted Wayland.

I can imagine several schemes for allowing a generic Wayland client program
to transparently find some systemwide compositor instance. The client could
automatically attempt to open /var/run/${WAYLAND_DISPLAY} if
${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY} doesn't exist. That approach could
also be modified to require the client to explicitly opt-in by setting some
environment variable such as WAYLAND_RUNTIME_DIR that is treated as a
fallback if XDG_RUNTIME_DIR contains no server socket.

Thoughts? It's very encouraging to see all the technical capabilities that
allow Wayland/Weston to be a credible option for making embedded devices
with great graphics support, but there are a few rough edges such as these
that make deployment a challenge.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Automatically choosing a VT for DRM compositor?

2017-10-31 Thread Matt Hoosier
On Mon, Oct 30, 2017 at 9:45 AM, Pekka Paalanen  wrote:

> On Mon, 30 Oct 2017 13:45:24 +
> "Ucan, Emre (ADITG/ESB)"  wrote:
>
> > Hello Matt and Pekka,
> >
> > Actually, we are OK with running weston with a static tty within a
> > systemd service file, but launcher_direct_connect() function is
> > failing when user is not root.
> >
> > Therefore, we patched weston to remove the expilicit check for root
> > user.  I am planning to send this patch to the mailing list.
> >
> > We also updated ownership of drm and tty devices accordingly, so that
> > weston can run as non-root user.
>
> Hi,
>
> IMO that is bad practice. I'll explain why below, since Matt had a
> similar question. There is a reason why launcher-direct requires root.
> It has not been written for anything else.
>
> > > -Original Message-
> > > From: wayland-devel [mailto:wayland-devel-
> > > boun...@lists.freedesktop.org] On Behalf Of Matt Hoosier
> > > Sent: Montag, 30. Oktober 2017 14:28
> > > To: Pekka Paalanen
> > > Cc: wayland mailing list
> > > Subject: Re: Automatically choosing a VT for DRM compositor?
> > >
> > > On Mon, Oct 30, 2017 at 5:10 AM, Pekka Paalanen
> > >  wrote:
> > >
>
> > >
> > > Does this mean you would only cater for the "run weston as
> > > root" case? ;-)
> > >
> > > No, that wasn't the intent. Running a DRM instance of Weston as a
> > > non-root user doesn't require anything really more than adjusting
> > > ownership on the /dev/tty* character devices so that the VT can be
> > > configured. I.e. a user 'weston' would need added to the 'tty' Unix
> > > group to have permission to run ioctl(/dev/tty0, VT_OPENQRY) and
> > > would also need assigned ownership of /dev/tty[0...7] in order to
> > > do KDSETMODE. Nobody would want to do that on the desktop because
> > > of the security implications, but that seems fine to do on an
> > > embedded system.
>
> You need elevated privileges for:
> - the tty unless already owning the session
> - the DRM KMS device node and DRM master
> - the input devices
>
> All these are preferably handled by an agent like weston-launch or
> logind, because giving weston and therefore usually all graphical
> applications access to them is a security risk. Particularly input
> devices: if Weston can open them, by default there is nothing to
> prevent random apps opening them as well.
>
> You are on embedded, you audit your software, you don't care about
> the above security issues because you don't have input devices or
> no-one can run arbitrary programs or whatever. You can invent your own
> security setups by running weston as one user and jumping through hoops
> to run apps as other users. Running apps as a different user does have
> its merits.
>
> But why would you bother inventing an inferior setup if you use systemd
> already and therefore you have logind where all this has already been
> implemented with security in mind?
>

This discussion seems to hinge on whether logind can be made to work for
Weston as spawned by system units (rather than user-session units). I find
that sd_pid_get_session() has always failed for me (leading to the "logind
not running in a systemd session" Weston log entry).

The documentation on sd_pid_get_session() says:

Note that not all processes are part of a login session (e.g. system
> service processes, user processes that are shared between multiple sessions
> of the same user, or kernel threads).


So both empirically and in documentation, this seems like something that
won't work. Do you have some successful past experience in using logind
with system service processes? I'm not seeing the path to success here for
system units.


>
> Even if you got everything right, I think it shows a bad example to
> others who are not aware of all the caveats you have carefully avoided.
>
> Is there something wrong with logind?
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] launcher: don't exit when user is not root

2017-10-30 Thread Matt Hoosier
On Mon, Oct 30, 2017 at 10:02 AM, Pekka Paalanen 
wrote:

> On Mon, 30 Oct 2017 15:20:42 +0100
> Emre Ucan  wrote:
>
> > weston does not need to be root.
> > It requires adjusting ownership on the given tty device.
> >
> > If weston does not have proper rights, it will get
> > an error at startup anyway.
> >
> > Signed-off-by: Emre Ucan 
> > ---
> >  libweston/launcher-direct.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/libweston/launcher-direct.c b/libweston/launcher-direct.c
> > index a5d3ee5..b05d214 100644
> > --- a/libweston/launcher-direct.c
> > +++ b/libweston/launcher-direct.c
> > @@ -276,9 +276,6 @@ launcher_direct_connect(struct weston_launcher
> **out, struct weston_compositor *
> >  {
> >   struct launcher_direct *launcher;
> >
> > - if (geteuid() != 0)
> > - return -EINVAL;
> > -
> >   launcher = zalloc(sizeof(*launcher));
> >   if (launcher == NULL)
> >   return -ENOMEM;
>
> NAK, for the reasons explained in
> https://lists.freedesktop.org/archives/wayland-devel/2017-
> October/035582.html
>
> To summarize, it's not only tty permissions but DRM and input devices
> as well. If you set all these so that weston can actually run without
> root using the direct launcher, then quite likely you have opened some
> security holes.


Just to confirm then: you are asserting that Weston is making a policy
decision that the system has been configured poorly if it finds that, even
though all the requested ioctl()'s and open()'s and friends have succeeded,
that it didn't happen to be running as root?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Automatically choosing a VT for DRM compositor?

2017-10-30 Thread Matt Hoosier
On Mon, Oct 30, 2017 at 5:10 AM, Pekka Paalanen  wrote:

> On Fri, 27 Oct 2017 09:10:02 -0500
> Matt Hoosier  wrote:
>
> > On Fri, Oct 27, 2017 at 8:05 AM, Pekka Paalanen 
> wrote:
> >
> > > On Fri, 27 Oct 2017 07:05:13 -0500
> > > Matt Hoosier  wrote:
> > >
> > > > HI Pekka,
> > > >
> > > > On Thu, Oct 26, 2017 at 2:06 AM, Pekka Paalanen  >
> > > wrote:
> > > >
> > > > > On Tue, 24 Oct 2017 08:26:04 -0500
> > > > > Matt Hoosier  wrote:
> > > > >
> > > > > > On Tue, Oct 24, 2017 at 1:33 AM, Pekka Paalanen <
> ppaala...@gmail.com
> > > >
> > > > > wrote:
> > > > > > > On Mon, 23 Oct 2017 14:37:37 -0500
> > > > > > > Matt Hoosier  wrote:
> > > > > > >
> > > > > > >> It would be nice for non-session uses of Weston (embedded
> > > systems) if
> > > > > > >> the controlling TTY didn't need to be manually supplied.
> > > > > > >>
> > > > > > >> Has anybody suggested using something like VT_OPENQRY in the
> > > > > > >> weston-launcher code to pick a TTY if one wasn't manually
> given?
> > > (Or
> > > > > > >> if that's too auto-magic for taste here, then only do that if
> a
> > > > > > >> specific command-line option is supplied.)
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I do not recall such proposals. I'd like to hear more about
> your
> > > use
> > > > > > > case.
> > >
> > > > > You're right in that Weston shouldn't care, but I would expect the
> > > > > system designer or administrator to care, usually.
> > > > >
> > > >
> > > > I think that it would only typically be important if the designer has
> > > some
> > > > use-case for actively hopping among more than one VT. That is, at
> least,
> > > > not my case. Weston is the only DRM user -- it just needs some valid
> VT
> > > to
> > > > use. I would be interested to hear counterexamples though. Are you
> > > thinking
> > > > that some program like Plymouth would get used early during boot,
> and it
> > > > would need to persist and occasionally show itself during the main
> body
> > > of
> > > > the runtime?
> > >
> > > Nope. My rationale is simply: if you don't care which VT is being used,
> > > why not hardcode tty0 in a systemd unit and be done with it?
> > >
> >
> > I appreciate that though. In practice, it's difficult to guarantee that
> > there won't be some transient early splash type program squatting on the
> > first TTY which is just waiting to die until somebody comes along and
> > VT_ACTIVATE's away from it.
> >
> > I hope that doesn't sound like a bait-and-switch argument. I'm not
> denying
> > that there may be some random little graphical program that uses VT's. My
> > claim is just that these are usually not relevant to the main runtime of
> > the system. Once Weston get running, it owns the foreground graphics
> > permanently.
>
> Hi,
>
> I'm afraid that sound quite much like you don't actually know what runs
> in the (embedded) system you are developing.
>

For any one specific device, I do do know the fine-grained details. The
logistical trouble is in trying to harmonize the Weston configuration
across numerous devices (from different chip vendors), all of which differ
in ways which (to me) seem incidental. There is always a surplus of VT's
compared to the zero (or one) early-boot users of DRM, so it doesn't cause
me much worry that Weston will fail to select an available TTY.

It seems like there's a small mismatch of expectations. The existing
weston-launch implementation already uses VT_OPENQRY to select whatever
free VT happens to be available, if you do 'weston-launch -u '.
I'm not really seeing the reason for that being okay, but regarding similar
automatic selection in the direct launcher as sloppy design.


>
> > > > I think I'd just like to float the original question again: would
> anybody
> > > > be receptive to a very limited patch that does nothing more than
> narrowly
> > > > allow Weston to survive the TTY selection phase in the absence of any
> > > > ext

To the libinput people: standard network protocol for remote input devices?

2017-10-27 Thread Matt Hoosier
This question is mainly meant for the input device developers that hang out
here.

Does anybody know of a de facto network protocol that should be used for
relaying input devices across a network? I don't have full remote-desktop
situations in mind -- this is narrowly a question about devices with some
touchpanel or similar that isn't directly cabled to the circuit board.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Automatically choosing a VT for DRM compositor?

2017-10-27 Thread Matt Hoosier
On Fri, Oct 27, 2017 at 8:05 AM, Pekka Paalanen  wrote:

> On Fri, 27 Oct 2017 07:05:13 -0500
> Matt Hoosier  wrote:
>
> > HI Pekka,
> >
> > On Thu, Oct 26, 2017 at 2:06 AM, Pekka Paalanen 
> wrote:
> >
> > > On Tue, 24 Oct 2017 08:26:04 -0500
> > > Matt Hoosier  wrote:
> > >
> > > > On Tue, Oct 24, 2017 at 1:33 AM, Pekka Paalanen  >
> > > wrote:
> > > > > On Mon, 23 Oct 2017 14:37:37 -0500
> > > > > Matt Hoosier  wrote:
> > > > >
> > > > >> It would be nice for non-session uses of Weston (embedded
> systems) if
> > > > >> the controlling TTY didn't need to be manually supplied.
> > > > >>
> > > > >> Has anybody suggested using something like VT_OPENQRY in the
> > > > >> weston-launcher code to pick a TTY if one wasn't manually given?
> (Or
> > > > >> if that's too auto-magic for taste here, then only do that if a
> > > > >> specific command-line option is supplied.)
> > > > >
> > > > > Hi,
> > > > >
> > > > > I do not recall such proposals. I'd like to hear more about your
> use
> > > > > case.
>
> > > You're right in that Weston shouldn't care, but I would expect the
> > > system designer or administrator to care, usually.
> > >
> >
> > I think that it would only typically be important if the designer has
> some
> > use-case for actively hopping among more than one VT. That is, at least,
> > not my case. Weston is the only DRM user -- it just needs some valid VT
> to
> > use. I would be interested to hear counterexamples though. Are you
> thinking
> > that some program like Plymouth would get used early during boot, and it
> > would need to persist and occasionally show itself during the main body
> of
> > the runtime?
>
> Nope. My rationale is simply: if you don't care which VT is being used,
> why not hardcode tty0 in a systemd unit and be done with it?
>

I appreciate that though. In practice, it's difficult to guarantee that
there won't be some transient early splash type program squatting on the
first TTY which is just waiting to die until somebody comes along and
VT_ACTIVATE's away from it.

I hope that doesn't sound like a bait-and-switch argument. I'm not denying
that there may be some random little graphical program that uses VT's. My
claim is just that these are usually not relevant to the main runtime of
the system. Once Weston get running, it owns the foreground graphics
permanently.


>
> > >
> > > If there actually is a need to runtime discover a free VT because there
> > > are also other units grabbing VTs, then would you not have a race
> > > causing everything to be launched on random VTs?
> > >
> >
> > In the spirit of my reply above, I don't think there are really other
> units
> > to be worried about racing against.
> >
> >
> > >
> > > You should be able to avoid the TTYPath hardcoding by using a systemd
> > > instantiated units (e.g. getty@.service) or template units. The
> > > instantiated approach will leave the TTY choice for the system
> > > administrator, but guarantee the same tty is always used once
> > > "configured".
> > >
> > > FWIW, running Weston directly as root is meant only for debugging. If
> > > you are using systemd units, I would very much recommend using the
> > > systemd PAM directives to run weston as non-root and relying on logind
> > > instead of launcher-direct.c. An alternative would be to arrange
> > > auto-login with a getty and launch weston from systemd user units along
> > > with all the other stuff you need, but it's not always as flexible as
> > > one might like.
> > >
> >
> > I had a look at the logind support code in Weston does. Unless I'm
> missing
> > something, I think it's fair to summarize it as simply code that queries
> > systemd to ask "what TTY has already been allocated for the cgroup in
> which
> > my PID is a member?". If that's right, then I think taking this approach
> > still tacitly agrees to the idea that a system designer has hand-picked a
> > TTY (whether through a getty@.service default instance, a weston@
> .service
> > default instance, or similar).
>
> I'm not sure of the details, but yes, that's what it does. From what I
> could tell, I suppose the logind-launcher intergration also cannot be

Re: Automatically choosing a VT for DRM compositor?

2017-10-27 Thread Matt Hoosier
HI Pekka,

On Thu, Oct 26, 2017 at 2:06 AM, Pekka Paalanen  wrote:

> On Tue, 24 Oct 2017 08:26:04 -0500
> Matt Hoosier  wrote:
>
> > On Tue, Oct 24, 2017 at 1:33 AM, Pekka Paalanen 
> wrote:
> > > On Mon, 23 Oct 2017 14:37:37 -0500
> > > Matt Hoosier  wrote:
> > >
> > >> It would be nice for non-session uses of Weston (embedded systems) if
> > >> the controlling TTY didn't need to be manually supplied.
> > >>
> > >> Has anybody suggested using something like VT_OPENQRY in the
> > >> weston-launcher code to pick a TTY if one wasn't manually given? (Or
> > >> if that's too auto-magic for taste here, then only do that if a
> > >> specific command-line option is supplied.)
> > >
> > > Hi,
> > >
> > > I do not recall such proposals. I'd like to hear more about your use
> > > case.
> > >
> > > Do you mean in 'weston-launch' specifically, or in all the launcher
> > > implementations?
> >
> > Sorry for the imprecise wording. I'm not thinking of a adding a duty
> that's
> > specifically carried out by the weston-launch executable. My suggestion
> > would be to add the logic somewhere in with the code in launcher-direct.c
> > that gets executed directly in-process when Weston is invoked something
> > like 'weston --tty=2 --backend=drm-backend.so ...'.
> >
> > I think that the call graph in mind would be something like:
> >
> >   compositor-drm.c:  drm_backend_create()
> >   launcher-util.c: weston_launcher_connect()
> >   launcher-direct.c: launcher_direct_connect()
> >   launcher-direct.c:   setup_tty()
>
> Ah, so the launcher direct path only, which means you run weston and
> all its clients as root. Weston does spawn some clients of its own
> automatically, depending on configuration. None of them are written to
> be safe to run as root.
>

That's understood. And while I admit to being presently guilty of running
Weston as root in this style, it's not difficult at all to use system's
'User=' directive to change that. As far as I can tell though, choosing to
execute as some non-zero UID is a separate question than TTY management
though.


>
> > > If for 'weston-launch' specifically, then I'd like to ask why you want
> > > that instead of a logind service?
> > >
> > > I've been hoping there would be no need for new development on
> > > weston-launch. It is sensitive code, being setuid root. It seems it
> > > cannot be nicely generalized to support all libweston-based
> compositors.
> >
> > I'm uncertain whether the code impacted in launcher-direct.c actually
> gets
> > exercised too by some indirect path in weston-launch. Maybe you know the
> > answer to that?
>
> No, it does not. launcher-direct.c is purely for running weston as
> root, without any helpers like logind or weston-launch.
>
> > >
> > > What is the launching context in you use case? E.g.
> > > - manual login, type 'weston' in VT
> > > - type 'weston -Bdrm-backend.so' in a terminal window
> > > - launching weston from a systemd system unit
> > > - launching weston from a systemd user unit
> > > - something else?
> >
> > The case I'd like to improve is launching weston from a systemd system
> > unit. Currently that involves either hacking the systemd unit with a
> > manually chosen TTYPath= directive, or else baking in a command-line
> option
> > like --tty=3 to Weston. Those both feel forced and (in my opinion)
> require
> > building in an unnecessary amount of product-specific knowledge into the
> > systemd unit; there's no reason I can see that Weston should really care
> > what VT is used.
> >
> > Maybe I'm not recognizing some reason why it's advantageous for Weston to
> > have an explicitly chosen VT though?
>
> You're right in that Weston shouldn't care, but I would expect the
> system designer or administrator to care, usually.
>

I think that it would only typically be important if the designer has some
use-case for actively hopping among more than one VT. That is, at least,
not my case. Weston is the only DRM user -- it just needs some valid VT to
use. I would be interested to hear counterexamples though. Are you thinking
that some program like Plymouth would get used early during boot, and it
would need to persist and occasionally show itself during the main body of
the runtime?


>
> If there actually is a need to ru

Re: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell

2017-10-25 Thread Matt Hoosier
On Wed, Oct 25, 2017 at 2:09 AM, Pekka Paalanen  wrote:

> Hi,
>
> the very reason why ivi-shell ever came to be was that the operating
> environment of graphical applications in IVI was so different from a
> normal desktop, that it was simply impossible to have desktop
> applications work in a meaningful way there.
>
> I would like to hear why and how this has now changed.
>
> The premise of supporting desktop shell protocols in an IVI-shell
> environment is that all desktop applications using the full extent of
> the desktop shell protocols will always work correctly and
> meaningfully, without modification.
>
> How will that be possible without specifically writing the applications
> to behave in IVI-specific ways?
>

 To me, this doesn't seem like an all-or-nothing proposition. Surely an
IVI-capable shell can provide implementations for the basic desktop
operations:

* The shell is already allowed to ignore move requests
* The shell is already allowed to ignore resize requests
* Very little is guaranteed about a shell surface in toplevel state, so it
seems reasonable for the IVI shell to apply domain-specific defaults
* It seems just fine to allow transients and popups to be sorted all within
the same Z level of their corresponding shell surface, just as
libweston-desktop does
* Fullscreen and maximized are pretty straightforward, provided that the
IVI shell gives heuristics about default stacking policies relative to
native IVI surfaces


> Assuming the above is possible, I also see the risk that lego-block
> desktop environments will start using ivi-shell to push window
> management into an external process, undoing a lot of the benefits of a
> Wayland architecture simply because that is how X11 worked.
>

Wouldn't that argument just as well apply to the existing ivi-shell
implementation that has its controller process?


>
> When I glanced through the proposal, I didn't find an example
> implementation of the most important new component introduced: the
> ivi-surface id agent. Hence I do not think it is possible to fully
> evaluate this work as is.
>
> I don't even understand how it can show desktop shell protocol using
> windows at all without an id agent - I believe it should not, because
> if it does, then it is bypassing the IVI controller which I cannot
> imagine to be a wanted feature. Simply the fact that it is using
> libweston-desktop means that the IVI controller cannot manage all the
> surfaces (libweston-desktop exposes only top-level windows and handles
> everything else internally - how could the internal handling be always
> correct in an IVI environment?).
>


I agree that there are probably some internal questions about the mechanics
of assignment of surface IDs from desktop surfaces needs specified. But
that doesn't seem like a wholesale refutation of the idea that generic
desktop programs can be displayed sanely in a shell which happens to
support IVI concepts.


>
> IMO, an id agent should be mandatory. Otherwise it is too easy to just
> forget about it and trust your luck that the desktop apps and
> libweston-desktop will keep on behaving as you happened to test and
> that the behaviour would be appropriate in an IVI environment to begin
> with.
>

It seems like this might be driven by some past negative experience. Is
there anything you could share? As an integrator of Wayland/Weston onto
embedded systems, I'm not off-put by the idea that I need to thoroughly
test the applications I'm deploying. The use-cases are typically very well
understood and constrained though, so I've not had the experience that some
wild desktop-centric action is requested when the app runs in the wild,
that I never saw during development.

BTW, I'm trying to be careful to draw the distinction between saying that I
have the responsibility to thoroughly test my applications, from the
conclusion that this justifies re-writing the applications in terms of some
or other ivi-shell API. That's very difficult to arrange when independently
suppliers are delivering applications.


>
> If you proposed that e.g. some outputs were dedicated for desktop apps
> and some outputs for IVI apps, then I could see it working without
> complete IVI controller integration, as the two categories would be
> kept separate. It would be like running a desktop compositor on some
> outputs and an IVI compositor on the other outputs (which is actually
> implementable real soon now thanks to DRM leases, or already with
> fullscreen-shell protocol). But as I understand, this proposal is
> aiming to mix both kinds of apps on the same outputs, is it not?
>
> I am confused how this proposal is a proper solution, as I'm not sure
> what the problem to be solved is. Why do you want to run desktop apps
> on an IVI system instead of apps that are designed for work right in an
> IVI system?
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop

Re: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell

2017-10-24 Thread Matt Hoosier
I'm not at all familiar with the internal implementation of ivi-shell, so I
can't give much meaningful review. But I am very much in favor of this
patch series. Without wl_shell and xdg_shell support, I've never been able
to really give ivi-shell serious consideration on my products. The ability
to use generic client Wayland programs is very important.

-Matt

On Tue, Oct 17, 2017 at 5:51 AM, Ucan, Emre (ADITG/ESB) <
eu...@de.adit-jv.com> wrote:

> Hi,
>
> I already reviewed the patches before Michael sent:
> Reviewed-by: Emre Ucan 
>
> Best regards
>
> Emre Ucan
> Engineering Software Base (ADITG/ESB)
>
> Tel. +49 5121 49 6937
>
> > -Original Message-
> > From: wayland-devel [mailto:wayland-devel-
> > boun...@lists.freedesktop.org] On Behalf Of Michael Teyfel
> > Sent: Dienstag, 17. Oktober 2017 12:02
> > To: wayland-devel@lists.freedesktop.org
> > Subject: [PATCH weston 00/14] Desktop Protocol Support for IVI-Shell
> >
> > Hello all,
> >
> > since some time I’m working on ivi-shell to add xdg-protocol support by
> > means of libweston-desktop. Due to my changes both xdg-protocol
> > applications and ivi-shell / ivi-application-protocol applications are
> supported
> > within ivi-shell now. The known functionality is preserved and just
> extended
> > by a further protocol. The advantage is that client applications do not
> need to
> > be edited to generate an id and are also not limited to use the custom
> ivi-
> > application protocol anymore, since the ids are handled by an id agent
> inside
> > of weston now.
> >
> > As a preparation for the changes the goto labels in ivi-shell have been
> > reworked to avoid memory leaks. In ivi-layout I added an interface
> > (ivi_layout_surface_set_id) to set the surface-id of an
> ivi-layout-surface. It
> > can be done once after being created by an xdg-protocol application to
> assign
> > a numeric id by means of an id agent for example. Additionally I
> introduced a
> > new event to notify about a desktop surface being configured
> > (desktop_surface_configured). An id agent can register to this event and
> > react to this accordingly by assigning an id by means of
> > ivi_layout_surface_set_id. As a result I also changed the test client
> > applications in the Weston repository and removed the ivi-application
> > protocol support since that has only been used, if xdg protocol is not
> > supported. Finally hmi-controller has been edited to accept desktop
> > surfaces.
> >
> > There are some things that can be done in the future:  At first it would
> be
> > diligent, if hmi-controller would also use xdg protocol for the GUI
> itself. Then
> > also the surface_configure event could be removed from hmi-controller.
> > Secondly the weston unit tests should also test the interface changes for
> > surface_set_id and also should stop using the ivi-application protocol.
> >
> > Thanks for reading and questions are very welcome.
> >
> >
> > Best regards
> >
> > Michael Teyfel
> > Advanced Driver Information Technology GmbH
> > Engineering Software Base (ADITG/ESB)
> > Robert-Bosch-Str. 200
> > 31139 Hildesheim
> > Germany
> > Tel. +49 5121 49 6932
> > Fax +49 5121 49 6999
> > mtey...@de.adit-jv.com
> > ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car
> > Multimedia GmbH and DENSO Corporation
> > Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
> > Geschäftsführung: Wilhelm Grabow, Ken Yaguchi
> >
> >
> > Michael Teyfel (14):
> >   ivi-shell: rework goto labels to avoid memory leaks
> >   ivi-shell: removed assert
> >   ivi-shell: introduction of IVI_INVALID_ID
> >   layout-interface: added interface to change surface id
> >   ivi-layout: introduced configure_desktop_changed
> >   ivi-layout: introduced surface create and configure
> >   ivi-shell: linked libweston-desktop and added structs
> >   ivi-shell: added libweston-desktop-api implementation
> >   ivi-shell: remove surface_destroy_listener
> >   ivi-shell: create weston_desktop in wet_shell_init
> >   hmi-controller: register for desktop_surface_configured
> >   simple-egl: remove ivi-application support
> >   simple-shm: remove ivi-application support
> >   window client: remove ivi-application support
> >
> >  Makefile.am|  11 +--
> >  clients/simple-egl.c   |  86 +++-
> >  clients/simple-shm.c   |  40 
> >  clients/window.c   |  44 +
> >  ivi-shell/hmi-controller.c |  70 +++--
> >  ivi-shell/ivi-layout-export.h  |  18 
> >  ivi-shell/ivi-layout-private.h |   3 +
> >  ivi-shell/ivi-layout-shell.h   |   8 ++
> >  ivi-shell/ivi-layout.c | 142 ++-
> >  ivi-shell/ivi-shell.c  | 218
> > ++---
> >  ivi-shell/ivi-shell.h  |   2 +
> >  11 files changed, 407 insertions(+), 235 deletions(-)
> >
> > --
> > 2.7.4
> >
> > ___
> > wayland-devel mailing list
> > way

Re: Automatically choosing a VT for DRM compositor?

2017-10-24 Thread Matt Hoosier
On Tue, Oct 24, 2017 at 1:33 AM, Pekka Paalanen  wrote:
> On Mon, 23 Oct 2017 14:37:37 -0500
> Matt Hoosier  wrote:
>
>> It would be nice for non-session uses of Weston (embedded systems) if
>> the controlling TTY didn't need to be manually supplied.
>>
>> Has anybody suggested using something like VT_OPENQRY in the
>> weston-launcher code to pick a TTY if one wasn't manually given? (Or
>> if that's too auto-magic for taste here, then only do that if a
>> specific command-line option is supplied.)
>
> Hi,
>
> I do not recall such proposals. I'd like to hear more about your use
> case.
>
> Do you mean in 'weston-launch' specifically, or in all the launcher
> implementations?

Sorry for the imprecise wording. I'm not thinking of a adding a duty that's
specifically carried out by the weston-launch executable. My suggestion
would be to add the logic somewhere in with the code in launcher-direct.c
that gets executed directly in-process when Weston is invoked something
like 'weston --tty=2 --backend=drm-backend.so ...'.

I think that the call graph in mind would be something like:

  compositor-drm.c:  drm_backend_create()
  launcher-util.c: weston_launcher_connect()
  launcher-direct.c: launcher_direct_connect()
  launcher-direct.c:   setup_tty()

> If for 'weston-launch' specifically, then I'd like to ask why you want
> that instead of a logind service?
>
> I've been hoping there would be no need for new development on
> weston-launch. It is sensitive code, being setuid root. It seems it
> cannot be nicely generalized to support all libweston-based compositors.

I'm uncertain whether the code impacted in launcher-direct.c actually gets
exercised too by some indirect path in weston-launch. Maybe you know the
answer to that?

>
> What is the launching context in you use case? E.g.
> - manual login, type 'weston' in VT
> - type 'weston -Bdrm-backend.so' in a terminal window
> - launching weston from a systemd system unit
> - launching weston from a systemd user unit
> - something else?

The case I'd like to improve is launching weston from a systemd system
unit. Currently that involves either hacking the systemd unit with a
manually chosen TTYPath= directive, or else baking in a command-line option
like --tty=3 to Weston. Those both feel forced and (in my opinion) require
building in an unnecessary amount of product-specific knowledge into the
systemd unit; there's no reason I can see that Weston should really care
what VT is used.

Maybe I'm not recognizing some reason why it's advantageous for Weston to
have an explicitly chosen VT though?

> What is the "non-session" use exactly? Why do you not care which VT
> weston will occupy in that case?

This is another case where I was groping for a good term to use.
"Non-session" isn't quite right. What I meant was: the instance of Weston is

- Well known to systemd;
- Not the parent of the graphical clients (those are each managed from
their own systemd units); and
- Not a "session" in the sense of being a login initiated explicitly from a
VT in the way that GDM does.

The identity of the VT on which Weston binds doesn't really seem to matter
for an embedded device. There's no user sitting at the keyboard typing
Ctrl+Alt+F2 to multiplex among numerous sessions.

If any of the regular contributors here who work with embedded devices have
a different experience, I'd be interested to hear. But typically, I find
that every silicon manufacturer's delivery of Weston has a default systemd
unit that does one of the following:

  - uses TTYPath=
  - passes some arbitrary value to Weston with --tty=
  - wraps the entire Weston process with openvt(1)


> I haven't checked recently, but I have a feeling that Weston is not
> expecting to be spawned on a currently inactive VT, so that might need
> fixing as well.

I think this use-case actually works well already. If you open up an SSH
shell to an embedded device (hence, its controlling terminal is just a PTY
rather than a Linux VT) you can do:

weston --tty=1; sleep 1 &
weston --tty=2; sleep 1 &
weston --tty=3; sleep 1
...

Each instance of Weston will manage to activate the VT it's been told to
use.

>> The closest I can find to a question about this topic prior on the
>> mailing list is
>>
https://lists.freedesktop.org/archives/wayland-devel/2013-October/011472.html
.
>> That change was never adopted, but it doesn't look like the use of
>> VT_OPENQRY has anything to do with Kristian's objections at the time.
>
> Picking the first free VT sounds fine to me, but do display servers
> actually do that on their own or does e.g.

Automatically choosing a VT for DRM compositor?

2017-10-23 Thread Matt Hoosier
It would be nice for non-session uses of Weston (embedded systems) if
the controlling TTY didn't need to be manually supplied.

Has anybody suggested using something like VT_OPENQRY in the
weston-launcher code to pick a TTY if one wasn't manually given? (Or
if that's too auto-magic for taste here, then only do that if a
specific command-line option is supplied.)

The closest I can find to a question about this topic prior on the
mailing list is
https://lists.freedesktop.org/archives/wayland-devel/2013-October/011472.html.
That change was never adopted, but it doesn't look like the use of
VT_OPENQRY has anything to do with Kristian's objections at the time.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Contact for freenode #wayland adminstration questions?

2017-10-23 Thread Matt Hoosier
I'm having some trouble joining the Wayland channel on Freenode. Is
there a specific person to contact about that?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2] libweston-desktop: add signal for title/app-id changes

2017-10-23 Thread Matt Hoosier
Ping

On Tue, Sep 5, 2017 at 8:05 AM, Matt Hoosier  wrote:
> As discussed on
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034720.html,
> it's useful for the shell implementation to know when these change,
> for example to relay the information on to taskbars or similar.
>
> To avoid ABI changes or the need to make the weston_desktop_surface
> definition public, new functions are introduced for attaching
> listeners to these signals.
>
> Signed-off-by: Matt Hoosier 
> ---
>  libweston-desktop/libweston-desktop.h |  3 +++
>  libweston-desktop/surface.c   | 12 
>  2 files changed, 15 insertions(+)
>
> diff --git a/libweston-desktop/libweston-desktop.h 
> b/libweston-desktop/libweston-desktop.h
> index 03b04c7b..c43568ac 100644
> --- a/libweston-desktop/libweston-desktop.h
> +++ b/libweston-desktop/libweston-desktop.h
> @@ -164,6 +164,9 @@ weston_desktop_surface_set_size(struct 
> weston_desktop_surface *surface,
> int32_t width, int32_t height);
>  void
>  weston_desktop_surface_close(struct weston_desktop_surface *surface);
> +void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
> *surface,
> + struct wl_listener *listener);
>
>  void *
>  weston_desktop_surface_get_user_data(struct weston_desktop_surface *surface);
> diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
> index d3be9364..d00ba5d6 100644
> --- a/libweston-desktop/surface.c
> +++ b/libweston-desktop/surface.c
> @@ -64,6 +64,7 @@ struct weston_desktop_surface {
> char *title;
> char *app_id;
> pid_t pid;
> +   struct wl_signal title_signal;
> };
> struct {
> struct weston_desktop_surface *parent;
> @@ -287,6 +288,8 @@ weston_desktop_surface_create(struct weston_desktop 
> *desktop,
> wl_list_init(&surface->view_list);
> wl_list_init(&surface->grab_link);
>
> +   wl_signal_init(&surface->title_signal);
> +
> return surface;
>  }
>
> @@ -511,6 +514,13 @@ weston_desktop_surface_close(struct 
> weston_desktop_surface *surface)
>surface->implementation_data);
>  }
>
> +WL_EXPORT void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
> *surface,
> + struct wl_listener *listener)
> +{
> +   wl_signal_add(&surface->title_signal, listener);
> +}
> +
>  struct weston_desktop_surface *
>  weston_desktop_surface_from_client_link(struct wl_list *link)
>  {
> @@ -687,6 +697,7 @@ weston_desktop_surface_set_title(struct 
> weston_desktop_surface *surface,
>
> free(surface->title);
> surface->title = tmp;
> +   wl_signal_emit(&surface->title_signal, surface);
>  }
>
>  void
> @@ -701,6 +712,7 @@ weston_desktop_surface_set_app_id(struct 
> weston_desktop_surface *surface,
>
> free(surface->app_id);
> surface->app_id = tmp;
> +   wl_signal_emit(&surface->title_signal, surface);
>  }
>
>  void
> --
> 2.13.3
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Running multiple Wayland window managers in different ttys

2017-10-20 Thread Matt Hoosier
Hi Deepak,

On Fri, Oct 20, 2017 at 11:50 AM, Deepak Jois  wrote:
> Hi
>
> I have read the Wayland docs and skimmed through the API reference,
> and I am trying to understand some concepts better. The questions
> below are probably a bit naive, but I appreciate any
> suggestions/explanations.
>
> To illustrate my situation better, let me start with an actual
> scenario I tried to make work. I am trying to run Gnome and another
> Wayland-based window manager (Sway or Weston) in two different ttys as
> the same user.
>
> I have Gnome running under Wayland on tty2. The Gnome session is
> running some app like gnome-terminal.
>
> I do the following:
>
> * Press Ctrl-Alt-F3 (switching to tty3, the lowest free tty)
> * Login as the same user running the Gnome session
> * Launch sway
> * Launch urxvt and try to launch gnome-terminal.
>
> Now I have two issues I am trying to understand better:
>
> 1. When I check the WAYLAND_DISPLAY variable in the sway session, it
> shows 'wayland-0', which is the same as the one show in the Gnome
> session. Why is it the same? Does Wayland have a concept of multiple
> servers/sessions per user? Can I run the Gnome session and the Sway
> session separately?

Although you're right that the goal here is to bind the different
Wayland sessions' values for WAYLAND_DISPLAY to unique virtual
terminals, that won't happen automatically just because you launched
them from separate tty's.

The trick here is to inform each compositor that it should be
listening on a different Wayland socket name. For example, if you use
Weston:

$ weston --socket=wayland-1

This will cause Weston to establish its listening socket under the
name /run/user//wayland-1 rather than the default
/run/user//wayland-0. I think you're running into collisions
where each of your compositor instances is mislead into believing that
it successfully managed to appropriate /run/user//wayland-0 for
itself.

Also (at least for Weston), the value of the '--socket' parameter is
used to automatically compute and set the value of the
$WAYLAND_DISPLAY environment variable which is then passed along to
all children of the compositor. Presumably for you, your desktop
environment (e.g., Weston's desktop-shell) is the starting point from
which you're launching other graphical programs. They will all in turn
inherit this correct value of $WAYLAND_DISPLAY too.

>
> 2. I tried explicitly setting the WAYLAND_DISPLAY to wayland-1. It has
> no effect on Sway. The WAYLAND_DISPLAY variable is reset inside the
> session to wayland-0. But when I try to launch Weston, it errors out
> (log below). Is this expected, or is this a bug?

The way to get Weston to pass the right value of WAYLAND_DISPLAY to
its spawn is to use --socket=.

It's more difficult to invoke the Wayland gnome-session directly. I'm
not sure how you would micromanage its selection of the server socket
name.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] cursor: Use shm_open() instead of XDG_RUNTIME_DIR files

2017-10-18 Thread Matt Hoosier
On Wed, Oct 18, 2017 at 4:23 AM, Quentin Glidic
 wrote:
> +   fd = shm_open("/wayland-cursor-shared", O_CREAT | O_RDWR, 0);
> +   shm_unlink("/wayland-cursor-shared");

This seems to be a departure from the anonymous behavior that
mkstemp() previously offered. shm_open() says that it will open an
existing shared-memory object if that pathname already exists. Isn't
there a race between one thread doing shm_open() and a different
thread doing shm_unlink() such that you could accidentally end up with
two different filedescriptors pointing at the same SHM object?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Ideas on portable APIs to cheaply copy a GBM bo?

2017-10-11 Thread Matt Hoosier
On Tue, Oct 10, 2017 at 11:53 AM, Daniel Stone  wrote:
> That more or less makes sense at the moment, though there has been
> quite a bit of work on less insane remoting within Weston. And then
> GNOME is using PipeWire for this. But anyway, you probably know this
> and it's not your immediate question.

I hadn't heard of PipeWire; thanks, but at least the project
description seems way more ambitious than my needs at the moment.

>> Another option is to enforce a synchronous handshake between the
>> Weston foreground loop and the compressor/transmit asynchronous code.
>> The idea would be to (1) suck out the primary plane GBM bo's dmabuf,
>> (2) wait for the async stuff to work and then signal the completion of
>> its usage on the BO, and then (3) release the BO locked in the first
>> step. This has some pretty bad stalling implications though -- the
>> async code can at times take nearly a full frame to run. The spillover
>> into the next vblank period would basically force this scheme to run
>> at half the normal framerate even though better interleaved use of the
>> hardware can do much better.
>
> Well, when you say back/front ... it can be more than just two. By
> default, Mesa's GBM implementation should quad-buffer if you sit on
> unreleased buffers for long enough. Have you tried it out? That's
> definitely what I'd recommend, anyway: all the other options are, as
> you've noted, bad.

I'll have a look there. One thing that for simplicity I left out of
the original description above is that this custom Weston output has a
fast-path very similar to the normal DRM scanout optimizations. So if
that path gets hit, it's often the live DRM buffer supplied by the
client whose dmabuf fd we're using in the background processing. I'm
not sure that I can robustly assume that the clients support stacking
up arbitrarily deep numbers of renderbuffers. If all the buffers
getting chewed on were owned strictly inside the compositor, I think
that relying on 4-deep buffering in Mesa might be do-able. I'll have
to think about whether that makes sense to generalize to client
buffers as well.

>
>> Does the readership here on wayland-devel know of any DRM-centric API
>> (I looked and nothing came to mind) for leveraging a basic cheap blit
>> from one DRM fb into another?
>
> In a lot of cores, you don't get particularly easy access to blit
> engines as they're more general-purpose these days. There also isn't a
> generalsed API at all, even for the more fixed-function hardware.


Thanks for the ideas -- they're appreciated.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Ideas on portable APIs to cheaply copy a GBM bo?

2017-10-10 Thread Matt Hoosier
Hi all,

My organization maintains a small patch against the DRM compositor
that causes it to register another output. This output accepts the
usual compositor scenegraph, does the rendering down to a primary
plane, and then funnels the resulting GBM buffer through a codepath
that does video compression and network transmission. (Why hack this
into the DRM compositor? Mostly because it has all the infrastructure
for setting up GBM, which as far as I can tell ends up being pretty
much a requirement to get access to the composited scene as a dmabuf.)

There's a small bit of trouble involved in handing off the dmabuf of
the FB completed, composited scene to the compress/transmit code. That
stuff runs asynchronously from the Weston event loop by virtue of
living in GStreamer. The means that it's technically prone to tearing
if the compositor gets around to flipping the back/front buffer sooner
than the GStreamer compress/transmit stuff finishes accessing the GBM
bo's dmabuf.

One possible way to remove this race would be to use GL to take a
private copy of the rendered primary plane. That's fairly expensive
though, so it would be nice to avoid if at all possible.

Another option is to enforce a synchronous handshake between the
Weston foreground loop and the compressor/transmit asynchronous code.
The idea would be to (1) suck out the primary plane GBM bo's dmabuf,
(2) wait for the async stuff to work and then signal the completion of
its usage on the BO, and then (3) release the BO locked in the first
step. This has some pretty bad stalling implications though -- the
async code can at times take nearly a full frame to run. The spillover
into the next vblank period would basically force this scheme to run
at half the normal framerate even though better interleaved use of the
hardware can do much better.

Does the readership here on wayland-devel know of any DRM-centric API
(I looked and nothing came to mind) for leveraging a basic cheap blit
from one DRM fb into another?

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v12 00/40] Atomic modesetting

2017-09-27 Thread Matt Hoosier
On Tue, Sep 26, 2017 at 12:15 PM, Daniel Stone  wrote:
> Hi,
> Here's a cleaned-up and fully-tested version of the atomic series.

Hi;

Just wondering: do you maintain these in a publicly fetchable repo
somewhere? I didn't turn up anything after some light searching --
there's git://people.freedesktop.org/~daniels/westonover on FDO's Git
server, but it looks disused for a few years now.

Cheers,
Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] compositor: fix starvation of wl_buffer::release

2017-09-26 Thread Matt Hoosier
Done: 
https://lists.freedesktop.org/archives/wayland-devel/2017-September/035191.html

On Tue, Sep 26, 2017 at 2:28 AM, Pekka Paalanen  wrote:
> On Mon, 25 Sep 2017 10:31:35 -0500
> Matt Hoosier  wrote:
>
>> This change replaces a queued emission of buffer-release events (which
>> is prone to starvation) with a regular event emission. This means that
>> client programs no longer need to secretly install surface frame
>> listeners just to guarantee that they get correctly notified of buffer
>> lifecycle events.
>>
>> Signed-off-by: Matt Hoosier 
>> ---
>>  clients/nested.c   | 3 +--
>>  libweston/compositor.c | 3 +--
>>  2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/clients/nested.c b/clients/nested.c
>> index e9070e9b..e2bdf684 100644
>> --- a/clients/nested.c
>> +++ b/clients/nested.c
>> @@ -228,8 +228,7 @@ nested_buffer_reference(struct nested_buffer_reference 
>> *ref,
>>   ref->buffer->busy_count--;
>>   if (ref->buffer->busy_count == 0) {
>>   assert(wl_resource_get_client(ref->buffer->resource));
>> - wl_resource_queue_event(ref->buffer->resource,
>> - WL_BUFFER_RELEASE);
>> + wl_buffer_send_release(ref->buffer->resource);
>>   }
>>   wl_list_remove(&ref->destroy_listener.link);
>>   }
>> diff --git a/libweston/compositor.c b/libweston/compositor.c
>> index 813b6634..878cd535 100644
>> --- a/libweston/compositor.c
>> +++ b/libweston/compositor.c
>> @@ -1954,8 +1954,7 @@ weston_buffer_reference(struct weston_buffer_reference 
>> *ref,
>>   ref->buffer->busy_count--;
>>   if (ref->buffer->busy_count == 0) {
>>   assert(wl_resource_get_client(ref->buffer->resource));
>> - wl_resource_queue_event(ref->buffer->resource,
>> - WL_BUFFER_RELEASE);
>> + wl_buffer_send_release(ref->buffer->resource);
>>   }
>>   wl_list_remove(&ref->destroy_listener.link);
>>   }
>
> Hi,
>
> this patch is exactly right, but I feel we should be more verbose on
> the reasoning behind this change. The old behaviour has been kept for a
> long time. Maybe a link to my email?
>
> That's the only thing I'd like to have before giving my Reviewed-by.
>
> Let's also wait for other comments before landing this.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v2] compositor: fix starvation of wl_buffer::release

2017-09-26 Thread Matt Hoosier
This change replaces a queued emission of buffer-release events (which
is prone to starvation) with a regular event emission. This means that
client programs no longer need to secretly install surface frame
listeners just to guarantee that they get correctly notified of buffer
lifecycle events.

v2:

More information about the historical reasons why this change hadn't
happened yet, and the consensus to finally move ahead with it can be
found at the discussion terminating in this message:

https://lists.freedesktop.org/archives/wayland-devel/2017-September/035147.html

Signed-off-by: Matt Hoosier 
---
 clients/nested.c   | 3 +--
 libweston/compositor.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/clients/nested.c b/clients/nested.c
index e9070e9b..e2bdf684 100644
--- a/clients/nested.c
+++ b/clients/nested.c
@@ -228,8 +228,7 @@ nested_buffer_reference(struct nested_buffer_reference *ref,
ref->buffer->busy_count--;
if (ref->buffer->busy_count == 0) {
assert(wl_resource_get_client(ref->buffer->resource));
-   wl_resource_queue_event(ref->buffer->resource,
-   WL_BUFFER_RELEASE);
+   wl_buffer_send_release(ref->buffer->resource);
}
wl_list_remove(&ref->destroy_listener.link);
}
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 813b6634..878cd535 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1954,8 +1954,7 @@ weston_buffer_reference(struct weston_buffer_reference 
*ref,
ref->buffer->busy_count--;
if (ref->buffer->busy_count == 0) {
assert(wl_resource_get_client(ref->buffer->resource));
-   wl_resource_queue_event(ref->buffer->resource,
-   WL_BUFFER_RELEASE);
+   wl_buffer_send_release(ref->buffer->resource);
}
wl_list_remove(&ref->destroy_listener.link);
}
-- 
2.13.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols] Add zwp_linux_explicit_synchronization_v1

2017-09-25 Thread Matt Hoosier
On Fri, Sep 22, 2017 at 6:18 PM, Matt Hoosier  wrote:
> On Fri, Sep 22, 2017 at 10:24 AM, Pekka Paalanen  wrote:
>> On Wed, 20 Sep 2017 07:11:57 -0700
>> Daniel Stone  wrote:
>>
>>> Hi Matt,
>>>
>>> On 20 September 2017 at 05:48, Matt Hoosier  wrote:
>>> > On a related subject, there was discussion back at
>>> > https://lists.freedesktop.org/archives/wayland-devel/2013-September/011091.html
>>> > that acknowledged a longstanding bug where wl_buffer::release events
>>> > are starved indefinitely in the outbound server socket if there
>>> > doesn't happen to be any frame callback installed. That discussion
>>> > petered out, but it looked like there was a consensus that the issue
>>> > should be fixed (post the WL_BUFFER_RELEASE immediately rather than
>>> > queue if no frame callbacks attached).
>>> >
>>> > That seems like a good issue to get closed up. Any objections to
>>> > reviving the patch along the lines that Pekka suggested?
>>>
>>> Since then, we've moved the frame events around, such that they're no
>>> longer sent immediately, which might change the calculus a bit. Pekka?
>>
>> Hi,
>>
>> since 2013, we have got rid of the separate wl_event_loop used to
>> throttle the input events as well, IIRC.
>>
>> I have to say today I would be completely fine with just posting the
>> release events always and never delaying them. The queueing has been
>> such a pain for EGL implementations.
>>
>> I'm not sure we would even have the necessary information in the
>> compositor about whether a client wants release event ASAP or is it
>> happy with throttling. I suppose we could track whether the
>> wl_surface.commit that makes a wl_buffer reserved in the compositor
>> also had a wl_surface.frame request and only in that case queue the
>> release, otherwise post. I haven't thought it through.
>>
>> Frame callbacks are sent as the last thing in weston_output_repaint(),
>> that has not changed. What did change is that there is now a delay
>> between the pageflip event and the call to weston_output_repaint(). Now
>> we also have the repaint grouping across outputs but I don't think that
>> makes much of a difference. So I would say the situation has not
>> changed wrt. to when the frame callbacks are sent out.
>>
>> Well, if we need a pageflip event to release a buffer, then yes, the
>> frame callback will be coming later than in 2013. But that implies the
>> client buffer was on scanout.
>>
>> I don't really like the idea that a client needs to "flush out" the
>> release events somehow. That is like leaking compositor implementation
>> details into clients. And that's exactly what asking for frame or sync
>> callbacks is when you have no other use for them.
>>
>> I also don't quite like the idea of magically inferring whether a
>> client is speed-runner game or a leisurely office app. We might even
>> need explicit protocol for that, and then could use it for both buffer
>> releases and input event throttling and coalescing.
>>
>> All in all, in the spirit of avoiding premature optimization, I'd go
>> for the simple solution and let profiling drive more complicated
>> solutions when the need arises. We're talking about buffer releases.
>> Buffer releases happen when clients animate. Surely animation is heavy
>> enough to obliterate any power savings we might get from optimizing
>> when to flush out release events?
>
> I completely agree. I'll submit something to wayland-devel on Monday.

Done, see 
https://lists.freedesktop.org/archives/wayland-devel/2017-September/035175.html.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] compositor: fix starvation of wl_buffer::release

2017-09-25 Thread Matt Hoosier
This change replaces a queued emission of buffer-release events (which
is prone to starvation) with a regular event emission. This means that
client programs no longer need to secretly install surface frame
listeners just to guarantee that they get correctly notified of buffer
lifecycle events.

Signed-off-by: Matt Hoosier 
---
 clients/nested.c   | 3 +--
 libweston/compositor.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/clients/nested.c b/clients/nested.c
index e9070e9b..e2bdf684 100644
--- a/clients/nested.c
+++ b/clients/nested.c
@@ -228,8 +228,7 @@ nested_buffer_reference(struct nested_buffer_reference *ref,
ref->buffer->busy_count--;
if (ref->buffer->busy_count == 0) {
assert(wl_resource_get_client(ref->buffer->resource));
-   wl_resource_queue_event(ref->buffer->resource,
-   WL_BUFFER_RELEASE);
+   wl_buffer_send_release(ref->buffer->resource);
}
wl_list_remove(&ref->destroy_listener.link);
}
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 813b6634..878cd535 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1954,8 +1954,7 @@ weston_buffer_reference(struct weston_buffer_reference 
*ref,
ref->buffer->busy_count--;
if (ref->buffer->busy_count == 0) {
assert(wl_resource_get_client(ref->buffer->resource));
-   wl_resource_queue_event(ref->buffer->resource,
-   WL_BUFFER_RELEASE);
+   wl_buffer_send_release(ref->buffer->resource);
}
wl_list_remove(&ref->destroy_listener.link);
}
-- 
2.13.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols] Add zwp_linux_explicit_synchronization_v1

2017-09-22 Thread Matt Hoosier
On Fri, Sep 22, 2017 at 10:24 AM, Pekka Paalanen  wrote:
> On Wed, 20 Sep 2017 07:11:57 -0700
> Daniel Stone  wrote:
>
>> Hi Matt,
>>
>> On 20 September 2017 at 05:48, Matt Hoosier  wrote:
>> > On a related subject, there was discussion back at
>> > https://lists.freedesktop.org/archives/wayland-devel/2013-September/011091.html
>> > that acknowledged a longstanding bug where wl_buffer::release events
>> > are starved indefinitely in the outbound server socket if there
>> > doesn't happen to be any frame callback installed. That discussion
>> > petered out, but it looked like there was a consensus that the issue
>> > should be fixed (post the WL_BUFFER_RELEASE immediately rather than
>> > queue if no frame callbacks attached).
>> >
>> > That seems like a good issue to get closed up. Any objections to
>> > reviving the patch along the lines that Pekka suggested?
>>
>> Since then, we've moved the frame events around, such that they're no
>> longer sent immediately, which might change the calculus a bit. Pekka?
>
> Hi,
>
> since 2013, we have got rid of the separate wl_event_loop used to
> throttle the input events as well, IIRC.
>
> I have to say today I would be completely fine with just posting the
> release events always and never delaying them. The queueing has been
> such a pain for EGL implementations.
>
> I'm not sure we would even have the necessary information in the
> compositor about whether a client wants release event ASAP or is it
> happy with throttling. I suppose we could track whether the
> wl_surface.commit that makes a wl_buffer reserved in the compositor
> also had a wl_surface.frame request and only in that case queue the
> release, otherwise post. I haven't thought it through.
>
> Frame callbacks are sent as the last thing in weston_output_repaint(),
> that has not changed. What did change is that there is now a delay
> between the pageflip event and the call to weston_output_repaint(). Now
> we also have the repaint grouping across outputs but I don't think that
> makes much of a difference. So I would say the situation has not
> changed wrt. to when the frame callbacks are sent out.
>
> Well, if we need a pageflip event to release a buffer, then yes, the
> frame callback will be coming later than in 2013. But that implies the
> client buffer was on scanout.
>
> I don't really like the idea that a client needs to "flush out" the
> release events somehow. That is like leaking compositor implementation
> details into clients. And that's exactly what asking for frame or sync
> callbacks is when you have no other use for them.
>
> I also don't quite like the idea of magically inferring whether a
> client is speed-runner game or a leisurely office app. We might even
> need explicit protocol for that, and then could use it for both buffer
> releases and input event throttling and coalescing.
>
> All in all, in the spirit of avoiding premature optimization, I'd go
> for the simple solution and let profiling drive more complicated
> solutions when the need arises. We're talking about buffer releases.
> Buffer releases happen when clients animate. Surely animation is heavy
> enough to obliterate any power savings we might get from optimizing
> when to flush out release events?

I completely agree. I'll submit something to wayland-devel on Monday.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols] Add zwp_linux_explicit_synchronization_v1

2017-09-20 Thread Matt Hoosier
On Tue, Sep 19, 2017 at 6:46 PM, Daniel Stone  wrote:
> Signed-off-by: Daniel Stone 
> ---
>  Makefile.am|   1 +
>  unstable/linux-explicit-synchronization/README |   5 +
>  .../linux-explicit-synchronization-unstable-v1.xml | 208 
> +
>  3 files changed, 214 insertions(+)
>  create mode 100644 unstable/linux-explicit-synchronization/README
>  create mode 100644 
> unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 5b5ae96..941692c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -15,6 +15,7 @@ unstable_protocols =
>   \
> 
> unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml  \
> 
> unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml
>  \
> unstable/xdg-output/xdg-output-unstable-v1.xml
>   \
> +   
> unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
>  \
> $(NULL)
>
>  stable_protocols =   
>   \
> diff --git a/unstable/linux-explicit-synchronization/README 
> b/unstable/linux-explicit-synchronization/README
> new file mode 100644
> index 000..fac116f
> --- /dev/null
> +++ b/unstable/linux-explicit-synchronization/README
> @@ -0,0 +1,5 @@
> +Linux explicit synchronization (dma-fence) protocol
> +
> +Maintainers:
> +David Reveman 
> +Daniel Stone 
> diff --git 
> a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
>  
> b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> new file mode 100644
> index 000..4a6636e
> --- /dev/null
> +++ 
> b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> @@ -0,0 +1,208 @@
> +
> +
> +
> +  
> +Copyright 2016 The Chromium Authors.
> +Copyright 2017 Intel Corporation
> +
> +Permission is hereby granted, free of charge, to any person obtaining a
> +copy of this software and associated documentation files (the 
> "Software"),
> +to deal in the Software without restriction, including without limitation
> +the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +and/or sell copies of the Software, and to permit persons to whom the
> +Software is furnished to do so, subject to the following conditions:
> +
> +The above copyright notice and this permission notice (including the next
> +paragraph) shall be included in all copies or substantial portions of the
> +Software.
> +
> +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> OR
> +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> OTHER
> +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +DEALINGS IN THE SOFTWARE.
> +  
> +
> +  
> +
> +  This global is a factory interface, allowing clients to request
> +  explicit synchronization for buffers on a per-surface basis.
> +
> +  See zwp_surface_synchronization_v1 for more information.
> +
> +  This interface is derived from Chromium's
> +  zcr_linux_explicit_synchronization_v1.
> +
> +  Warning! The protocol described in this file is experimental and
> +  backward incompatible changes may be made. Backward compatible changes
> +  may be added together with the corresponding interface version bump.
> +  Backward incompatible changes are done by bumping the version number in
> +  the protocol and interface names and resetting the interface version.
> +  Once the protocol is to be declared stable, the 'z' prefix and the
> +  version number in the protocol and interface names are removed and the
> +  interface version number is reset.
> +
> +
> +
> +  
> +   Destroy this explicit synchronization factory object. Other objects,
> +   including zwp_surface_synchronization_v1 objects created by this
> +   factory, shall not be affected by this request.
> +  
> +
> +
> +
> +   +summary="the surface already has a synchronization object 
> associated"/>
> +
> +
> +
> +  
> +   Instantiate an interface extension for the given wl_surface to
> +   provide explicit synchronization.
> +
> +   If the given wl_surface already has an explicit synchronization object
> +   associated, the synchronization_exists protocol error is raised.
> +  
> +
> +   +  summary="the new synchronization interface id"/>
> +   +  summary="the surface"/>
> +

Re: [PATCH] compositor-drm: fix z-order inversion in plane assignment

2017-09-05 Thread Matt Hoosier
On Tue, Sep 5, 2017 at 9:00 AM, Daniel Stone  wrote:
> It can't be correct to raise it to the cursor plane either, since both
> cursor and overlay planes strictly stack above the scanout plane. I
> guess this would read a lot easier with:
> if (picked_scanout)
> next_plane = primary;
> at the top of the loop.

Okay, sure. I think that in practice, no cursor surface would ever
have the right dimensions to get picked by prepare_overlay_view() or
prepare_scanout_view(), but I agree with you in principle.

Thanks for the feedback. I see that you've resubmitted the atomic
modesetting series again. Do you feel like it's near enough landing
that there's no use in trying to commit a copy of this fix to master?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v2] libweston-desktop: add signal for title/app-id changes

2017-09-05 Thread Matt Hoosier
As discussed on
https://lists.freedesktop.org/archives/wayland-devel/2017-August/034720.html,
it's useful for the shell implementation to know when these change,
for example to relay the information on to taskbars or similar.

To avoid ABI changes or the need to make the weston_desktop_surface
definition public, new functions are introduced for attaching
listeners to these signals.

Signed-off-by: Matt Hoosier 
---
 libweston-desktop/libweston-desktop.h |  3 +++
 libweston-desktop/surface.c   | 12 
 2 files changed, 15 insertions(+)

diff --git a/libweston-desktop/libweston-desktop.h 
b/libweston-desktop/libweston-desktop.h
index 03b04c7b..c43568ac 100644
--- a/libweston-desktop/libweston-desktop.h
+++ b/libweston-desktop/libweston-desktop.h
@@ -164,6 +164,9 @@ weston_desktop_surface_set_size(struct 
weston_desktop_surface *surface,
int32_t width, int32_t height);
 void
 weston_desktop_surface_close(struct weston_desktop_surface *surface);
+void
+weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
*surface,
+ struct wl_listener *listener);
 
 void *
 weston_desktop_surface_get_user_data(struct weston_desktop_surface *surface);
diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
index d3be9364..d00ba5d6 100644
--- a/libweston-desktop/surface.c
+++ b/libweston-desktop/surface.c
@@ -64,6 +64,7 @@ struct weston_desktop_surface {
char *title;
char *app_id;
pid_t pid;
+   struct wl_signal title_signal;
};
struct {
struct weston_desktop_surface *parent;
@@ -287,6 +288,8 @@ weston_desktop_surface_create(struct weston_desktop 
*desktop,
wl_list_init(&surface->view_list);
wl_list_init(&surface->grab_link);
 
+   wl_signal_init(&surface->title_signal);
+
return surface;
 }
 
@@ -511,6 +514,13 @@ weston_desktop_surface_close(struct weston_desktop_surface 
*surface)
   surface->implementation_data);
 }
 
+WL_EXPORT void
+weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
*surface,
+ struct wl_listener *listener)
+{
+   wl_signal_add(&surface->title_signal, listener);
+}
+
 struct weston_desktop_surface *
 weston_desktop_surface_from_client_link(struct wl_list *link)
 {
@@ -687,6 +697,7 @@ weston_desktop_surface_set_title(struct 
weston_desktop_surface *surface,
 
free(surface->title);
surface->title = tmp;
+   wl_signal_emit(&surface->title_signal, surface);
 }
 
 void
@@ -701,6 +712,7 @@ weston_desktop_surface_set_app_id(struct 
weston_desktop_surface *surface,
 
free(surface->app_id);
surface->app_id = tmp;
+   wl_signal_emit(&surface->title_signal, surface);
 }
 
 void
-- 
2.13.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] compositor-drm: fix z-order inversion in plane assignment

2017-08-29 Thread Matt Hoosier
On Thu, Aug 24, 2017 at 9:24 AM, Matt Hoosier  wrote:
> As discussed in the following thread:
>
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034755.html
>
> the existing plane assignment in the DRM backend is vulnerable to
> accidental masking of the intended fullscreen surface. This change
> adds a simple stateful memory to the plane assignment algorithm
> to prevent that.
> ---
>  libweston/compositor-drm.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8e1e788f..81dc8fe6 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1495,6 +1495,7 @@ drm_assign_planes(struct weston_output *output_base, 
> void *repaint_data)
> struct weston_view *ev, *next;
> pixman_region32_t overlap, surface_overlap;
> struct weston_plane *primary, *next_plane;
> +   bool picked_scanout;
>
> /*
>  * Find a surface for each sprite in the output using some heuristics:
> @@ -1516,6 +1517,8 @@ drm_assign_planes(struct weston_output *output_base, 
> void *repaint_data)
> output->cursor_plane.x = INT32_MIN;
> output->cursor_plane.y = INT32_MIN;
>
> +   picked_scanout = false;
> +
> wl_list_for_each_safe(ev, next, &output_base->compositor->view_list, 
> link) {
> struct weston_surface *es = ev->surface;
>
> @@ -1545,10 +1548,22 @@ drm_assign_planes(struct weston_output *output_base, 
> void *repaint_data)
> next_plane = primary;
> if (next_plane == NULL)
> next_plane = drm_output_prepare_cursor_view(output, 
> ev);
> -   if (next_plane == NULL)
> +
> +   /* If a higher-stacked view already got assigned to scanout, 
> it's incorrect to
> +* assign a subsequent (lower-stacked) view to scanout.
> +*/
> +   if (next_plane == NULL && !picked_scanout) {
> next_plane = drm_output_prepare_scanout_view(output, 
> ev);
> -   if (next_plane == NULL)
> +   if (next_plane)
> +   picked_scanout = true;
> +   }
> +
> +   /* Similarly, it's incorrect to elevate a view to an overlay 
> if some higher-stacked
> +* view is already identified as full-screen scanout.
> +*/
> +   if (next_plane == NULL && !picked_scanout)
> next_plane = drm_output_prepare_overlay_view(output, 
> ev);
> +
> if (next_plane == NULL)
> next_plane = primary;
>
> --
> 2.13.3
>

Any opinion on this being a correct fix for the problem discussed
previously in master's plane assignment loop, Daniel?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC] compositor-drm: name outputs according to connectors if EDID is unavailable

2017-08-24 Thread Matt Hoosier
On Thu, Aug 24, 2017 at 2:49 AM, Pekka Paalanen  wrote:
> Hi,
>
> using connector name to describe the connected monitor doesn't sound
> right to me.

I didn't really like it either. Thanks for the suggestions below.

>
> If you actually need the connector name in clients then let's
> design an extension that gives you exactly that instead of twisting the
> poorly defined semantics of wl_output make/model even further. We have
> long email threads discussing the design of such things already:
> https://lists.freedesktop.org/archives/wayland-devel/2017-May/034083.html
> https://lists.freedesktop.org/archives/wayland-devel/2017-August/034719.html
>
> However, I could imagine weston.ini options to set the make/model
> strings for a DRM connector. Like you say, EDID is not always
> available, but the system administrator or vendor may actually have the
> values and just needs a place to hard-code them. But, this raises a
> question, why not ship and inject an EDID blob through the kernel, so
> that you do not need to go fixing each and every compositor out there
> to pick up the hard-coded values?
>
> You could configure so much more in an EDID blob as well.
>
> From the Linux kernel-parameters.txt:
>
> 
> drm_kms_helper.edid_firmware=[:][,[:]]
> Broken monitors, graphic adapters, KVMs and EDIDless
> panels may send no or incorrect EDID data sets.
> This parameter allows to specify an EDID data sets
> in the /lib/firmware directory that are used instead.
> Generic built-in EDID data sets are used, if one of
> edid/1024x768.bin, edid/1280x1024.bin,
> edid/1680x1050.bin, or edid/1920x1080.bin is given
> and no file with the same name exists. Details and
> instructions how to build your own EDID data are
> available in Documentation/EDID/HOWTO.txt. An EDID
> data set will only be used for a particular connector,
> if its name and a colon are prepended to the EDID
> name. Each connector may use a unique EDID data
> set by separating the files with a comma.  An EDID
> data set with no connector name will be used for
> any connectors not explicitly specified.
>
> Is there a reason why you would not want to use this kernel DRM option?

No, the kernel support for manually supplying EDID information is much
better. I just didn't realize this KMS feature feature existed.

>
> What is the actual use case behind this patch? Why do you absolutely
> have to have a make/model?

The desire is to allow applications to distinguish which output to
select when making set_fullscreen() calls. I mentioned "embedded
devices", which probably gave the impression of something like a phone
or tablet. That's not quite my situation. The devices I'm thinking
about are multiheaded things built into the infrastructure of cars,
boats, etc, where multiple screens are common.

Thanks for the hint about drm_kms_helper.edid_firmware; I think that
does the job I need just fine.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Make DRM plane-assignment algorithm tolerant of more than one fullscreen opaque surface?

2017-08-24 Thread Matt Hoosier
On Tue, Aug 15, 2017 at 9:16 AM, Daniel Stone  wrote:
>
> No, that's all the way broken. I assume the only way this hasn't yet
> bitten us is that the background in stock desktop-shell is a SHM-only
> client, so won't get promoted to a plane.
>
> That should be fixed in the atomic branch though, AFAICT. I'd happily
> review a discrete fix, but if you can hold on then it should already
> (hopefully) be fixed.

Thanks for the background. I posted a possible fix for your review at
https://lists.freedesktop.org/archives/wayland-devel/2017-August/034845.html.
I'd find your feedback valuable even if you prefer just to wait for
atomics to merge to master, because I'm probably going to end up using
existing releases on Weston for quite some time.

-Matt
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] compositor-drm: fix z-order inversion in plane assignment

2017-08-24 Thread Matt Hoosier
As discussed in the following thread:

https://lists.freedesktop.org/archives/wayland-devel/2017-August/034755.html

the existing plane assignment in the DRM backend is vulnerable to
accidental masking of the intended fullscreen surface. This change
adds a simple stateful memory to the plane assignment algorithm
to prevent that.
---
 libweston/compositor-drm.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 8e1e788f..81dc8fe6 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1495,6 +1495,7 @@ drm_assign_planes(struct weston_output *output_base, void 
*repaint_data)
struct weston_view *ev, *next;
pixman_region32_t overlap, surface_overlap;
struct weston_plane *primary, *next_plane;
+   bool picked_scanout;
 
/*
 * Find a surface for each sprite in the output using some heuristics:
@@ -1516,6 +1517,8 @@ drm_assign_planes(struct weston_output *output_base, void 
*repaint_data)
output->cursor_plane.x = INT32_MIN;
output->cursor_plane.y = INT32_MIN;
 
+   picked_scanout = false;
+
wl_list_for_each_safe(ev, next, &output_base->compositor->view_list, 
link) {
struct weston_surface *es = ev->surface;
 
@@ -1545,10 +1548,22 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
next_plane = primary;
if (next_plane == NULL)
next_plane = drm_output_prepare_cursor_view(output, ev);
-   if (next_plane == NULL)
+
+   /* If a higher-stacked view already got assigned to scanout, 
it's incorrect to
+* assign a subsequent (lower-stacked) view to scanout.
+*/
+   if (next_plane == NULL && !picked_scanout) {
next_plane = drm_output_prepare_scanout_view(output, 
ev);
-   if (next_plane == NULL)
+   if (next_plane)
+   picked_scanout = true;
+   }
+
+   /* Similarly, it's incorrect to elevate a view to an overlay if 
some higher-stacked
+* view is already identified as full-screen scanout.
+*/
+   if (next_plane == NULL && !picked_scanout)
next_plane = drm_output_prepare_overlay_view(output, 
ev);
+
if (next_plane == NULL)
next_plane = primary;
 
-- 
2.13.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[RFC] compositor-drm: name outputs according to connectors if EDID is unavailable

2017-08-23 Thread Matt Hoosier
On embedded systems, EDID is typically unavailble for connectors on buses
such as LVDS or DSI. This change enhances the original opportunistic
naming done in 2b2092adb499d8cdc10390cf19876706bcfe69cb to report the
make and model of an output according to the KMS connector (e.g., "DSI-1")
if no dynamic information about an attached monitor is available for
the connector.
---
 libweston/compositor-drm.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 10adb46..ccae228 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2304,7 +2304,7 @@ edid_parse(struct drm_edid *edid, const uint8_t *data, 
size_t length)
return 0;
 }
 
-static void
+static bool
 find_and_parse_output_edid(struct drm_backend *b,
   struct drm_output *output,
   drmModeConnector *connector)
@@ -2326,7 +2326,7 @@ find_and_parse_output_edid(struct drm_backend *b,
drmModeFreeProperty(property);
}
if (!edid_blob)
-   return;
+   return false;
 
rc = edid_parse(&output->edid,
edid_blob->data,
@@ -2344,6 +2344,7 @@ find_and_parse_output_edid(struct drm_backend *b,
output->base.serial_number = output->edid.serial_number;
}
drmModeFreePropertyBlob(edid_blob);
+   return !rc;
 }
 
 
@@ -2650,7 +2651,12 @@ drm_output_enable(struct weston_output *base)
 
output->base.subpixel = 
drm_subpixel_to_wayland(output->connector->subpixel);
 
-   find_and_parse_output_edid(b, output, output->connector);
+   if (!find_and_parse_output_edid(b, output, output->connector)) {
+   /* Fall back to using connector names if EDID isn't applicable 
for the connector */
+   output->base.make = output->base.name;
+   output->base.model = output->base.name;
+   }
+
if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
output->connector->connector_type == DRM_MODE_CONNECTOR_eDP)
output->base.connection_internal = 1;
-- 
2.9.5

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Make DRM plane-assignment algorithm tolerant of more than one fullscreen opaque surface?

2017-08-10 Thread Matt Hoosier
Currently the DRM backend ends up picking the bottom-most view which meets
all the checks for eligibility, for direct scanout usage. If more than one
such view exists, we get a visual result counter to expectations -- it
should be the highest-sorted such view that the user sees.

The big loop in drm_assign_planes() that assigns views to planes iterates
top-down through the weston_layer's, and the particular way that the
results are progressively stored in variables means that if more than one
pass through the loop encounters a view which is fullscreen and opaque then
the last (bottom-most) of these passes is the one whose results are
preserved upon exit from the loop.

This normally isn't a problem because mostly only the desktop shell has a
notion of fullscreen surfaces. desktop-shell takes care (whether
intentionally or not I can't tell) that when running full-screen only the
logical topmost view is actively left in a visible weston_layer.

Is it informally expected that the shell must not allow more than one
fullscreen view at a time? Would there be interest in a patch to
drm_assign_planes() that adds a bit more awareness to the plane assignment
so that only the topmost fullscreen opaque view is picked for scanout?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] libweston-desktop: add signal title/app-id changes

2017-08-08 Thread Matt Hoosier
Okay, this one should be ready to review. Sorry for the noise on the first
attempt.

On Tue, Aug 8, 2017 at 10:56 AM, Matt Hoosier 
wrote:

> It's useful for the shell implementation to know when these change,
> for example to relay the information on to taskbars or similar.
>
> To avoid ABI changes or the need to make the weston_desktop_surface
> definition public, new functions are introduced for attaching
> listeners to these signals.
>
> Signed-off-by: Matt Hoosier 
> ---
>  libweston-desktop/libweston-desktop.h |  3 +++
>  libweston-desktop/surface.c   | 12 
>  2 files changed, 15 insertions(+)
>
> diff --git a/libweston-desktop/libweston-desktop.h
> b/libweston-desktop/libweston-desktop.h
> index 03b04c7b..c43568ac 100644
> --- a/libweston-desktop/libweston-desktop.h
> +++ b/libweston-desktop/libweston-desktop.h
> @@ -164,6 +164,9 @@ weston_desktop_surface_set_size(struct
> weston_desktop_surface *surface,
> int32_t width, int32_t height);
>  void
>  weston_desktop_surface_close(struct weston_desktop_surface *surface);
> +void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface
> *surface,
> + struct wl_listener *listener);
>
>  void *
>  weston_desktop_surface_get_user_data(struct weston_desktop_surface
> *surface);
> diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
> index d3be9364..d00ba5d6 100644
> --- a/libweston-desktop/surface.c
> +++ b/libweston-desktop/surface.c
> @@ -64,6 +64,7 @@ struct weston_desktop_surface {
> char *title;
> char *app_id;
> pid_t pid;
> +   struct wl_signal title_signal;
> };
> struct {
> struct weston_desktop_surface *parent;
> @@ -287,6 +288,8 @@ weston_desktop_surface_create(struct weston_desktop
> *desktop,
> wl_list_init(&surface->view_list);
> wl_list_init(&surface->grab_link);
>
> +   wl_signal_init(&surface->title_signal);
> +
> return surface;
>  }
>
> @@ -511,6 +514,13 @@ weston_desktop_surface_close(struct
> weston_desktop_surface *surface)
>
>  surface->implementation_data);
>  }
>
> +WL_EXPORT void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface
> *surface,
> + struct wl_listener *listener)
> +{
> +   wl_signal_add(&surface->title_signal, listener);
> +}
> +
>  struct weston_desktop_surface *
>  weston_desktop_surface_from_client_link(struct wl_list *link)
>  {
> @@ -687,6 +697,7 @@ weston_desktop_surface_set_title(struct
> weston_desktop_surface *surface,
>
> free(surface->title);
> surface->title = tmp;
> +   wl_signal_emit(&surface->title_signal, surface);
>  }
>
>  void
> @@ -701,6 +712,7 @@ weston_desktop_surface_set_app_id(struct
> weston_desktop_surface *surface,
>
> free(surface->app_id);
> surface->app_id = tmp;
> +   wl_signal_emit(&surface->title_signal, surface);
>  }
>
>  void
> --
> 2.13.3
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] libweston-desktop: add signal title/app-id changes

2017-08-08 Thread Matt Hoosier
This revision was broken. Disregard, I'm re-sending.

On Tue, Aug 8, 2017 at 10:45 AM, Matt Hoosier 
wrote:

> It's useful for the shell implementation to know when these change,
> for example to relay the information on to taskbars or similar.
>
> To avoid ABI changes or the need to make the weston_desktop_surface
> definition public, new functions are introduced for attaching
> listeners to these signals.
>
> Signed-off-by: Matt Hoosier 
> ---
>  libweston-desktop/libweston-desktop.h |  6 ++
>  libweston-desktop/surface.c   | 21 +
>  2 files changed, 27 insertions(+)
>
> diff --git a/libweston-desktop/libweston-desktop.h
> b/libweston-desktop/libweston-desktop.h
> index 03b04c7b..e38257e5 100644
> --- a/libweston-desktop/libweston-desktop.h
> +++ b/libweston-desktop/libweston-desktop.h
> @@ -164,6 +164,12 @@ weston_desktop_surface_set_size(struct
> weston_desktop_surface *surface,
> int32_t width, int32_t height);
>  void
>  weston_desktop_surface_close(struct weston_desktop_surface *surface);
> +void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface
> *surface,
> + struct wl_listener *listener);
> +void
> +weston_desktop_surface_add_app_id_listener(struct weston_desktop_surface
> *surface,
> +  struct wl_listener *listener);
>
>  void *
>  weston_desktop_surface_get_user_data(struct weston_desktop_surface
> *surface);
> diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
> index d3be9364..97a455c6 100644
> --- a/libweston-desktop/surface.c
> +++ b/libweston-desktop/surface.c
> @@ -64,6 +64,8 @@ struct weston_desktop_surface {
> char *title;
> char *app_id;
> pid_t pid;
> +   struct wl_signal title_signal;
> +   struct wl_signal app_id_signal;
> };
> struct {
> struct weston_desktop_surface *parent;
> @@ -287,6 +289,9 @@ weston_desktop_surface_create(struct weston_desktop
> *desktop,
> wl_list_init(&surface->view_list);
> wl_list_init(&surface->grab_link);
>
> +   wl_signal_init(&surface->title_signal);
> +   wl_signal_init(&surface->app_id_signal);
> +
> return surface;
>  }
>
> @@ -511,6 +516,20 @@ weston_desktop_surface_close(struct
> weston_desktop_surface *surface)
>
>  surface->implementation_data);
>  }
>
> +WL_EXPORT void
> +weston_desktop_surface_add_title_listener(struct weston_desktop_surface
> *surface,
> + struct wl_listener *listener)
> +{
> +   wl_signal_add(&surface->title_signal, listener);
> +}
> +
> +WL_EXPORT void
> +weston_desktop_surface_add_app_id_listener(struct weston_desktop_surface
> *surface,
> +  struct wl_listener *listener)
> +{
> +   wl_signal_add(&surface->app_id_signal, listener);
> +}
> +
>  struct weston_desktop_surface *
>  weston_desktop_surface_from_client_link(struct wl_list *link)
>  {
> @@ -687,6 +706,7 @@ weston_desktop_surface_set_title(struct
> weston_desktop_surface *surface,
>
> free(surface->title);
> surface->title = tmp;
> +   wl_signal_emit(&surface->title_signal, surface->title);
>  }
>
>  void
> @@ -701,6 +721,7 @@ weston_desktop_surface_set_app_id(struct
> weston_desktop_surface *surface,
>
> free(surface->app_id);
> surface->app_id = tmp;
> +   wl_signal_emit(&surface->app_id_signal, surface->app_id);
>  }
>
>  void
> --
> 2.13.3
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] libweston-desktop: add signal title/app-id changes

2017-08-08 Thread Matt Hoosier
It's useful for the shell implementation to know when these change,
for example to relay the information on to taskbars or similar.

To avoid ABI changes or the need to make the weston_desktop_surface
definition public, new functions are introduced for attaching
listeners to these signals.

Signed-off-by: Matt Hoosier 
---
 libweston-desktop/libweston-desktop.h |  3 +++
 libweston-desktop/surface.c   | 12 
 2 files changed, 15 insertions(+)

diff --git a/libweston-desktop/libweston-desktop.h 
b/libweston-desktop/libweston-desktop.h
index 03b04c7b..c43568ac 100644
--- a/libweston-desktop/libweston-desktop.h
+++ b/libweston-desktop/libweston-desktop.h
@@ -164,6 +164,9 @@ weston_desktop_surface_set_size(struct 
weston_desktop_surface *surface,
int32_t width, int32_t height);
 void
 weston_desktop_surface_close(struct weston_desktop_surface *surface);
+void
+weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
*surface,
+ struct wl_listener *listener);
 
 void *
 weston_desktop_surface_get_user_data(struct weston_desktop_surface *surface);
diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
index d3be9364..d00ba5d6 100644
--- a/libweston-desktop/surface.c
+++ b/libweston-desktop/surface.c
@@ -64,6 +64,7 @@ struct weston_desktop_surface {
char *title;
char *app_id;
pid_t pid;
+   struct wl_signal title_signal;
};
struct {
struct weston_desktop_surface *parent;
@@ -287,6 +288,8 @@ weston_desktop_surface_create(struct weston_desktop 
*desktop,
wl_list_init(&surface->view_list);
wl_list_init(&surface->grab_link);
 
+   wl_signal_init(&surface->title_signal);
+
return surface;
 }
 
@@ -511,6 +514,13 @@ weston_desktop_surface_close(struct weston_desktop_surface 
*surface)
   surface->implementation_data);
 }
 
+WL_EXPORT void
+weston_desktop_surface_add_title_listener(struct weston_desktop_surface 
*surface,
+ struct wl_listener *listener)
+{
+   wl_signal_add(&surface->title_signal, listener);
+}
+
 struct weston_desktop_surface *
 weston_desktop_surface_from_client_link(struct wl_list *link)
 {
@@ -687,6 +697,7 @@ weston_desktop_surface_set_title(struct 
weston_desktop_surface *surface,
 
free(surface->title);
surface->title = tmp;
+   wl_signal_emit(&surface->title_signal, surface);
 }
 
 void
@@ -701,6 +712,7 @@ weston_desktop_surface_set_app_id(struct 
weston_desktop_surface *surface,
 
free(surface->app_id);
surface->app_id = tmp;
+   wl_signal_emit(&surface->title_signal, surface);
 }
 
 void
-- 
2.13.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


  1   2   >