Re: Same ilm surface on multiple layer support

[Vikas Patil]

Hi Emre,

Could you please suggest on this blocking behavior of LayerManagerControl
with multi screen/layer?

Thank You.

Best Regards,
Vikash

On Wed, Apr 11, 2018 at 11:35 AM, Vikas Patil  wrote:

> Hi Emre Ucan,
>
> Thanks a lot for your quick response. I am able to show same surface on
> two layers now. I have taken following two commit to weston 1.11.0.
> Attached here same as  patch to weston 1.11.0.
>
> "5e8d55da698e58"
> "67bd21232fa549"
>
> However if I use any of the below commands to analyze then it is not
> exiting and I need to prress "CTRl+C" to come out from command. Do you know
> if this is the normal behavior or some fix is available for this ?
>
> root@linux-a1 :~# LayerManagerControl analyze surface 10
> ^C
>
> root@ linux-a1 :~# LayerManagerControl get scene
> screen 0 (0x0)
> ---
> - resolution:   x=800, y=480
> - hardware layer count: 0
> - layer render order:   1000(0x3e8), 2000(0x7d0),
>
> layer 1000 (0x3e8)
> ---
> - created by pid:   0
> - original size:x=400, y=480
> - destination region:   x=0, y=0, w=400, h=480
> - source region:x=0, y=0, w=400, h=480
> - orientation:  0 (up is top)
> - opacity:  1
> - visibility:   1
> - type: 0 (unknown)
> ^C
>
> root@linux-a1:~# LayerManagerControl get layer 1000
> layer 1000 (0x3e8)
> ---
> - created by pid:   0
> - original size:x=400, y=480
> - destination region:   x=0, y=0, w=400, h=480
> - source region:x=0, y=0, w=400, h=480
> - orientation:  0 (up is top)
> - opacity:  1
> - visibility:   1
> - type: 0 (unknown)
> ^C
>
> root@orinoco-9939-a1:~# LayerManagerControl get surface 10
> surface 10 (0xa)
> ---
> - created by pid:   821
> - original size:  x=800, y=480
> - destination region: x=0, y=0, w=400, h=480
> - source region:  x=0, y=0, w=800, h=480
> - orientation:0 (up is top)
> - opacity:1
> - visibility: 1
> - pixel format:   0 (R-8)
> - native surface: 0
> - counters:   frame=0, draw=0, update=0
> ^C
>
>
> Also following commands worked successfully.
>
>
> LayerManagerControl get screen 0
> LayerManagerControl get layer 2000
> LayerManagerControl get layers
> LayerManagerControl get surfaces
>
> I used following commands to setup  and test
>
> export XDG_RUNTIME_DIR=/var/run/root/1000
>
> LayerManagerControl create layer 1000 400 480
> LayerManagerControl set layer 1000 visibility 1
> LayerManagerControl set layer 1000 destination region 0 0 400 480
>
> LayerManagerControl create layer 2000 400 480
> LayerManagerControl set layer 2000 visibility 1
> LayerManagerControl set layer 2000 destination region 400 0 400 480
>
> LayerManagerControl set screen 0 render order 1000,2000
>
> EGLWLMockNavigation &
> LayerManagerControl add surface 10 to layer 1000
> LayerManagerControl add surface 10 to layer 2000
> LayerManagerControl set surface 10 visibility 1
> LayerManagerControl set surface 10 source region 0 0 800 480
> LayerManagerControl set surface 10 destination region 0 0 400 480
>
> Best Regards,
> Vikash
>
> On Tue, Apr 10, 2018 at 7:43 PM, Ucan, Emre (ADITG/ESB) <
> eu...@de.adit-jv.com> wrote:
>
>> Hi Vikas,
>>
>>
>>
>> This patch “5e8d55da698e58”  enabled the feature. It is part of weston
>> 1.12 release.
>>
>>
>>
>> Best regards
>>
>> *Emre Ucan*
>> Engineering Software Base (ADITG/ESB)
>>
>> Tel. +49 5121 49 6937
>>
>> *From:* wayland-devel [mailto:wayland-devel-boun...@lists.freedesktop.org]
>> *On Behalf Of *Vikas Patil
>> *Sent:* Dienstag, 10. April 2018 14:58
>> *To:* genivi-ivi-layer-managem...@lists.genivi.org; Mizuno, Wataru
>> (ADITJ/SWG); wayland mailing list
>> *Subject:* Same ilm surface on multiple layer support
>>
>>
>>
>> +Subject
>>
>> Dear All,
>>
>> We are facing issue when we are trying to add same surface to multiple
>> layers. When we try to attach surface to another layer, it is getting
>> detached from the earlier layer.
>>
>> We are using wayland/weston/wayland-ivi-extension 1.11.0 with
>> drm-backend on TI's Soc.
>>
>> Could anyone know if this is the limitation of ILM 1.11.0 ? Is this fixed
>> in newer version and can it be ported to 1.11.0 ? or Is there any other way
>> to show same surface on multiple layers?
>>
>> I see it was the limitation with wayland-ivi-extesnion 1.9.0 as below
>> [1].
>>
>>
>>
>> *"Currently 1 layer can be only on 1 screen, and 1 surface can be only on 1 
>> layer, we are planning to relax this limitation And allow 1 surface to be on 
>> many layers but we would need to break the ABI and change the  
>> ivi-controller protocol."*
>>
>> [1] https://lists.genivi.org/pipermail/genivi-ivi-layer-manageme
>> nt/2016-October/005416.html
>>
>>
>>
>> Thanking you in advance.
>>
>>
>>
>> 

