Re: Migrating Wayland & Weston to GitLab

2018-05-31 Thread Kristian Høgsberg 
On Thu, May 31, 2018 at 3:53 AM Pekka Paalanen  wrote:
>
> On Thu, 31 May 2018 08:47:48 +0100
> Daniel Stone  wrote:
>
> > Hi all,
> >
> > On 29 May 2018 at 10:59, Daniel Stone  wrote:
> > > I intend to migrate the wayland/wayland-protocols/wayland-web/weston
> > > repository hosting only (issues disabled, MR submission disabled) this
> > > evening. Anyone trying to push to git.fd.o will get an error message
> > > pointing them to the wiki page telling them how to configure their
> > > remotes and set up their accounts. Anyone having trouble with this is
> > > welcome to contact me and I can help figure it out.
> > >
> > > This leaves the wayland-build-tools and wayland-java repositories
> > > orphaned in cgit; both have been inactive for quite some time.
> >
> > I've done this now. For the meantime, I've left wayland-protocols back
> > on the old infrastructure; given the discussion about what we should
> > be doing with wayland-protocols, it seemed prudent to wait.
> >
> > The following people have access from the Wayland group as it stands:
> >   daniels, anholt, derek, bryce, whot, pq, krh, jwrdegoede, tomeu,
> > nroberts, dvdhrm, jadahl, jekstrand, rdp.effort, zubzub, sardemff7,
> > iksaif, bnf, uartie, darxus, sree
> >
> > I would propose to prune the following developers who are no longer
> > (TTBOMK) active in Wayland and haven't been for at least a couple of
> > years:
> >   anholt
> >   tomeu
> >   nroberts
> >   dvdhrm
> >   jekstrand
> >   iksaif
> >   bnf
> >   uartie
> >   darxus
> >   sree
> >
> > Pruning people wouldn't at all be a one-way process; everyone would of
> > course be more than welcome back if they rejoined, and I'd be happy to
> > see them keep contributing.
>
> Sounds good to me.
>
> > I also propose to give the following people 'master' permission,
> > allowing them to add/remove people from the project, as well as do
> > special things like force-push: myself, Derek, Kristian, and Pekka.
> >
> > Does anyone have any thoughts at all on this, or alternative
> > suggestions, or?
>
> Fine by me. I wonder if Kristian wants it, he has been totally silent
> on the Wayland side for years now, but I do like having one more admin
> there.

Silent, but still here :) I don't mind helping out with the admin role.

Kristian

> Thanks,
> pq
> ___
> 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 wayland-protocols v2] unstable/drm-lease: DRM lease protocol support

2018-05-31 Thread Pekka Paalanen 
On Wed, 30 May 2018 12:38:22 +
Marius-cristian Vlad  wrote:

> On Tue, 2018-05-29 at 17:10 +0300, Pekka Paalanen wrote:
> > On Mon, 12 Feb 2018 16:51:51 +0200
> > Marius Vlad  wrote:
> >   
> > > Simple protocol extension to manage DRM lease. Based on the work by
> > > Keith
> > > Packard in [1], respectively [2].
> > > 
> > > [1] https://cgit.freedesktop.org/mesa/drm/commit/?id=c4171535389d72
> > > e9135c9615cecd07b346fd6d7e
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.
> > > git/commit/?h=v4.15-rc9=62884cd386b876638720ef88374b31a84ca7ee5f
> > > 
> > > Signed-off-by: Marius Vlad 
> > > 
> > > Changes since v1:
> > > - added manager: advertise lease capability and manage the lease
> > > (Daniel Stone)
> > > - add request(s) for adding connector/crtc/plane to behave more
> > > like dmabuf (Daniel Stone)
> > > ---
> > >  Makefile.am  |   1 +
> > >  unstable/drm-lease/README|   4 +
> > >  unstable/drm-lease/drm-lease-unstable-v1.xml | 150
> > > +++
> > >  3 files changed, 155 insertions(+)
> > >  create mode 100644 unstable/drm-lease/README
> > >  create mode 100644 unstable/drm-lease/drm-lease-unstable-v1.xml  
> > 
> > Hi Marius,
> > 
> > it's great to have someone working on this!
> > 
> > I finally got a chance to look at it. Comments are inline as usual.
> > Most
> > of my questions call for an answer in the spec text.  
> 
> Thanks for taking the time to go over this. I have some minor follow-up
> questions bellow.