Re: [PATCH v2 0/3] Deal with destroy signal use after free issues

[Derek Foreman]

On 2018-04-16 04:29 PM, Markus Ongyerth wrote:
> Hi,
> 
> Thanks for getting to this. I was waiting for the release, but I'm currently 
> not at full capacity, so you got it before me.
> 
> The commit message of patch 1 contains a lie. The second paragraph should 
> contain "IF there was only one listener object", which the testcase in Patch 
> 3 
> shows. But I don't think that's a deal breaker.

Oops, you're absolutely right, I didn't re-write the text after seeing
your test case.  Until then I hadn't thought about that case.  (I'm
still surprised it took us until now to actually hit this.)

If none object I'll correct that while landing.  If another revision is
required I'll update the text then.

> For Patch 1/2:
> Reviewed-by: Markus Ongyerth 
> 
> I'm fine with moving/resubmit of my code and am happy I could provide the 
> testcase that shows an issue.
> Since it's originally authored by me, I guess my R-B would be weird there :)

I didn't put my R-B on it because I made a (mostly cosmetic) change to
it, and wasn't sure if that was ok.

Thanks,
Derek

> Cheers,
> ongy
> 
> On 2018/April/16 03:00, Derek Foreman wrote:
>> Now that the release is out, I'd like to dig back into this mess.
>> This is a round up of some patches that were on list shortly before
>> the release to deal with a problem where many existing libwayland
>> users don't delete their destroy signal listeners before freeing
>> them.
>>
>> These leads to a bit of a mess (as Markus' test illustrates) if there
>> are multiple destroy listeners.
>>
>> I've included:
>> My test patch to ensure the existing behaviour continues to work
>> (users like weston and enlightenment can free during destroy
>> listener)
>>
>> The special case destroy emit path for wl_priv_signal - this is
>> an attempt to "fix" the problem, by making the destroy signal
>> emit operate without ever touching potentially free()d elements
>> again.
>>
>> Markus' test that would fail without patch 2/3, as it catches the
>> free() without removal case we've all come to know any love.
>>
>> Derek Foreman (2):
>>   tests: Test for use after free in resource destruction signals
>>changes since first appearance: none
>>
>>   server: Add special case destroy signal emitter
>>changes since first appearance:  stop trying to maintain a list head
>>
>> Markus Ongyerth (1):
>>   tests: Add free-without-remove test
>>changes since first appearance: I moved it into an existing file
>>
>>  src/wayland-private.h  |  3 +++
>>  src/wayland-server.c   | 46 +++---
>>  tests/resources-test.c | 39 +++
>>  3 files changed, 85 insertions(+), 3 deletions(-)
>>
>> -- 
>> 2.14.3
>>

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

Re: [PATCH v2 0/3] Deal with destroy signal use after free issues

[Markus Ongyerth]

Hi,

Thanks for getting to this. I was waiting for the release, but I'm currently 
not at full capacity, so you got it before me.

The commit message of patch 1 contains a lie. The second paragraph should 
contain "IF there was only one listener object", which the testcase in Patch 3 
shows. But I don't think that's a deal breaker.

For Patch 1/2:
Reviewed-by: Markus Ongyerth 

I'm fine with moving/resubmit of my code and am happy I could provide the 
testcase that shows an issue.
Since it's originally authored by me, I guess my R-B would be weird there :)

Cheers,
ongy

On 2018/April/16 03:00, Derek Foreman wrote:
> Now that the release is out, I'd like to dig back into this mess.
> This is a round up of some patches that were on list shortly before
> the release to deal with a problem where many existing libwayland
> users don't delete their destroy signal listeners before freeing
> them.
> 
> These leads to a bit of a mess (as Markus' test illustrates) if there
> are multiple destroy listeners.
> 
> I've included:
> My test patch to ensure the existing behaviour continues to work
> (users like weston and enlightenment can free during destroy
> listener)
> 
> The special case destroy emit path for wl_priv_signal - this is
> an attempt to "fix" the problem, by making the destroy signal
> emit operate without ever touching potentially free()d elements
> again.
> 
> Markus' test that would fail without patch 2/3, as it catches the
> free() without removal case we've all come to know any love.
> 
> Derek Foreman (2):
>   tests: Test for use after free in resource destruction signals
>changes since first appearance: none
> 
>   server: Add special case destroy signal emitter
>changes since first appearance:  stop trying to maintain a list head
> 
> Markus Ongyerth (1):
>   tests: Add free-without-remove test
>changes since first appearance: I moved it into an existing file
> 
>  src/wayland-private.h  |  3 +++
>  src/wayland-server.c   | 46 +++---
>  tests/resources-test.c | 39 +++
>  3 files changed, 85 insertions(+), 3 deletions(-)
> 
> -- 
> 2.14.3
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2 2/3] server: Add special case destroy signal emitter

[Simon Ser]

> In the past much code (weston, efl/enlightenment, mutter) has
> freed structures containing wl_listeners from destroy handlers
> without first removing the listener from the signal.  As the
> destroy notifier only fires once, this has largely gone
> unnoticed until recently.
> 
> Other code does not (Qt, wlroots) - and removes itself from
> the signal before free.
> 
> If somehow a destroy signal is listened to by code from both
> kinds of callers, those that free will corrupt the lists for
> those that don't, and Bad Things will happen.
> 
> To avoid these bad things, remove every item from the signal list
> during destroy emit, and put it in a list all its own.  This way
> whether the listener is removed or not has no impact on the
> following emits.
> 
> Signed-off-by: Derek Foreman 

Thanks for looking into this, I think it's a great idea to add this behaviour to
the ABI. This patch LGTM.

Reviewed-by: Simon Ser 

> ---
> 
> Changes since v1:
>  In v1 I went through some ugly steps to ensure wl_priv_signal_get()
>  worked.  It seems this is actually a useless requirement, and the
>  code is prettier without it.
> 
>  src/wayland-private.h |  3 +++
>  src/wayland-server.c  | 46 +++---
>  2 files changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/src/wayland-private.h b/src/wayland-private.h
> index 12b9032..29516ec 100644
> --- a/src/wayland-private.h
> +++ b/src/wayland-private.h
> @@ -253,6 +253,9 @@ wl_priv_signal_get(struct wl_priv_signal *signal, 
> wl_notify_func_t notify);
>  void
>  wl_priv_signal_emit(struct wl_priv_signal *signal, void *data);
>  
> +void
> +wl_priv_signal_final_emit(struct wl_priv_signal *signal, void *data);
> +
>  void
>  wl_connection_close_fds_in(struct wl_connection *connection, int max);
>  
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index eb1e500..8c3b800 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -682,7 +682,7 @@ destroy_resource(void *element, void *data, uint32_t 
> flags)
>   /* Don't emit the new signal for deprecated resources, as that would
>* access memory outside the bounds of the deprecated struct */
>   if (!resource_is_deprecated(resource))
> - wl_priv_signal_emit(>destroy_signal, resource);
> + wl_priv_signal_final_emit(>destroy_signal, resource);
>  
>   if (resource->destroy)
>   resource->destroy(resource);
> @@ -841,7 +841,7 @@ wl_client_destroy(struct wl_client *client)
>  {
>   uint32_t serial = 0;
>  
> - wl_priv_signal_emit(>destroy_signal, client);
> + wl_priv_signal_final_emit(>destroy_signal, client);
>  
>   wl_client_flush(client);
>   wl_map_for_each(>objects, destroy_resource, );
> @@ -1089,7 +1089,7 @@ wl_display_destroy(struct wl_display *display)
>   struct wl_socket *s, *next;
>   struct wl_global *global, *gnext;
>  
> - wl_priv_signal_emit(>destroy_signal, display);
> + wl_priv_signal_final_emit(>destroy_signal, display);
>  
>   wl_list_for_each_safe(s, next, >socket_list, link) {
>   wl_socket_destroy(s);
> @@ -2025,6 +2025,46 @@ wl_priv_signal_emit(struct wl_priv_signal *signal, 
> void *data)
>   }
>  }
>  
> +/** Emit the signal for the last time, calling all the installed listeners
> + *
> + * Iterate over all the listeners added to this \a signal and call
> + * their \a notify function pointer, passing on the given \a data.
> + * Removing or adding a listener from within wl_priv_signal_emit()
> + * is safe, as is freeing the structure containing the listener.
> + *
> + * A large body of external code assumes it's ok to free a destruction
> + * listener without removing that listener from the list.  Mixing code
> + * that acts like this and code that doesn't will result in list
> + * corruption.
> + *
> + * We resolve this by removing each item from the list and isolating it
> + * in another list.  We discard it completely after firing the notifier.
> + * This should allow interoperability between code that unlinks its
> + * destruction listeners and code that just frees structures they're in.
> + *
> + */
> +void
> +wl_priv_signal_final_emit(struct wl_priv_signal *signal, void *data)
> +{
> + struct wl_listener *l;
> + struct wl_list *pos;
> +
> + /* During a destructor notifier isolate every list item before
> +  * notifying.  This renders harmless the long standing misuse
> +  * of freeing listeners without removing them, but allows
> +  * callers that do choose to remove them to interoperate with
> +  * ones that don't. */
> + while (!wl_list_empty(>listener_list)) {
> + pos = signal->listener_list.next;
> + l = wl_container_of(pos, l, link);
> +
> + wl_list_remove(pos);
> + wl_list_init(pos);
> +
> + l->notify(l, data);
> + }
> +}
> +
>  /** \endcond INTERNAL */
>  
>  /** \cond */ 

[PATCH v2 3/3] tests: Add free-without-remove test

[Derek Foreman]

From: Markus Ongyerth 

[Derek Foreman  moved this into resources-test]

---

I moved this behind Markus' back, so let's not go landing it if he's
not ok with that change.  I think it's a great illustration of the
problem and would like to see it land though.

 tests/resources-test.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/tests/resources-test.c b/tests/resources-test.c
index 76c9eb8..fa6ba2b 100644
--- a/tests/resources-test.c
+++ b/tests/resources-test.c
@@ -182,3 +182,27 @@ TEST(create_resource_with_same_id)
wl_display_destroy(display);
close(s[1]);
 }
+
+static void
+display_destroy_notify(struct wl_listener *l, void *data)
+{
+   l->link.prev = l->link.next = NULL;
+}
+
+TEST(free_without_remove)
+{
+   struct wl_display *display;
+   struct wl_listener a, b;
+
+   display = wl_display_create();
+   a.notify = display_destroy_notify;
+   b.notify = display_destroy_notify;
+
+   wl_display_add_destroy_listener(display, );
+   wl_display_add_destroy_listener(display, );
+
+   wl_display_destroy(display);
+
+   assert(a.link.next == a.link.prev && a.link.next == NULL);
+   assert(b.link.next == b.link.prev && b.link.next == NULL);
+}
-- 
2.14.3

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

[PATCH v2 2/3] server: Add special case destroy signal emitter

[Derek Foreman]

In the past much code (weston, efl/enlightenment, mutter) has
freed structures containing wl_listeners from destroy handlers
without first removing the listener from the signal.  As the
destroy notifier only fires once, this has largely gone
unnoticed until recently.

Other code does not (Qt, wlroots) - and removes itself from
the signal before free.

If somehow a destroy signal is listened to by code from both
kinds of callers, those that free will corrupt the lists for
those that don't, and Bad Things will happen.

To avoid these bad things, remove every item from the signal list
during destroy emit, and put it in a list all its own.  This way
whether the listener is removed or not has no impact on the
following emits.