Hi Marius,

I have written some pondering as replies to your questions below. I hope
they make sense.

I had a chat with Keith Packard in IRC about some details of how DRM
leasing actually works. These details affect the implementations more,
but there might be something we need to take into account with the
protocol as well.

These are my notes from the chat:

- When there is a change with DRM leases, a normal DRM hotplug event
  will be emitted.

- Particularly Lessor needs to listen for hotplug events and re-query
  the state of all DRM leases. This is how Lessor finds out a lease has
  been terminated by a Lessee. This needs to happen also on gaining DRM
  master.

- VT-switching: When the original DRM master (Lessor) drops the master
  status, all DRM leases it has given out will be temporarily disabled.
  When Lessor regains DRM master, the disabled leases are automatically
  reinstated.

- Lessee can tell the difference between a permanent revoke and a
  temporary disable of a lease by the error it gets from the KMS API
  (drmModeSetCrtc, drmModePageFlip, drmModeAtomicCommit): EACCESS for
  temporary disable, and ENOENT for a revoked lease that is not coming
  back.

- If Lessee gets EACCESS, it should stand by and wait for hotplug
  events to try again with the hope that the lease comes back.


Some random thoughts from me:

- Need to make sure the client has enough information to listen for
  hotplug events in a multi-card system.

- If the Wayland client disconnects, revoke all its leases.
  Implementation-wise this is easy as the wl_resource gets
  destroyed.

- We probably do not want a Wayland event for compositor dropping DRM
  master, because it would be racy in the client against KMS calls and
  the client will notice on its own anyway.

- There could be a Wayland event for compositor gaining DRM master, to
  tell the client to try KMS again now, but it seems redundant: the DRM
  hotplug event provides the same, and needs to be listened to anyway
  if the client wants to react to hotplug on its leased connector.


> > > diff --git a/unstable/drm-lease/drm-lease-unstable-v1.xml
> > > b/unstable/drm-lease/drm-lease-unstable-v1.xml
> > > new file mode 100644
> > > index 000..907efb0
> > > --- /dev/null
> > > +++ b/unstable/drm-lease/drm-lease-unstable-v1.xml
> > > @@ -0,0 +1,150 @@
> > > +
> > > +

...

> > 
> > To me this sounds like a client needs to first open a DRM card node,
> > iterate through all the resources (is that even possible if it is not
> > a
> > DRM master?), look at the information and guess which ones the
> > compositor might be willing to lease out, and then through trial and
> > error maybe find something that will actually be accepted.  
> 
> Indeed, that's how the client demo/example does it now. 
> 
> > 
> > I do not like this plan.
> > 
> > It assumes that the client can open a DRM card node itself - this is
> > often not the case. Even display servers do not open the DRM card
> > node
> > themselves, they ask logind, so it is actually likely that the client
> > does not have the file system access to the card node.  
> 
> Well... I assumed that the client can do this on its own.

DRM leasing protocol could be used to allow direct display access from
application sandboxes.

There is one more problem I forgot about: multiple card nodes.
How would a client know which card nodes the compositor is using and
might be able to give out leases for?

Re: unique id for wayland objects

2018-05-31 Thread Pekka Paalanen 
On Thu, 31 May 2018 11:32:40 +0800
zou lan  wrote:

> >>The intended behaviour is that Weston repaints at most once for each
> >>display refresh cycle, the deadline being 7 ms (if using the default
> >>value of repaint-window) before the next vblank. In other words, there
> >>should be a significant delay (say, 50-90% of a refresh period) between
> >>receiving the pageflip event and repaint.  
> I can't understand this. I use weston 1.9 now, the repaint_msc is 7ms.
> our code always execute output_repaint_timer_handler() as soon as possible
> because msec always lower than 1ms. Why there is delay?

Hi Nancy,