Signed-off-by: Derek Foreman 
---

Changes since v1:
 In v1 I went through some ugly steps to ensure wl_priv_signal_get()
 worked.  It seems this is actually a useless requirement, and the
 code is prettier without it.

 src/wayland-private.h |  3 +++
 src/wayland-server.c  | 46 +++---
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/wayland-private.h b/src/wayland-private.h
index 12b9032..29516ec 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -253,6 +253,9 @@ wl_priv_signal_get(struct wl_priv_signal *signal, 
wl_notify_func_t notify);
 void
 wl_priv_signal_emit(struct wl_priv_signal *signal, void *data);
 
+void
+wl_priv_signal_final_emit(struct wl_priv_signal *signal, void *data);
+
 void
 wl_connection_close_fds_in(struct wl_connection *connection, int max);
 
diff --git a/src/wayland-server.c b/src/wayland-server.c
index eb1e500..8c3b800 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -682,7 +682,7 @@ destroy_resource(void *element, void *data, uint32_t flags)
/* Don't emit the new signal for deprecated resources, as that would
 * access memory outside the bounds of the deprecated struct */
if (!resource_is_deprecated(resource))
-   wl_priv_signal_emit(>destroy_signal, resource);
+   wl_priv_signal_final_emit(>destroy_signal, resource);
 
if (resource->destroy)
resource->destroy(resource);
@@ -841,7 +841,7 @@ wl_client_destroy(struct wl_client *client)
 {
uint32_t serial = 0;
 
-   wl_priv_signal_emit(>destroy_signal, client);
+   wl_priv_signal_final_emit(>destroy_signal, client);
 
wl_client_flush(client);
wl_map_for_each(>objects, destroy_resource, );
@@ -1089,7 +1089,7 @@ wl_display_destroy(struct wl_display *display)
struct wl_socket *s, *next;
struct wl_global *global, *gnext;
 
-   wl_priv_signal_emit(>destroy_signal, display);
+   wl_priv_signal_final_emit(>destroy_signal, display);
 
wl_list_for_each_safe(s, next, >socket_list, link) {
wl_socket_destroy(s);
@@ -2025,6 +2025,46 @@ wl_priv_signal_emit(struct wl_priv_signal *signal, void 
*data)
}
 }
 
+/** Emit the signal for the last time, calling all the installed listeners
+ *
+ * Iterate over all the listeners added to this \a signal and call
+ * their \a notify function pointer, passing on the given \a data.
+ * Removing or adding a listener from within wl_priv_signal_emit()
+ * is safe, as is freeing the structure containing the listener.
+ *
+ * A large body of external code assumes it's ok to free a destruction
+ * listener without removing that listener from the list.  Mixing code
+ * that acts like this and code that doesn't will result in list
+ * corruption.
+ *
+ * We resolve this by removing each item from the list and isolating it
+ * in another list.  We discard it completely after firing the notifier.
+ * This should allow interoperability between code that unlinks its
+ * destruction listeners and code that just frees structures they're in.
+ *
+ */
+void
+wl_priv_signal_final_emit(struct wl_priv_signal *signal, void *data)
+{
+   struct wl_listener *l;
+   struct wl_list *pos;
+
+   /* During a destructor notifier isolate every list item before
+* notifying.  This renders harmless the long standing misuse
+* of freeing listeners without removing them, but allows
+* callers that do choose to remove them to interoperate with
+* ones that don't. */
+   while (!wl_list_empty(>listener_list)) {
+   pos = signal->listener_list.next;
+   l = wl_container_of(pos, l, link);
+
+   wl_list_remove(pos);
+   wl_list_init(pos);
+
+   l->notify(l, data);
+   }
+}
+
 /** \endcond INTERNAL */
 
 /** \cond */ /* Deprecated functions below. */
-- 
2.14.3

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

[PATCH v2 0/3] Deal with destroy signal use after free issues

[Derek Foreman]

Now that the release is out, I'd like to dig back into this mess.
This is a round up of some patches that were on list shortly before
the release to deal with a problem where many existing libwayland
users don't delete their destroy signal listeners before freeing
them.

These leads to a bit of a mess (as Markus' test illustrates) if there
are multiple destroy listeners.

I've included:
My test patch to ensure the existing behaviour continues to work
(users like weston and enlightenment can free during destroy
listener)

The special case destroy emit path for wl_priv_signal - this is
an attempt to "fix" the problem, by making the destroy signal
emit operate without ever touching potentially free()d elements
again.

Markus' test that would fail without patch 2/3, as it catches the
free() without removal case we've all come to know any love.

Derek Foreman (2):
  tests: Test for use after free in resource destruction signals
   changes since first appearance: none

  server: Add special case destroy signal emitter
   changes since first appearance:  stop trying to maintain a list head

Markus Ongyerth (1):
  tests: Add free-without-remove test
   changes since first appearance: I moved it into an existing file

 src/wayland-private.h  |  3 +++
 src/wayland-server.c   | 46 +++---
 tests/resources-test.c | 39 +++
 3 files changed, 85 insertions(+), 3 deletions(-)

-- 
2.14.3

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

[PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

[Daniel Vetter]

This tries to align with the X.org communities's long-standing
tradition of trying to be an inclusive community and handing out
commit rights fairly freely.

We also tend to not revoke commit rights for people no longer
regularly active in a given project, as long as they're still part of
the larger community.

Finally make sure that commit rights, like anything happening on fd.o
infrastructre, is subject to the fd.o's Code of Conduct.

v2: Point at MAINTAINERS for contact info (Daniel S.)

v3:
- Make it clear that commit rights are voluntary and that committers
  need to acknowledge positively when they're nominated by someone
  else (Keith).
- Encourage committers to drop their commit rights when they're no
  longer active, and make it clear they'll get readded (Keith).
- Add a line that maintainers and committers should actively nominate
  new committers (me).

v4: Typo (Petri).

v5: Typo (Sean).

v6: Wording clarifications and spelling (Jani).

v7: Require an explicit commitment to the documented merge criteria
and rules, instead of just the implied one through the Code of Conduct
threat (Jani).

Acked-by: Alex Deucher 
Acked-by: Arkadiusz Hiler 
Acked-by: Daniel Stone 
Acked-by: Eric Anholt 
Acked-by: Gustavo Padovan 
Acked-by: Petri Latvala 
Cc: Alex Deucher 
Cc: Arkadiusz Hiler 
Cc: Ben Widawsky 
Cc: Daniel Stone 
Cc: Dave Airlie 
Cc: Eric Anholt 
Cc: Gustavo Padovan 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Keith Packard 
Cc: Kenneth Graunke 
Cc: Kristian H. Kristensen 
Cc: Maarten Lankhorst 
Cc: Petri Latvala 
Cc: Rodrigo Vivi 
Cc: Sean Paul 
Reviewed-by: Keith Packard 
Signed-off-by: Daniel Vetter 
---
If you wonder about the wide distribution list for an igt patch: I'd
like to start a discussions about x.org community norms around commit
rights at large, at least for all the shared repos. I plan to propose
the same text for drm-misc and libdrm too, and hopefully others like
mesa/xserver/wayland would follow.

fd.o admins also plan to discuss this (and a pile of other topics and
hosting and code of conduct) with all projects, ideally this here
would end up as the starting point for establishing some community
norms.
-Daniel
---
 CONTRIBUTING | 48 
 1 file changed, 48 insertions(+)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 0180641be3aa..8a118134275c 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -51,4 +51,52 @@ A short list of contribution guidelines:
 - Changes to the testcases are automatically tested. Take the results into
   account before merging.
 
+Commit rights
+-
+
+Commit rights will be granted to anyone who requests them and fulfills the
+below criteria:
+
+- Submitted a few (5-10 as a rule of thumb) non-trivial (not just simple
+  spelling fixes and whitespace adjustment) patches that have been merged
+  already.
+
+- Are actively participating on discussions about their work (on the mailing
+  list or IRC). This should not be interpreted as a requirement to review other
+  peoples patches but just make sure that patch submission isn't one-way
+  communication. Cross-review is still highly encouraged.
+
+- Will be regularly contributing further patches. This includes regular
+  contributors to other parts of the open source graphics stack who only
+  do the oddball rare patch within igt itself.
+
+- Agrees to use their commit rights in accordance with the documented merge
+  criteria, tools, and processes.
+
+Apply for an account (and any other account change requests) through
+
+https://www.freedesktop.org/wiki/AccountRequests/
+
+and please ping the maintainers if your request is stuck.
+
+Committers are encouraged to request their commit rights get removed when they
+no longer contribute to the project. Commit rights will be reinstated when they
+come back to the project.
+
+Maintainers and committers should encourage contributors to request commit
+rights, especially junior contributors tend to underestimate their skills.
+
+Code of Conduct
+---
+
+Please be aware the fd.o Code of Conduct also applies to igt:
+
+https://www.freedesktop.org/wiki/CodeOfConduct/
+
+See the MAINTAINERS file for contact details of the igt maintainers.
+
+Abuse of commit rights, like engaging in commit fights or willfully pushing
+patches that violate the documented merge criteria, will also be handled 
through
+the Code of Conduct enforcement process.
+
 Happy hacking!
-- 

Re: [PATCH i-g-t] [RFC] CONTRIBUTING: commit rights docs

[Harry Wentland]

On 2018-04-13 06:00 AM, Daniel Vetter wrote:
> This tries to align with the X.org communities's long-standing
> tradition of trying to be an inclusive community and handing out
> commit rights fairly freely.
> 
> We also tend to not revoke commit rights for people no longer
> regularly active in a given project, as long as they're still part of
> the larger community.
> 
> Finally make sure that commit rights, like anything happening on fd.o
> infrastructre, is subject to the fd.o's Code of Conduct.
> 
> v2: Point at MAINTAINERS for contact info (Daniel S.)
> 
> v3:
> - Make it clear that commit rights are voluntary and that committers
>   need to acknowledge positively when they're nominated by someone
>   else (Keith).
> - Encourage committers to drop their commit rights when they're no
>   longer active, and make it clear they'll get readded (Keith).
> - Add a line that maintainers and committers should actively nominate
>   new committers (me).
> 
> v4: Typo (Petri).
> 
> v5: Typo (Sean).
> 
> v6: Wording clarifications and spelling (Jani).
> 
> v7: Require an explicit commitment to the documented merge criteria
> and rules, instead of just the implied one through the Code of Conduct
> threat (Jani).
> 
> Acked-by: Alex Deucher 
> Acked-by: Arkadiusz Hiler 
> Acked-by: Daniel Stone 
> Acked-by: Eric Anholt 
> Acked-by: Gustavo Padovan 
> Acked-by: Petri Latvala 

Acked-by: Harry Wentland 

Harry

> Cc: Alex Deucher 
> Cc: Arkadiusz Hiler 
> Cc: Ben Widawsky 
> Cc: Daniel Stone 
> Cc: Dave Airlie 
> Cc: Eric Anholt 
> Cc: Gustavo Padovan 
> Cc: Jani Nikula 
> Cc: Joonas Lahtinen 
> Cc: Keith Packard 
> Cc: Kenneth Graunke 
> Cc: Kristian H. Kristensen 
> Cc: Maarten Lankhorst 
> Cc: Petri Latvala 
> Cc: Rodrigo Vivi 
> Cc: Sean Paul 
> Reviewed-by: Keith Packard 
> Signed-off-by: Daniel Vetter 
> ---
> If you wonder about the wide distribution list for an igt patch: I'd
> like to start a discussions about x.org community norms around commit
> rights at large, at least for all the shared repos. I plan to propose
> the same text for drm-misc and libdrm too, and hopefully others like
> mesa/xserver/wayland would follow.
> 
> fd.o admins also plan to discuss this (and a pile of other topics and
> hosting and code of conduct) with all projects, ideally this here
> would end up as the starting point for establishing some community
> norms.
> -Daniel
> ---
>  CONTRIBUTING | 48 
>  1 file changed, 48 insertions(+)
> 
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index 0180641be3aa..8a118134275c 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -51,4 +51,52 @@ A short list of contribution guidelines:
>  - Changes to the testcases are automatically tested. Take the results into
>account before merging.
>  
> +Commit rights
> +-
> +
> +Commit rights will be granted to anyone who requests them and fulfills the
> +below criteria:
> +
> +- Submitted a few (5-10 as a rule of thumb) non-trivial (not just simple
> +  spelling fixes and whitespace adjustment) patches that have been merged
> +  already.
> +
> +- Are actively participating on discussions about their work (on the mailing
> +  list or IRC). This should not be interpreted as a requirement to review 
> other
> +  peoples patches but just make sure that patch submission isn't one-way
> +  communication. Cross-review is still highly encouraged.
> +
> +- Will be regularly contributing further patches. This includes regular
> +  contributors to other parts of the open source graphics stack who only
> +  do the oddball rare patch within igt itself.
> +
> +- Agrees to use their commit rights in accordance with the documented merge
> +  criteria, tools, and processes.
> +
> +Apply for an account (and any other account change requests) through
> +
> +https://www.freedesktop.org/wiki/AccountRequests/
> +
> +and please ping the maintainers if your request is stuck.
> +
> +Committers are encouraged to request their commit rights get removed when 
> they
> +no longer contribute to the project. Commit rights will be reinstated when 
> they
> +come back to the project.
> +
> +Maintainers and committers should encourage contributors to request commit
> +rights, especially junior contributors tend to underestimate their skills.
> +
> +Code of Conduct
> +---
> +
> +Please be aware the fd.o Code of Conduct also applies to 

Re: [PATCHv4] Add name event to xdg-output

[Drew DeVault]

On 2018-04-16  2:57 PM, Jonas Ådahl wrote:
> I'd still like a bit more clarification about what to expect of this
> string. What I'm trying to avoid is one compositor sending "eDP-1" while
> another sends "Built-in Display". For example, the first is suitable for
> command line interfaces (e.g. movie-player --fullscreen-on HDMI-2), but
> the second is suitable for GUI's (e.g. a widget for selecting what
> monitor to play a movie on). If it can be either one, I don't see its
> usefulness in a generic client.

I'm explicitly not trying to avoid that. To me it's acceptable that one
compositor uses "eDP-1" and another uses "Built-in Display".

> I'm suspecting, given what you've written in other E-mails in this
> thread that you intend to use this for the "HDMI-1" style names, but at
> the same time I've seen the word "human readable" been used which more
> commonly refers to "Built-in Display" or "ASUS 24"", which might not
> even be unique (there might be two 24 inch ASUS monitors connected).

HDMI-1 is human readable to the sort of humans that use my compositor.
Each compositor has a different target audience and should cater their
naming conventions accordingly.

> I don't want to end up with a situation where we get wildly different
> results depending on what compositor is the one sending the value.

Why is this important?

> What I'm assuming is a major reason for keeping things relatively vague
> is to make sure that it's not specifically connector data, as that may
> not be available for centain types of compositors.

Yes, that is a major reason. This also isn't some vague theoretical
compositor either, my own compositor has situations where connector
names don't make sense.

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

Re: [PATCHv4] Add name event to xdg-output

[Jonas Ådahl]

On Sat, Apr 14, 2018 at 10:15:08AM -0400, Drew DeVault wrote:
> Signed-off-by: Drew DeVault 
> Reviewed-by: Simon Ser 
> ---
> This revision addresses Pekka's feedback, specifying that the output
> name will not change over the lifetime of the xdg_output. This also
> answers a question from an earlier email:
> 
> On 2018-04-11 11:02 AM, Pekka Paalanen wrote:
> > There is still the corner-case of: can removing wl_output global A
> > cause the name for wl_output global B to change, but I suppose that
> > falls to common sense to not do so strange things.
> 
> Since the name can no longer change, this is implicitly addressed.
> 
> Also bumps the version on zxdg_output_manager_v1.
> 
>  .../xdg-output/xdg-output-unstable-v1.xml | 22 +--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml 
> b/unstable/xdg-output/xdg-output-unstable-v1.xml
> index 0c0c481..c0f6b0e 100644
> --- a/unstable/xdg-output/xdg-output-unstable-v1.xml
> +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
> @@ -54,7 +54,7 @@
>  reset.
>
>  
> -  
> +  
>  
>A global factory interface for xdg_output objects.
>  
> @@ -77,7 +77,7 @@
>  
>
>  
> -  
> +  
>  
>An xdg_output describes part of the compositor geometry.
>  
> @@ -157,5 +157,23 @@
>
>  
>  
> +
> +  
> +Many compositors will assign names to their outputs, show them to the 
> user,
> +allow them to be configured by name, etc. The client may wish to know 
> this
> +name as well to offer the user similar behaviors.
> +
> +The naming convention is compositor defined. Each name is unique among 
> all
> +wl_output globals, but if a wl_output global is destroyed the same name 
> may
> +be reused later. The names will also remain consistent across sessions 
> with
> +the same hardware and software configuration.

I'd still like a bit more clarification about what to expect of this
string. What I'm trying to avoid is one compositor sending "eDP-1" while
another sends "Built-in Display". For example, the first is suitable for
command line interfaces (e.g. movie-player --fullscreen-on HDMI-2), but
the second is suitable for GUI's (e.g. a widget for selecting what
monitor to play a movie on). If it can be either one, I don't see its
usefulness in a generic client.