the intention of the deliberate delay is explained in depth here:
http://ppaalanen.blogspot.com/2015/02/weston-repaint-scheduling.html


Thanks,
pq


pgp7r9o9gjlS5.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Migrating Wayland & Weston to GitLab

2018-05-31 Thread Pekka Paalanen 
On Thu, 31 May 2018 08:47:48 +0100
Daniel Stone  wrote:

> Hi all,
> 
> On 29 May 2018 at 10:59, Daniel Stone  wrote:
> > I intend to migrate the wayland/wayland-protocols/wayland-web/weston
> > repository hosting only (issues disabled, MR submission disabled) this
> > evening. Anyone trying to push to git.fd.o will get an error message
> > pointing them to the wiki page telling them how to configure their
> > remotes and set up their accounts. Anyone having trouble with this is
> > welcome to contact me and I can help figure it out.
> >
> > This leaves the wayland-build-tools and wayland-java repositories
> > orphaned in cgit; both have been inactive for quite some time.  
> 
> I've done this now. For the meantime, I've left wayland-protocols back
> on the old infrastructure; given the discussion about what we should
> be doing with wayland-protocols, it seemed prudent to wait.
> 
> The following people have access from the Wayland group as it stands:
>   daniels, anholt, derek, bryce, whot, pq, krh, jwrdegoede, tomeu,
> nroberts, dvdhrm, jadahl, jekstrand, rdp.effort, zubzub, sardemff7,
> iksaif, bnf, uartie, darxus, sree
> 
> I would propose to prune the following developers who are no longer
> (TTBOMK) active in Wayland and haven't been for at least a couple of
> years:
>   anholt
>   tomeu
>   nroberts
>   dvdhrm
>   jekstrand
>   iksaif
>   bnf
>   uartie
>   darxus
>   sree
> 
> Pruning people wouldn't at all be a one-way process; everyone would of
> course be more than welcome back if they rejoined, and I'd be happy to
> see them keep contributing.

Sounds good to me.

> I also propose to give the following people 'master' permission,
> allowing them to add/remove people from the project, as well as do
> special things like force-push: myself, Derek, Kristian, and Pekka.
> 
> Does anyone have any thoughts at all on this, or alternative
> suggestions, or?

Fine by me. I wonder if Kristian wants it, he has been totally silent
on the Wayland side for years now, but I do like having one more admin
there.


Thanks,
pq


pgpJEhLlJ478B.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] Fix a crash when unlocking or unconfining a pointer

2018-05-31 Thread Pekka Paalanen 
On Thu, 31 May 2018 00:16:14 -0700
Dima Ryazanov  wrote:

> On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen  wrote:
> 
> > On Thu, 10 May 2018 00:53:38 -0700
> > Dima Ryazanov  wrote:
> >  
> > > In GNOME (but not in Weston), if a window loses focus, the client first  
> > receives  
> > > the focus event, then the unlock/unconfine event. This causes toytoolkit  
> > to  
> > > dereference a NULL window when unlocking or unconfining the pointer.
> > >
> > > To repro:
> > > - Run weston-confine
> > > - Click the window
> > > - Alt-Tab away from it
> > >
> > > Result:
> > >
> > > [1606837.869] wl_keyboard@19.modifiers(63944, 524352, 0, 0, 0)
> > > [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
> > > [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
> > > [1606837.956] wl_pointer@18.frame()
> > > [1606837.961] zwp_confined_pointer_v1@26.unconfined()
> > > Segmentation fault (core dumped)
> > >
> > > To fix this, get the input from the window instead of the other way  
> > around.  
> > >
> > > Signed-off-by: Dima Ryazanov 
> > > ---
> > >  clients/window.c | 23 +--
> > >  1 file changed, 13 insertions(+), 10 deletions(-)

> > > @@ -4860,7 +4861,7 @@ window_lock_pointer(struct window *window, struct  
> > input *input)  
> > >  
> >  ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_ONESHOT);  
> > >   zwp_locked_pointer_v1_add_listener(locked_pointer,
> > >  _pointer_listener,
> > > -input);
> > > +window);  
> >
> > Now that this object will have a pointer to window, how do we safeguard
> > against window_destroy() getting called before we dispatch a locked or
> > unlocked event? Wouldn't that be necessary?
> >  
> 
> Ah, interesting, I did not think about that. I'll see if I can make it
> happen. I'm guessing window_destroy should call window_unconfine_pointer and
> window_unlock_pointer.