I'm suspecting, given what you've written in other E-mails in this
thread that you intend to use this for the "HDMI-1" style names, but at
the same time I've seen the word "human readable" been used which more
commonly refers to "Built-in Display" or "ASUS 24"", which might not
even be unique (there might be two 24 inch ASUS monitors connected).

I don't want to end up with a situation where we get wildly different
results depending on what compositor is the one sending the value.

What I'm assuming is a major reason for keeping things relatively vague
is to make sure that it's not specifically connector data, as that may
not be available for centain types of compositors.


Jonas

> +
> +The name event is sent after creating an xdg_output (see
> +xdg_output_manager.get_xdg_output). The name does not change over the
> +lifetime of the xdg_output, and this event will not be sent again.
> +  
> +  
> +
> +
>
>  
> -- 
> 2.17.0
> 
> ___
> 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: [PATCHv4] Add name event to xdg-output

[Drew DeVault]

On 2018-04-16 10:53 AM, Pekka Paalanen wrote:
> That's very clear, but is it precisely your intention? Would it make
> more sense to define that the name does not change during the lifetime
> of the wl_output global instead? That would guarantee that the name
> will stay the same for the same wl_output global even if one creates
> new xdg_output objects or even restarts the application or starts
> another application.
> 
> Or do you think that's already implied by the previous paragraph?
> 
> Would there be any harm in using the lifetime of the wl_output global
> in the wording? I think it would be more clear than the current wording.

I have updated it to the following:

+The name event is sent after creating an xdg_output (see
+xdg_output_manager.get_xdg_output). This event is only sent once per
+xdg_output, and the name does not change over the lifetime of the
+wl_output.

Will wait a while for a little more feedback to come in and then will
send off a new patch.

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

Re: [PATCHv3] Add name event to xdg-output

[Drew DeVault]

On 2018-04-16 10:36 AM, Pekka Paalanen wrote:
> that's seems contradictory to what you replied here:

You're right, it does.

> > You could do this but it would be exceedingly silly of you considering
> > that the other xdg_output requests furnish the client with layout
> > information anyway.
> 
> So, I really could not do that. Very well.

Aye, that's my conclusion as well.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCHv3] Add name event to xdg-output

[Pekka Paalanen]

On Fri, 13 Apr 2018 16:16:34 +0200
Jonas Ådahl  wrote:

> On Fri, Apr 13, 2018 at 10:05:39AM -0400, Drew DeVault wrote:
> > On 2018-04-13  4:02 PM, Jonas Ådahl wrote:  
> > > 

> > > > - Xwayland can name the X11 outputs based on their genuine names rather
> > > >   than assigning e.g. XWAYLAND0  
> > > 
> > > If the protocol is loosly specified as in it's not known what format
> > > you'll get, I don't think Xwayland can make any use of this.  
> > 
> > Can you elaborate on what expectations Xwayland would have? I am open to
> > restricting the format a bit, e.g. to ASCII characters or so.  
> 
> If we'd add quotation marks, spaces etc I'm sure many things would break
> here and there. If we start adding such restrictions, it'd hardly be
> suitable as a "human readable" kind of thing.