> > > @@ -4984,8 +4985,9 @@ window_confine_pointer_to_rectangles(struct window  
> > *window,  
> > >
> > >   zwp_confined_pointer_v1_add_listener(confined_pointer,
> > >_pointer_listener,
> > > -  input);
> > > +  window);  
> >
> > The same safeguard question here.
> >  
> > >
> > > + window->confined_input = input;  
> >
> > I was also wondering about what would clean up this field when 'input'
> > gets destroyed, but I suppose a proper event sequence will clean things
> > up before 'input' is destroyed.
> >  
> 
> Interesting. I can't convince myself that cleanup will happen correctly.
> Now that I think about it, I wonder if I should've kept "input" in event
> handlers - but instead of accessing input->pointer_focus, I should've added
> new variables (confined_window, locked_window?).
> 
> Anyways, I'll try a few things. Still trying to figure out how all of this
> works.

Good luck! I appreciate the effort and I would be exactly in the same
place, not having much clue about how window.c works today. :-)


Thanks,
pq


pgpDgKg5MCMok.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v9 8/9] weston: support clone mode on DRM-frontend

2018-05-31 Thread Pekka Paalanen 
On Wed, 30 May 2018 10:00:57 +
Marius-cristian Vlad  wrote:

> Hi, 
> 
> This no longer applies, due to man/weston-drm changes. 

Hi,

yeah, I should probably rebase and re-send.

> Fixing that I get "atomic: get couldn't commit new state: Invalid
> argument". Does that mean that HW doesn't support CRTC sharing you
> mention in the description? Am I using it properly?

Judging from the below, you are using it properly. Unfortunately the PC
graphics cards do not tend to support shared-CRTC clone mode anymore.
Even when they did, there were only specific combinations of connectors
that could be used. One might have more luck with some embedded boards.
The one I've tested with was an i.MX6 board.

Not having a fallback to independent-CRTC clone mode (needs a damage
tracking re-design) makes the clone mode feature hard to work with.
That combined with Weston's current inability to deal with mode setting
failures gracefully makes it a really bad user experience. I believe
the situation will get better eventually with the use of atomic
modesetting with TEST_ONLY.

> One more thing I found that using the following config:
> 
> [output]
> name=HDMI-A-1
> mode=current
> same-as=HDMI-A-2
> 
> [output]
> name=HDMI-A-2
> mode=current
> 
> I get ``Output 'HDMI-A-2' enabled with head(s) HDMI-A-1, HDMI-A-2.''
> 
> I would've expected  to see ``Output 'HDMI-A-1' enabled with head(s)
> HDMI-A-1, HDMI-A-2.'' I don't know of this matters or not... the other
> way around is the same (using in HDMI-A-2 section same-as=HDMI-A-1). 

Right. The name is usually chosen by the controlling output section.
The output section with the "same-as" key is a slave section, where all
other keys are ignored. Following the chain of "same-as" keys one finds
the controlling output section, identified by not having a "same-as"
key.

That's odd that the name didn't change with the opposite configuration.
I'll have to look into it.

To be honest, I haven't paid too much attention to the output naming,
as long as there is a unique name. In libweston the output name is
arbitrary, and it's just weston that uses a head name as an output name.

> On Thu, 2018-04-19 at 15:09 +0300, Pekka Paalanen wrote:
> > From: Pekka Paalanen 
> > 
> > Add a new output section key "same-as" for configuring clone mode. An
> > output marked "same-as" another output will be configured identically
> > to
> > the other output.
> > 
> > The current implementation supports only CRTC sharing for clone mode.
> > Independent CRTC clone mode cannot be supported until output layout
> > logic is moved from libweston into the frontend and libweston's
> > damage
> > tracking issues stemming from overlapping outputs are solved.
> > 
> > Quite a lot of infrastructure is needed to properly configure clone
> > mode. The implemented logic allows easy addition of independent CRTC
> > clone mode once libweston supports it. The idea is that wet_layoutput
> > is
> > the item to be laid out and all weston_outputs a wet_layoutput
> > contains show exactly the same area of the desktop.
> > 
> > The configuration logic attempts to automatically fall back to
> > creating
> > more weston_outputs when all heads do not work under the same
> > weston_output. For now, the fallback path ends with an error message.
> > 
> > Enabling a weston_output is bit complicated, because one needs to
> > first
> > collect all relevant heads, try to attach them all to the
> > weston_output,
> > and then back up head by head until enabling the weston_output
> > succeeds.
> > A new weston_output is created for the left-over heads and the
> > process
> > is repeated.
> > 
> > CRTC-sharing clone mode is the most efficient clone mode, offering
> > synchronized scanout timings, but it is not always supported by
> > hardware.
> > 
> > v9:
> > - replace weston_compositor_set_heads_changed_cb() with
> >   weston_compositor_add_heads_changed_listener()
> > - remove workaround in simple_head_enable()
> > 
> > v6:
> > - Add man-page note about cms-colord.
> > - Don't create an output just to turn it off.
> > 
> > Fixes: https://emea01.safelinks.protection.outlook.com/?url=https%3A%
> > 2F%2Fphabricator.freedesktop.org%2FT7727=02%7C01%7Cmarius-
> > cristian.vlad%40nxp.com%7C2382a4e34bb74b66b07c08d5a5ee862a%7C686ea1d3
> > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636597366236269669=xoi2xcBR3
> > YD9gBAUidQ%2BmoO6oH0QhX3HIvGbGYqySU0%3D=0

Whoa, that was not in my patch.

> > 
> > Signed-off-by: Pekka Paalanen 
> > Acked-by: Derek Foreman 
> > ---
> >  compositor/main.c  | 492
> > ++---
> >  man/weston-drm.man |  12 ++
> >  2 files changed, 484 insertions(+), 20 deletions(-)

Thanks,
pq


pgpvK9IOxJMuv.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [writing a compositor] Displaying something on the screen

2018-05-31 Thread Quentin Glidic 

Hi,

On 5/31/18 5:42 AM, adlo wrote:

I am attempting to write a simple test compositor using libweston.

Here is my code so far:

https://github.com/adlocode/test-libweston-desktop

What else do I need to do in order to get it to the point where something 
appears on the screen, such as a rectangle of solid colour?


For regular (desktop) clients, you’ll need to properly implement the 
libweston-desktop hooks (at least the two mandatory ones), create views 
for surfaces, put them in a layer, and making sure a repaint will 
happen. You can find the very minimal libweston(-desktop) compositor 
here[1].


For other more specific clients, you’ll need to implement the relevant 
protocols and view creation. Or you can use libweston/Weston plugins[2] 
handling that.


libweston allows to create a solid colour surface, which is used for the 
black-surface-under-fullscreen requirement[3] and by some compositors to 
give a default black background.


I hope this helps.

Cheers,


[1] 
[2] 
[3] 



--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Migrating Wayland & Weston to GitLab

2018-05-31 Thread Erik De Rijcke 
Hi All,

First of all I'd like to say that the move to Gitlab makes me really happy
\o/! It will definitely  lower the contribution barrier for a lot of people
(including me!) as things are now far more accessible, visible and overall
easier to manage.

Which brings me to a remark/question on how merge requests are done. Is
there any plan to allow/move merge requests to Gitlab? Having a central
point of all things code related would really make things clear and
visible, and overall easier to contribute.
Being able to utilize Gitlab, manage your account, create your own fork and
then having to do a git-send-email would really defeat the point of the
whole move to Gitlab imho.

Anyway, just my 2 cents. Very glad to see this Gitlab thing moving forward!

Erik 'zubzub' De Rijcke

2018-05-31 9:47 GMT+02:00 Daniel Stone :