I believe what Jonas says as well. It could lead to another variant of
https://bugs.freedesktop.org/show_bug.cgi?id=94589
or such. Restricting to a suitable set would pretty much remove the
human-friendlyness from the name.


Thanks,
pq


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

Re: [PATCHv4] Add name event to xdg-output

[Pekka Paalanen]

Hi,

one elementary detail I have missed is that you have no commit message.
For the record, you should give a justification for why xdg_output
needs a name event added.


On Sat, 14 Apr 2018 10:15:08 -0400
Drew DeVault  wrote:

> Signed-off-by: Drew DeVault 
> Reviewed-by: Simon Ser 
> ---
> This revision addresses Pekka's feedback, specifying that the output
> name will not change over the lifetime of the xdg_output. This also
> answers a question from an earlier email:
> 
> On 2018-04-11 11:02 AM, Pekka Paalanen wrote:
> > There is still the corner-case of: can removing wl_output global A
> > cause the name for wl_output global B to change, but I suppose that
> > falls to common sense to not do so strange things.  
> 
> Since the name can no longer change, this is implicitly addressed.
> 
> Also bumps the version on zxdg_output_manager_v1.
> 
>  .../xdg-output/xdg-output-unstable-v1.xml | 22 +--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml 
> b/unstable/xdg-output/xdg-output-unstable-v1.xml
> index 0c0c481..c0f6b0e 100644
> --- a/unstable/xdg-output/xdg-output-unstable-v1.xml
> +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
> @@ -54,7 +54,7 @@
>  reset.
>
>  
> -  
> +  
>  
>A global factory interface for xdg_output objects.
>  
> @@ -77,7 +77,7 @@
>  
>
>  
> -  
> +  
>  
>An xdg_output describes part of the compositor geometry.
>  
> @@ -157,5 +157,23 @@
>
>  
>  
> +

All the versioning stuff seem good now.

> +  
> +Many compositors will assign names to their outputs, show them to the 
> user,
> +allow them to be configured by name, etc. The client may wish to know 
> this
> +name as well to offer the user similar behaviors.
> +
> +The naming convention is compositor defined. Each name is unique among 
> all
> +wl_output globals, but if a wl_output global is destroyed the same name 
> may
> +be reused later. The names will also remain consistent across sessions 
> with
> +the same hardware and software configuration.
> +
> +The name event is sent after creating an xdg_output (see
> +xdg_output_manager.get_xdg_output). The name does not change over the
> +lifetime of the xdg_output, and this event will not be sent again.

That's very clear, but is it precisely your intention? Would it make
more sense to define that the name does not change during the lifetime
of the wl_output global instead? That would guarantee that the name
will stay the same for the same wl_output global even if one creates
new xdg_output objects or even restarts the application or starts
another application.

Or do you think that's already implied by the previous paragraph?

Would there be any harm in using the lifetime of the wl_output global
in the wording? I think it would be more clear than the current wording.


Thanks,
pq

> +  
> +  
> +
> +
>
>  


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

Re: [PATCHv3] Add name event to xdg-output

[Pekka Paalanen]

On Sat, 14 Apr 2018 10:08:35 -0400
Drew DeVault  wrote:

> On 2018-04-13  4:33 PM, Pekka Paalanen wrote:
> > The other events have more precise wording here, allowing the event to
> > be sent again if the information changes.  
> 
> This is deliberate - I don't think the name should change. I'll write it
> up explicitly.

Hi,

that's seems contradictory to what you replied here:

On Wed, 11 Apr 2018 08:38:44 -0400
Drew DeVault  wrote:

> > I do wonder, if I used a naming scheme like this:
> > 
> > on top: Intel GMA: DVI-D-1: Viewsonic VP171B (S/N: 8764358365)
> > on bottom: GeForce 8800: HDMI-2: HP ZDisplay K99 (S/N: 98728462)
> > 
> > and then the user reconfigures the output layout, can the compositor
> > send updated names to all clients that already have an xdg-output object?  
> 
> You could do this but it would be exceedingly silly of you considering
> that the other xdg_output requests furnish the client with layout
> information anyway.

So, I really could not do that. Very well.


Thanks,
pq


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