> Hi all,
>
> On 29 May 2018 at 10:59, Daniel Stone  wrote:
> > I intend to migrate the wayland/wayland-protocols/wayland-web/weston
> > repository hosting only (issues disabled, MR submission disabled) this
> > evening. Anyone trying to push to git.fd.o will get an error message
> > pointing them to the wiki page telling them how to configure their
> > remotes and set up their accounts. Anyone having trouble with this is
> > welcome to contact me and I can help figure it out.
> >
> > This leaves the wayland-build-tools and wayland-java repositories
> > orphaned in cgit; both have been inactive for quite some time.
>
> I've done this now. For the meantime, I've left wayland-protocols back
> on the old infrastructure; given the discussion about what we should
> be doing with wayland-protocols, it seemed prudent to wait.
>
> The following people have access from the Wayland group as it stands:
>   daniels, anholt, derek, bryce, whot, pq, krh, jwrdegoede, tomeu,
> nroberts, dvdhrm, jadahl, jekstrand, rdp.effort, zubzub, sardemff7,
> iksaif, bnf, uartie, darxus, sree
>
> I would propose to prune the following developers who are no longer
> (TTBOMK) active in Wayland and haven't been for at least a couple of
> years:
>   anholt
>   tomeu
>   nroberts
>   dvdhrm
>   jekstrand
>   iksaif
>   bnf
>   uartie
>   darxus
>   sree
>
> Pruning people wouldn't at all be a one-way process; everyone would of
> course be more than welcome back if they rejoined, and I'd be happy to
> see them keep contributing.
>
> I also propose to give the following people 'master' permission,
> allowing them to add/remove people from the project, as well as do
> special things like force-push: myself, Derek, Kristian, and Pekka.
>
> Does anyone have any thoughts at all on this, or alternative suggestions,
> or?
>
> 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: Migrating Wayland & Weston to GitLab

2018-05-31 Thread Daniel Stone 
Hi all,

On 29 May 2018 at 10:59, Daniel Stone  wrote:
> I intend to migrate the wayland/wayland-protocols/wayland-web/weston
> repository hosting only (issues disabled, MR submission disabled) this
> evening. Anyone trying to push to git.fd.o will get an error message
> pointing them to the wiki page telling them how to configure their
> remotes and set up their accounts. Anyone having trouble with this is
> welcome to contact me and I can help figure it out.
>
> This leaves the wayland-build-tools and wayland-java repositories
> orphaned in cgit; both have been inactive for quite some time.

I've done this now. For the meantime, I've left wayland-protocols back
on the old infrastructure; given the discussion about what we should
be doing with wayland-protocols, it seemed prudent to wait.

The following people have access from the Wayland group as it stands:
  daniels, anholt, derek, bryce, whot, pq, krh, jwrdegoede, tomeu,
nroberts, dvdhrm, jadahl, jekstrand, rdp.effort, zubzub, sardemff7,
iksaif, bnf, uartie, darxus, sree

I would propose to prune the following developers who are no longer
(TTBOMK) active in Wayland and haven't been for at least a couple of
years:
  anholt
  tomeu
  nroberts
  dvdhrm
  jekstrand
  iksaif
  bnf
  uartie
  darxus
  sree

Pruning people wouldn't at all be a one-way process; everyone would of
course be more than welcome back if they rejoined, and I'd be happy to
see them keep contributing.

I also propose to give the following people 'master' permission,
allowing them to add/remove people from the project, as well as do
special things like force-push: myself, Derek, Kristian, and Pekka.

Does anyone have any thoughts at all on this, or alternative suggestions, or?

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

Re: [PATCH weston] Fix a crash when unlocking or unconfining a pointer

2018-05-31 Thread Dima Ryazanov 
On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen  wrote:

> On Thu, 10 May 2018 00:53:38 -0700
> Dima Ryazanov  wrote:
>
> > In GNOME (but not in Weston), if a window loses focus, the client first
> receives
> > the focus event, then the unlock/unconfine event. This causes toytoolkit
> to
> > dereference a NULL window when unlocking or unconfining the pointer.
> >
> > To repro:
> > - Run weston-confine
> > - Click the window
> > - Alt-Tab away from it
> >
> > Result:
> >
> > [1606837.869] wl_keyboard@19.modifiers(63944, 524352, 0, 0, 0)
> > [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
> > [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
> > [1606837.956] wl_pointer@18.frame()
> > [1606837.961] zwp_confined_pointer_v1@26.unconfined()
> > Segmentation fault (core dumped)
> >
> > To fix this, get the input from the window instead of the other way
> around.
> >
> > Signed-off-by: Dima Ryazanov 
> > ---
> >  clients/window.c | 23 +--
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> >
>
> Hi Dima,
>
> thank you for the patch, and sorry, I know you have some very old
> patches still waiting for reviews.
>

Haha, no worries!


> > diff --git a/clients/window.c b/clients/window.c
> > index bcf2b017..dee4455f 100644
> > --- a/clients/window.c
> > +++ b/clients/window.c
> > @@ -286,6 +286,7 @@ struct window {
> >   confined_pointer_unconfined_handler_t pointer_unconfined_handler;
> >
> >   struct zwp_confined_pointer_v1 *confined_pointer;
> > + struct input *confined_input;
> >   struct widget *confined_widget;
> >   bool confined;
> >
> > @@ -4788,8 +4789,8 @@ static void
> >  locked_pointer_locked(void *data,
> > struct zwp_locked_pointer_v1 *locked_pointer)
> >  {
> > - struct input *input = data;
> > - struct window *window = input->pointer_focus;
> > + struct window *window = data;
> > + struct input *input = window->locked_input;
> >
> >   window->pointer_locked = true;
> >
> > @@ -4804,8 +4805,8 @@ static void
> >  locked_pointer_unlocked(void *data,
> >   struct zwp_locked_pointer_v1 *locked_pointer)
> >  {
> > - struct input *input = data;
> > - struct window *window = input->pointer_focus;
> > + struct window *window = data;
> > + struct input *input = window->locked_input;
> >
> >   window_unlock_pointer(window);
> >
> > @@ -4860,7 +4861,7 @@ window_lock_pointer(struct window *window, struct
> input *input)
> >
>  ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_ONESHOT);
> >   zwp_locked_pointer_v1_add_listener(locked_pointer,
> >  _pointer_listener,
> > -input);
> > +window);
>
> Now that this object will have a pointer to window, how do we safeguard
> against window_destroy() getting called before we dispatch a locked or
> unlocked event? Wouldn't that be necessary?
>

Ah, interesting, I did not think about that. I'll see if I can make it
happen. I'm guessing window_destroy should call window_unconfine_pointer and
window_unlock_pointer.

>
> >   window->locked_input = input;
> >   window->locked_pointer = locked_pointer;
> > @@ -4902,8 +4903,8 @@ static void
> >  confined_pointer_confined(void *data,
> > struct zwp_confined_pointer_v1 *confined_pointer)
> >  {
> > - struct input *input = data;
> > - struct window *window = input->pointer_focus;
> > + struct window *window = data;
> > + struct input *input = window->confined_input;
> >
> >   window->confined = true;
> >
> > @@ -4918,8 +4919,8 @@ static void
> >  confined_pointer_unconfined(void *data,
> >   struct zwp_confined_pointer_v1
> *confined_pointer)
> >  {
> > - struct input *input = data;
> > - struct window *window = input->pointer_focus;
> > + struct window *window = data;
> > + struct input *input = window->confined_input;
> >
> >   window_unconfine_pointer(window);
> >
> > @@ -4984,8 +4985,9 @@ window_confine_pointer_to_rectangles(struct window
> *window,
> >
> >   zwp_confined_pointer_v1_add_listener(confined_pointer,
> >_pointer_listener,
> > -  input);
> > +  window);
>
> The same safeguard question here.
>
> >
> > + window->confined_input = input;
>
> I was also wondering about what would clean up this field when 'input'
> gets destroyed, but I suppose a proper event sequence will clean things
> up before 'input' is destroyed.
>

Interesting. I can't convince myself that cleanup will happen correctly.
Now that I think about it, I wonder if I should've kept "input" in event
handlers - but instead of accessing input->pointer_focus, I should've added
new variables (confined_window, locked_window?).

Anyways, I'll try a few things. Still trying to figure out how