[PATCH libinput] test: move all the _setup() functions into a special section

[Peter Hutterer]

This way we can loop through them instead of having to add them manually.

Signed-off-by: Peter Hutterer 
---
 test/litest.c| 25 -
 test/litest.h| 14 ++
 test/test-device.c   |  3 +--
 test/test-gestures.c |  3 +--
 test/test-keyboard.c |  3 +--
 test/test-log.c  |  3 +--
 test/test-misc.c |  3 +--
 test/test-pad.c  |  3 +--
 test/test-path.c |  3 +--
 test/test-pointer.c  |  3 +--
 test/test-switch.c   |  4 ++--
 test/test-tablet.c   |  3 +--
 test/test-touch.c|  3 +--
 test/test-touchpad-buttons.c |  3 +--
 test/test-touchpad-tap.c |  3 +--
 test/test-touchpad.c |  3 +--
 test/test-trackball.c|  3 +--
 test/test-trackpoint.c   |  3 +--
 test/test-udev.c |  3 +--
 19 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/test/litest.c b/test/litest.c
index 7f4bd77a..f3d4cfee 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -3700,11 +3700,14 @@ litest_init_test_devices(void)
}
 }
 
+extern const struct test_collection __start_test_collection_section, 
__stop_test_collection_section;
+
 int
 main(int argc, char **argv)
 {
const struct rlimit corelimit = { 0, 0 };
enum litest_mode mode;
+   const struct test_collection *c;
 
if (getuid() != 0) {
fprintf(stderr,
@@ -3735,23 +3738,11 @@ main(int argc, char **argv)
if (mode == LITEST_MODE_ERROR)
return EXIT_FAILURE;
 
-   litest_setup_tests_udev();
-   litest_setup_tests_path();
-   litest_setup_tests_pointer();
-   litest_setup_tests_touch();
-   litest_setup_tests_log();
-   litest_setup_tests_tablet();
-   litest_setup_tests_pad();
-   litest_setup_tests_touchpad();
-   litest_setup_tests_touchpad_tap();
-   litest_setup_tests_touchpad_buttons();
-   litest_setup_tests_trackpoint();
-   litest_setup_tests_trackball();
-   litest_setup_tests_misc();
-   litest_setup_tests_keyboard();
-   litest_setup_tests_device();
-   litest_setup_tests_gestures();
-   litest_setup_tests_lid();
+   for (c = &__start_test_collection_section;
+c < &__stop_test_collection_section;
+c++) {
+   c->setup();
+   }
 
if (mode == LITEST_MODE_LIST) {
litest_list_tests(_tests);
diff --git a/test/litest.h b/test/litest.h
index df725f87..05ea2d7f 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -60,6 +60,20 @@ struct test_device {
__VA_ARGS__ \
};
 
+struct test_collection {
+   const char *name;
+   void (*setup)(void);
+} __attribute__((aligned(16)));
+
+#define TEST_COLLECTION(name) \
+   static void (name##_setup)(void); \
+   static const struct test_collection _test_collection \
+   __attribute__ ((used)) \
+   __attribute__ ((section ("test_collection_section"))) = { \
+   #name, name##_setup \
+   }; \
+   static void (name##_setup)(void)
+
 extern void litest_setup_tests_udev(void);
 extern void litest_setup_tests_path(void);
 extern void litest_setup_tests_pointer(void);
diff --git a/test/test-device.c b/test/test-device.c
index 769ecaf4..3a26c7d9 100644
--- a/test/test-device.c
+++ b/test/test-device.c
@@ -1577,8 +1577,7 @@ START_TEST(device_seat_phys_name)
 }
 END_TEST
 
-void
-litest_setup_tests_device(void)
+TEST_COLLECTION(device)
 {
struct range abs_range = { 0, ABS_MISC };
struct range abs_mt_range = { ABS_MT_SLOT + 1, ABS_CNT };
diff --git a/test/test-gestures.c b/test/test-gestures.c
index 1d0a13c5..db0e6731 100644
--- a/test/test-gestures.c
+++ b/test/test-gestures.c
@@ -1057,8 +1057,7 @@ START_TEST(gestures_3fg_buttonarea_scroll_btntool)
 }
 END_TEST
 
-void
-litest_setup_tests_gestures(void)
+TEST_COLLECTION(gestures)
 {
/* N, NE, ... */
struct range cardinals = { 0, 8 };
diff --git a/test/test-keyboard.c b/test/test-keyboard.c
index db836381..4b6b5315 100644
--- a/test/test-keyboard.c
+++ b/test/test-keyboard.c
@@ -476,8 +476,7 @@ START_TEST(keyboard_no_scroll)
 }
 END_TEST
 
-void
-litest_setup_tests_keyboard(void)
+TEST_COLLECTION(keyboard)
 {
litest_add_no_device("keyboard:seat key count", 
keyboard_seat_key_count);
litest_add_no_device("keyboard:key counting", 
keyboard_ignore_no_pressed_release);
diff --git a/test/test-log.c b/test/test-log.c
index 02ff0185..1dac01b6 100644
--- a/test/test-log.c
+++ b/test/test-log.c
@@ -200,8 +200,7 @@ START_TEST(log_axisrange_warning)
 }
 END_TEST
 
-void
-litest_setup_tests_log(void)
+TEST_COLLECTION(log)
 {
struct range axes = { ABS_X, ABS_Y + 1};
 
diff --git a/test/test-misc.c b/test/test-misc.c
index 94cbfcc1..c62cd03e 100644
--- a/test/test-misc.c
+++ b/test/test-misc.c
@@ -1492,8 +1492,7 @@ START_TEST(timer_flush)
 }
 END_TEST
 
-void

Re: T480s Trackpoint drifts

[Peter Hutterer]

On Wed, Mar 21, 2018 at 11:14:28AM +0800, Kai Hendry wrote:
> On Tue, 20 Mar 2018, at 12:05 PM, Peter Hutterer wrote:
> > what exactly is drift in this context? it moves on its own to the edge of
> > the screen?
> 
> It generally just moves diagonally south west for about a cm. Sometimes
> all the way to the bottom left. Initially I googled around and people said
> it was "re-calibrating"..

yeah, that's a built-in trackpoint feature that we don't have control over.
 
> > As for the drifting, is this something on your device only or a general
> > issue affecting this class of devices? Try playing around with the knobs 
> > in 
> > /sys/class/input/event18/device/device (replace event18 with your event 
> > node number)
> 
> Which knobs? Is there a guide?

judging by the output in the .txt file, you didn't run the cd command
correctly and ended up in some other device's directory. I'm honestly not
sure if there's a guide or even documentation but you can find bits and
pieces with google. drift_time is probably the key here, maybe resetafter or
resync_time. At some point I had a pdf somewhere that described some of
these but, ...

> > run libinput measure trackpoint-range and go from there, it's likely that
> > you need a custom range for that device. And the speed setting can be
> > changed, I'm having a hard time believing that at the lowest speed setting
> > it's still too fast given that it's almost tar-like at that speed on every
> > device I've tested.
> 
> https://www.youtube.com/watch?v=gDzWBhcOcCY
> https://s.natalian.org/2018-03-21/libinput.txt

> 
> > did you try changing the touchpad speed settings?
> 
> How do I do that? xinput?

use the gnome control center to change the pointer speed. or yeah, the
libinput Accel Speed property, set it to -1 and you'll have tar galore.

https://wayland.freedesktop.org/libinput/doc/latest/trackpoints.html matters
too

Cheers,
   Peter


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

[PATCH libinput] touchpad: only keep low-pressure fingers alive for 2+-slot touchpads

[Peter Hutterer]

Regression introduced by 3979b9e16a5ed141506d95f80ddfd7b94651dcfa, bug 105258.
With that commit, we only ended real touches when we had less than nslots fake
fingers down. i.e. tripletap on a 2 slot touchpad would not end the
first/second touch even if the pressure goes below the threshold. e.g. Lenovo
x270 needs this, see https://bugs.freedesktop.org/attachment.cgi?id=137672, it
dips below the pressure threshold for the first slot and ends the second slot
in the same frame as the third finger is detected. Fun times.

Anyway, this breaks semi-mt touchpads, another fine category of devices,
because some of those can detect hovering fingers at low pressure, see bug
105535. Because semi-mt devices are generally garbage, we treat them as
single-touch devices instead. So whenever two fingers are down, we treat both
as above the pressure threshold, even when they're physicall hovering.

Fix this by making the x270 fix conditional on at least 2 slots.

https://bugs.freedesktop.org/show_bug.cgi?id=105535

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c  |  8 ++--
 test/test-touchpad-tap.c |  5 -
 test/test-touchpad.c | 40 
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 1da85c17..99cbcc56 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1178,8 +1178,12 @@ tp_unhover_pressure(struct tp_dispatch *tp, uint64_t 
time)
tp_begin_touch(tp, t, time);
}
/* don't unhover for pressure if we have too many
-* fake fingers down, see comment below */
-   } else if (nfake_touches <= tp->num_slots) {
+* fake fingers down, see comment below. Except
+* for single-finger touches where the real touch
+* decides for the rest.
+*/
+   } else if (nfake_touches <= tp->num_slots ||
+  tp->num_slots == 1) {
if (t->pressure < tp->pressure.low) {
evdev_log_debug(tp->device,
"pressure: end touch 
%d\n",
diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c
index 2e1c556d..68b4dc06 100644
--- a/test/test-touchpad-tap.c
+++ b/test/test-touchpad-tap.c
@@ -1620,7 +1620,10 @@ START_TEST(touchpad_3fg_tap_pressure_btntool)
litest_drain_events(li);
 
/* drop below the pressure threshold in the same frame as starting a
-* third touch  */
+* third touch, see
+*   E: 8713.954784 0001 014e 0001 # EV_KEY / BTN_TOOL_TRIPLETAP   1
+* in https://bugs.freedesktop.org/attachment.cgi?id=137672
+*/
litest_event(dev, EV_ABS, ABS_MT_PRESSURE, 3);
litest_event(dev, EV_ABS, ABS_PRESSURE, 3);
litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
diff --git a/test/test-touchpad.c b/test/test-touchpad.c
index e2edc32e..4759bfb6 100644
--- a/test/test-touchpad.c
+++ b/test/test-touchpad.c
@@ -5503,6 +5503,45 @@ START_TEST(touchpad_pressure_btntool)
 }
 END_TEST
 
+START_TEST(touchpad_pressure_semi_mt_2fg_goes_light)
+{
+   struct litest_device *dev = litest_current_device();
+   struct libinput *li = dev->libinput;
+   struct axis_replacement axes[] = {
+   { ABS_PRESSURE, 2 },
+   { -1, 0 }
+   };
+
+   litest_enable_2fg_scroll(dev);
+   litest_drain_events(li);
+
+   litest_touch_down(dev, 0, 40, 50);
+   litest_touch_down(dev, 1, 60, 50);
+   litest_touch_move_two_touches(dev, 40, 50, 60, 50, 0, -20, 10, 0);
+
+   /* This should trigger a scroll end event */
+   litest_push_event_frame(dev);
+   litest_touch_move_extended(dev, 0, 40, 31, axes);
+   litest_touch_move_extended(dev, 1, 60, 31, axes);
+   litest_pop_event_frame(dev);
+   libinput_dispatch(li);
+
+   litest_assert_scroll(li, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, 0);
+
+   litest_push_event_frame(dev);
+   litest_touch_move_extended(dev, 0, 40, 35, axes);
+   litest_touch_move_extended(dev, 1, 60, 35, axes);
+   litest_pop_event_frame(dev);
+
+   litest_push_event_frame(dev);
+   litest_touch_move_extended(dev, 0, 40, 40, axes);
+   litest_touch_move_extended(dev, 1, 60, 40, axes);
+   litest_pop_event_frame(dev);
+   libinput_dispatch(li);
+   litest_assert_empty_queue(li);
+}
+END_TEST
+
 static inline bool
 touchpad_has_touch_size(struct litest_device *dev)
 {
@@ -5887,6 +5926,7 @@ litest_setup_tests_touchpad(void)
litest_add("touchpad:pressure", touchpad_pressure_tap_2fg, 
LITEST_TOUCHPAD, LITEST_ANY);
litest_add("touchpad:pressure", touchpad_pressure_tap_2fg_1fg_light, 

Re: T480s Trackpoint drifts

[Kai Hendry]

On Tue, 20 Mar 2018, at 12:05 PM, Peter Hutterer wrote:
> what exactly is drift in this context? it moves on its own to the edge of
> the screen?

It generally just moves diagonally south west for about a cm. Sometimes all the 
way to the bottom left. Initially I googled around and people said it was 
"re-calibrating"..

> As for the drifting, is this something on your device only or a general
> issue affecting this class of devices? Try playing around with the knobs 
> in 
> /sys/class/input/event18/device/device (replace event18 with your event 
> node number)

Which knobs? Is there a guide?

> run libinput measure trackpoint-range and go from there, it's likely that
> you need a custom range for that device. And the speed setting can be
> changed, I'm having a hard time believing that at the lowest speed setting
> it's still too fast given that it's almost tar-like at that speed on every
> device I've tested.

https://www.youtube.com/watch?v=gDzWBhcOcCY
https://s.natalian.org/2018-03-21/libinput.txt

> did you try changing the touchpad speed settings?

How do I do that? xinput?

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

build tools questions

[Joshua Marinacci]

I'm interested in getting the build up to snuff on Raspberry Pi 2/3. Where 
should I file bugs? The github repo doesn't have the 'issues' component turned 
on so there is no way to file an issue.

https://github.com/wayland-project/wayland-build-tools 



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

Re: [PATCH] xdg-shell: add enums for tiled window state to toplevel configure

[Drew DeVault]

On 2018-03-20  9:52 AM, Mike Blumenkrantz wrote:
> this adds implementation from a related discussion long ago in which
> it was decided that it would be useful for clients to know if/where their
> windows were tiled so that various behaviors and visuals could be modified
> to improve UX
> 
> a window which is e.g., tiled on the right side of the screen would set the
> right|top|bottom tiled states in configure

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

[PATCH v2] xwm: Fix memory leak

[Scott Moreau]

A memory leak introduced by 6b58ea8c led to me finding a bigger
leak, which is xwm was calling frame_create() without calling
frame_destroy(). This meant that the associated icon_surface
was not being destroyed, leaving the destroy handler for it
broken. Here we fix this by calling frame_destroy() when the
window is destroyed and free the reply in the icon_surface
destroy handler.
---
 xwayland/window-manager.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c307e19..9e61a67 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1353,6 +1353,14 @@ weston_wm_window_schedule_repaint(struct 
weston_wm_window *window)
 }
 
 static void
+handle_icon_surface_destroy(void *data)
+{
+   xcb_get_property_reply_t *reply = (xcb_get_property_reply_t *)data;
+
+   free(reply);
+}
+
+static void
 weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
 {
xcb_get_property_reply_t *reply;
@@ -1371,16 +1379,20 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
weston_wm_window *window)
length = xcb_get_property_value_length(reply);
 
/* This is in 32-bit words, not in bytes. */
-   if (length < 2)
+   if (length < 2) {
+   free(reply);
return;
+   }
 
data = xcb_get_property_value(reply);
width = *data++;
height = *data++;
 
/* Some checks against malformed input. */
-   if (width == 0 || height == 0 || length < 2 + width * height)
+   if (width == 0 || height == 0 || length < 2 + width * height) {
+   free(reply);
return;
+   }
 
new_surface =
cairo_image_surface_create_for_data((unsigned char *)data,
@@ -1390,9 +1402,13 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
weston_wm_window *window)
/* Bail out in case anything wrong happened during surface creation. */
if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
cairo_surface_destroy(new_surface);
+   free(reply);
return;
}
 
+   cairo_surface_set_user_data(new_surface, NULL, reply,
+   
_icon_surface_destroy);
+
if (window->frame)
frame_set_icon(window->frame, new_surface);
else /* We don’t have a frame yet */
@@ -1502,6 +1518,9 @@ weston_wm_window_destroy(struct weston_wm_window *window)
window->frame_id = XCB_WINDOW_NONE;
}
 
+   if (window->frame)
+   frame_destroy(window->frame);
+
if (window->surface_id)
wl_list_remove(>link);
 
-- 
2.7.4

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 15:09, Derek Foreman  wrote:
> On 2018-03-20 10:02 AM, Emil Velikov wrote:
>> On 20 March 2018 at 14:50, Derek Foreman  wrote:
>>> On 2018-03-20 07:11 AM, Daniel Stone wrote:
 On 20 March 2018 at 11:55, Emil Velikov  wrote:
> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>> Sure. As on IRC though, we definitely need to add at least _ftext for
>> MIPS anyway:
>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>
>> And probably some more for ARM toolchains using other linkers:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>
>> Doing a quick check across all the architectures on Debian shows that
>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>> need to respin for those as well, but I think at this point the
>> inconvenience of maintaining a list of every linker's implementation
>> detail on every platform, outweighs the risk of an exported
>> underscore-prefixed symbol slipping through review.
>
> I think you're preemptively worried about this. Plus there's no need
> to add every symbol a search could find.
>
> Even then, ask yourself the question:
> Which is better - updating an ugly looking list once in a blue moon or
> having apps use internal API?
>
> As said above: _if_ it turns out to be labour intensive - fine remove it.
> We've already spend more time researching than what the [expected]
> maintenance for 1 year would be ;-)

 It all depends on how you view scope and probability.

 I don't agree my concern is preemptive: we've just shipped a beta
 where 'make check' fails on a few architectures, and the proposed fix
 (adding several symbols to the platform list) will still fail on
 architectures Debian ships on, until it also adds _fbss, _fdata and
 _ftext. I think my suggestion is reasonable: it fixes the very real
 problem, in a way which avoids expanding the scope of our ABI checker
 to include the implementation details of every linker/architecture
 pair that people use Wayland on. The cost of this is the probability
 that someone manages to add a new underscored symbol marked with
 WL_EXPORT into a library which is currently 105 LoC, and have no-one
 notice that in review. I think that's a reasonable tradeoff.
>>>
>>> I'm inclined to agree with Daniel here.
>>>
>>> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
>>> review seems reasonably unlikely.
>>>
>>> And after reading this thread I'm still not entirely sure we have a
>>> complete list of what should be on that whitelist, and there are only a
>>> limited number of systems I can test build on right now.
>>>
>> Sharing a few ideas that are less obvious. Having the partial list
>> allows you to:
>>  - see active developers/users - be that by merging their patches or
>> skimming through the report
>
> I don't imagine we'll be adding new API to that library frequently
> enough to mine that data productively.
>
I am talking about the C runtime symbols.

>>  - have contact point for people with unusual platforms/setups
>
> By defaulting to a broken build state for unusual platforms, do you
> really think we'll attract many contact points?
>
Quite a serious typo here - s/build/check/. From experience - yes, you will.

>>  - is fairly trivial newbie task ;-)
>
> I don't think we should be going out of our way to generate problems for
> newbies to solve.  There are plenty of worthwhile tasks for new
> participants already.
>
Might want to make the list more obvious. I've been [sort of] around
and haven't seen any.

> I've confirmed that we once again pass make check on arm (something I
> really should've done prior to the beta release.  sigh.) and have pushed
> this patch.
>
You can never have 'enough' tests before a release ;-)

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Derek Foreman]

On 2018-03-20 10:02 AM, Emil Velikov wrote:
> On 20 March 2018 at 14:50, Derek Foreman  wrote:
>> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>>> On 20 March 2018 at 11:55, Emil Velikov  wrote:
 On 20 March 2018 at 11:46, Daniel Stone  wrote:
> Sure. As on IRC though, we definitely need to add at least _ftext for
> MIPS anyway:
> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>
> And probably some more for ARM toolchains using other linkers:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>
> Doing a quick check across all the architectures on Debian shows that
> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
> need to respin for those as well, but I think at this point the
> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.

 I think you're preemptively worried about this. Plus there's no need
 to add every symbol a search could find.

 Even then, ask yourself the question:
 Which is better - updating an ugly looking list once in a blue moon or
 having apps use internal API?

 As said above: _if_ it turns out to be labour intensive - fine remove it.
 We've already spend more time researching than what the [expected]
 maintenance for 1 year would be ;-)
>>>
>>> It all depends on how you view scope and probability.
>>>
>>> I don't agree my concern is preemptive: we've just shipped a beta
>>> where 'make check' fails on a few architectures, and the proposed fix
>>> (adding several symbols to the platform list) will still fail on
>>> architectures Debian ships on, until it also adds _fbss, _fdata and
>>> _ftext. I think my suggestion is reasonable: it fixes the very real
>>> problem, in a way which avoids expanding the scope of our ABI checker
>>> to include the implementation details of every linker/architecture
>>> pair that people use Wayland on. The cost of this is the probability
>>> that someone manages to add a new underscored symbol marked with
>>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>>> notice that in review. I think that's a reasonable tradeoff.
>>
>> I'm inclined to agree with Daniel here.
>>
>> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
>> review seems reasonably unlikely.
>>
>> And after reading this thread I'm still not entirely sure we have a
>> complete list of what should be on that whitelist, and there are only a
>> limited number of systems I can test build on right now.
>>
> Sharing a few ideas that are less obvious. Having the partial list
> allows you to:
>  - see active developers/users - be that by merging their patches or
> skimming through the report

I don't imagine we'll be adding new API to that library frequently
enough to mine that data productively.

>  - have contact point for people with unusual platforms/setups

By defaulting to a broken build state for unusual platforms, do you
really think we'll attract many contact points?

>  - is fairly trivial newbie task ;-)

I don't think we should be going out of our way to generate problems for
newbies to solve.  There are plenty of worthwhile tasks for new
participants already.

I've confirmed that we once again pass make check on arm (something I
really should've done prior to the beta release.  sigh.) and have pushed
this patch.

Thanks,
Derek

> 
> -Emil
> 

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

RE: [PATCH weston 1/7] compositor-drm: remove superfluos get_disable_state call

[Ucan, Emre (ADITG/ESB)]

Hi Daniel,

> -Original Message-
> From: Daniel Stone [mailto:dan...@fooishbar.org]
> Sent: Dienstag, 20. März 2018 15:48
> To: Ucan, Emre (ADITG/ESB)
> Cc: wayland-devel@lists.freedesktop.org
> Subject: Re: [PATCH weston 1/7] compositor-drm: remove superfluos
> get_disable_state call
> 
> Hi Emre,
> 
> On 20 March 2018 at 14:28, Emre Ucan  wrote:
> > drm_output_get_disable_state function returns
> > a duplicated output_state object.
> >
> > Here we are creating the object, but we are
> > never using it. Therefore, it is safe to remove.
> >
> > (Found by clang source code analyzer)
> 
> It's subtle, but we do need this here.
> 
> > @@ -3860,7 +3860,6 @@ drm_set_dpms(struct weston_output
> *output_base, enum dpms_enum level)
> 
> It's out of diff context, the if branch we're in guarantees that we
> have pending_state, i.e. after repaint_begin() but before
> repaint_flush().
> 
> > state = drm_pending_state_get_output(pending_state, output);
> > if (state)
> > drm_output_state_free(state);
> 
> Here we search pending_state for any existing output state for this
> output. If there are any, we free them and remove them from the
> pending state. So after this line, we are guaranteed not to be
> touching the output in this pending state.
> 
> > -   state = drm_output_get_disable_state(pending_state, output);
> 
> This line adds a disable state to the pending_state. We do not do
> anything with it: we just keep it there in the pending_state, to be
> applied when repaint_flush() is called. If we remove this, then we
> won't disable the output.

Oh, I missed that. 
> 
> Replacing 'state = ' with '(void) ' as the prefix of this line might
> make it clearer; does that also quiet Coverity?

Yes, adding '(void)' as the prefix of this line fixes the issue. I can send a 
second version of my patches after initial review, or you can fix this anyway, 
and we can ignore my first patch.
 
> 
> Cheers,
> Daniel

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 14:50, Derek Foreman  wrote:
> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
 Sure. As on IRC though, we definitely need to add at least _ftext for
 MIPS anyway:
 https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74

 And probably some more for ARM toolchains using other linkers:
 http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html

 Doing a quick check across all the architectures on Debian shows that
 your updated list is also missing _fbss, _fdata, and _ftext. So we'd
 need to respin for those as well, but I think at this point the
 inconvenience of maintaining a list of every linker's implementation
 detail on every platform, outweighs the risk of an exported
 underscore-prefixed symbol slipping through review.
>>>
>>> I think you're preemptively worried about this. Plus there's no need
>>> to add every symbol a search could find.
>>>
>>> Even then, ask yourself the question:
>>> Which is better - updating an ugly looking list once in a blue moon or
>>> having apps use internal API?
>>>
>>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>>> We've already spend more time researching than what the [expected]
>>> maintenance for 1 year would be ;-)
>>
>> It all depends on how you view scope and probability.
>>
>> I don't agree my concern is preemptive: we've just shipped a beta
>> where 'make check' fails on a few architectures, and the proposed fix
>> (adding several symbols to the platform list) will still fail on
>> architectures Debian ships on, until it also adds _fbss, _fdata and
>> _ftext. I think my suggestion is reasonable: it fixes the very real
>> problem, in a way which avoids expanding the scope of our ABI checker
>> to include the implementation details of every linker/architecture
>> pair that people use Wayland on. The cost of this is the probability
>> that someone manages to add a new underscored symbol marked with
>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>> notice that in review. I think that's a reasonable tradeoff.
>
> I'm inclined to agree with Daniel here.
>
> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
> review seems reasonably unlikely.
>
> And after reading this thread I'm still not entirely sure we have a
> complete list of what should be on that whitelist, and there are only a
> limited number of systems I can test build on right now.
>
Sharing a few ideas that are less obvious. Having the partial list
allows you to:
 - see active developers/users - be that by merging their patches or
skimming through the report
 - have contact point for people with unusual platforms/setups
 - is fairly trivial newbie task ;-)

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

Re: [PATCH] xdg-shell: add enums for tiled window state to toplevel configure

[Mike Blumenkrantz]

On Tue, Mar 20, 2018 at 10:15 AM Simon McVittie  wrote:

> On Tue, 20 Mar 2018 at 09:52:02 -0400, Mike Blumenkrantz wrote:
> > this adds implementation from a related discussion long ago in which
> > it was decided that it would be useful for clients to know if/where their
> > windows were tiled so that various behaviors and visuals could be
> modified
> > to improve UX
> >
> > a window which is e.g., tiled on the right side of the screen would set
> the
> > right|top|bottom tiled states in configure
>
> Are these for the same purpose as the tiled states in the (currently
> private)
> protocol between GTK+ and Mutter/GNOME Shell?
>
> This has separate per-edge flags for:
>
> - Tiling: each edge is tiled (aligned to some other object) or not, so
>   that windows and client-side decorations can make UI choices like
>   "draw shadows on each edge that is not tiled" or "draw square corners
>   at each corner involving a tiled edge, and rounded corners where
>   neither edge is tiled"
>
> - Resizability: each edge is resizable or not, so that client-side
>   decorations can show or not show resize handles as appropriate (in
>   GNOME Shell you can tile two windows and then drag their shared
>   border to adjust the split, and I suspect that the Wayland equivalents
>   of X11 tiling window managers would want this too)
>
> https://bugzilla.gnome.org/show_bug.cgi?id=751857
>
> https://gitlab.gnome.org/GNOME/mutter/blob/master/src/wayland/protocol/gtk-shell.xml
>
> Regards,
> smcv
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


They could be used for those purposes, yes.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: T480s Trackpoint drifts

[Emil Velikov]

On 20 March 2018 at 04:05, Peter Hutterer  wrote:
> On Mon, Mar 19, 2018 at 04:28:20PM +0800, Kai Hendry wrote:
>> Hi there,
>>
>> I'm an Archlinux user who has a new T480s Thinkpad and unfortunately
>> the Trackpoint can (not always or reproducibly) "drift". I hope that
>> makes sense!
>
> what exactly is drift in this context? it moves on its own to the edge of
> the screen?
>
FWIW I've experienced something that sounds like this.

Sharing some experience - it's _not_ mean to hijack the thread.

While the cursor is drifting in direction X, one can hardly move it
any another direction.
Unsurprisingly, the drift continues even after the cursor reaches the
end of the screen.

The only way to get it unstuck is by briefly moving the trackpoint in
the exact same direction where the drift is.

I've experienced that on x220 and X1C3 machines. Both running
Archlinux, in case it matters.

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Derek Foreman]

On 2018-03-20 07:11 AM, Daniel Stone wrote:
> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>> MIPS anyway:
>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>
>>> And probably some more for ARM toolchains using other linkers:
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>
>>> Doing a quick check across all the architectures on Debian shows that
>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>> need to respin for those as well, but I think at this point the
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> I think you're preemptively worried about this. Plus there's no need
>> to add every symbol a search could find.
>>
>> Even then, ask yourself the question:
>> Which is better - updating an ugly looking list once in a blue moon or
>> having apps use internal API?
>>
>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>> We've already spend more time researching than what the [expected]
>> maintenance for 1 year would be ;-)
> 
> It all depends on how you view scope and probability.
> 
> I don't agree my concern is preemptive: we've just shipped a beta
> where 'make check' fails on a few architectures, and the proposed fix
> (adding several symbols to the platform list) will still fail on
> architectures Debian ships on, until it also adds _fbss, _fdata and
> _ftext. I think my suggestion is reasonable: it fixes the very real
> problem, in a way which avoids expanding the scope of our ABI checker
> to include the implementation details of every linker/architecture
> pair that people use Wayland on. The cost of this is the probability
> that someone manages to add a new underscored symbol marked with
> WL_EXPORT into a library which is currently 105 LoC, and have no-one
> notice that in review. I think that's a reasonable tradeoff.

I'm inclined to agree with Daniel here.

Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
review seems reasonably unlikely.

And after reading this thread I'm still not entirely sure we have a
complete list of what should be on that whitelist, and there are only a
limited number of systems I can test build on right now.

Reviewed-by: Derek Foreman 

> 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: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 12:11, Daniel Stone  wrote:
> On 20 March 2018 at 11:55, Emil Velikov  wrote:
>> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>> MIPS anyway:
>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>
>>> And probably some more for ARM toolchains using other linkers:
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>
>>> Doing a quick check across all the architectures on Debian shows that
>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>> need to respin for those as well, but I think at this point the
>>> inconvenience of maintaining a list of every linker's implementation
>>> detail on every platform, outweighs the risk of an exported
>>> underscore-prefixed symbol slipping through review.
>>
>> I think you're preemptively worried about this. Plus there's no need
>> to add every symbol a search could find.
>>
>> Even then, ask yourself the question:
>> Which is better - updating an ugly looking list once in a blue moon or
>> having apps use internal API?
>>
>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>> We've already spend more time researching than what the [expected]
>> maintenance for 1 year would be ;-)
>
> It all depends on how you view scope and probability.
>
> I don't agree my concern is preemptive: we've just shipped a beta
> where 'make check' fails on a few architectures, and the proposed fix
> (adding several symbols to the platform list) will still fail on
> architectures Debian ships on, until it also adds _fbss, _fdata and
> _ftext. I think my suggestion is reasonable: it fixes the very real
> problem, in a way which avoids expanding the scope of our ABI checker
> to include the implementation details of every linker/architecture
> pair that people use Wayland on. The cost of this is the probability
> that someone manages to add a new underscored symbol marked with
> WL_EXPORT into a library which is currently 105 LoC, and have no-one
> notice that in review. I think that's a reasonable tradeoff.
>
Right, hopefully there won't be any internals leaked/used by other projects.
I reserve myself the right of the lovely "I told you so".

Feel free to continue with whichever solution you feel comfortable with.

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

Re: [PATCH weston 1/7] compositor-drm: remove superfluos get_disable_state call

[Daniel Stone]

Hi Emre,

On 20 March 2018 at 14:28, Emre Ucan  wrote:
> drm_output_get_disable_state function returns
> a duplicated output_state object.
>
> Here we are creating the object, but we are
> never using it. Therefore, it is safe to remove.
>
> (Found by clang source code analyzer)

It's subtle, but we do need this here.

> @@ -3860,7 +3860,6 @@ drm_set_dpms(struct weston_output *output_base, enum 
> dpms_enum level)

It's out of diff context, the if branch we're in guarantees that we
have pending_state, i.e. after repaint_begin() but before
repaint_flush().

> state = drm_pending_state_get_output(pending_state, output);
> if (state)
> drm_output_state_free(state);

Here we search pending_state for any existing output state for this
output. If there are any, we free them and remove them from the
pending state. So after this line, we are guaranteed not to be
touching the output in this pending state.

> -   state = drm_output_get_disable_state(pending_state, output);

This line adds a disable state to the pending_state. We do not do
anything with it: we just keep it there in the pending_state, to be
applied when repaint_flush() is called. If we remove this, then we
won't disable the output.

Replacing 'state = ' with '(void) ' as the prefix of this line might
make it clearer; does that also quiet Coverity?

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

Re: [PATCH] xwm: Fix memory leak

[Scott Moreau]

Hi Pekka,

On Tue, Mar 20, 2018 at 2:21 AM, Pekka Paalanen  wrote:

> On Mon, 19 Mar 2018 18:06:03 -0600
> Scott Moreau  wrote:
>
> > Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was
> > calling xcb_get_property_reply() without freeing the reply.
> > ---
> >  xwayland/window-manager.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> > index c307e19..24e7213 100644
> > --- a/xwayland/window-manager.c
> > +++ b/xwayland/window-manager.c
> > @@ -1387,6 +1387,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct
> weston_wm_window *window)
> >   CAIRO_FORMAT_ARGB32,
> >   width, height, width *
> 4);
> >
> > + free(reply);
> > +
> >   /* Bail out in case anything wrong happened during surface
> creation. */
> >   if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
> >   cairo_surface_destroy(new_surface);
>
> Hi,
>
> this is strictly an improvement, so I pushed it:
>dc402462..d2cb711d  master -> master
>

Thanks for picking it up, I thought this was a simple one at first too.


>
> It does still miss the early error returns the function, though, so
> it's only a partial solution.
>
> A few seconds later, I thought of a problem with this patch, so I had
> to push a revert immediately (no force-pushing allowed):
>d2cb711d..9fe5d5fa  master -> master
>
> I explained the issue in the revert, and it boils down to
> cairo_image_surface_create_for_data() not making a copy of the memory
> it is passed in, instead it will just keep using the pointer passed in,
> which means we cannot free the data until the cairo surface is
> destroyed. So while there is a leak to fix, the fix needs to be more
> involved.
>

Yes it does need a more involved fix. After hooking up a destroy listener
for the cairo surface, I noticed that window->icon_surface is never being
destroyed either, and there isn't a straight forward fix especially since
window->icon_surface being set to NULL in weston_wm_window_create_frame().

Also as a side note, I notice some strange effect sometimes where the icon
will appear where the mouse cursor is for a split second when opening a
window with an icon, and quickly do fade out animation on it.

Thanks,
Scott


>
> This happens when I try to rush things. Sorry.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston 6/7] compositor: initialize ret in repaint_timer_handler

[Emre Ucan]

If output_list of compositor is empty, value of
ret is read without initialization.

Signed-off-by: Emre Ucan 
---
 libweston/compositor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 67b5d28..4816f33 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2457,7 +2457,7 @@ output_repaint_timer_handler(void *data)
struct weston_output *output;
struct timespec now;
void *repaint_data = NULL;
-   int ret;
+   int ret = 0;
 
weston_compositor_read_presentation_clock(compositor, );
 
-- 
2.7.4

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

[PATCH weston 5/7] gl-renderer: set num_images after import_simple_dmabuf

[Emre Ucan]

we have to set num_images after import_simple_dmabuf
call. Otherwise, egl_images will not be correctly
referenced in gl_renderer_attach_dmabuf.

(Found by clang source code analyzer)

Signed-off-by: Emre Ucan 
---
 libweston/gl-renderer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index a6b29a9..2c50d2d 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -2216,6 +2216,7 @@ import_known_dmabuf(struct gl_renderer *gr,
image->images[0] = import_simple_dmabuf(gr, 
>dmabuf->attributes);
if (!image->images[0])
return false;
+   image->num_images = 1;
break;
 
case IMPORT_TYPE_GL_CONVERSION:
-- 
2.7.4

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

[PATCH weston 7/7] ivi-shell: remove dead assignments in layer transition

[Emre Ucan]

Signed-off-by: Emre Ucan 
---
 ivi-shell/ivi-layout-transition.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ivi-shell/ivi-layout-transition.c 
b/ivi-shell/ivi-layout-transition.c
index b887ff6..a223b58 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -843,10 +843,10 @@ ivi_layout_transition_fade_layer(
uint32_t duration)
 {
struct ivi_layout_transition *transition;
-   struct fade_layer_data *data = NULL;
-   wl_fixed_t fixed_opacity = 0.0;
-   double now_opacity = 0.0;
-   double remain = 0.0;
+   struct fade_layer_data *data;
+   wl_fixed_t fixed_opacity;
+   double now_opacity;
+   double remain;
 
transition = get_transition_from_type_and_id(
IVI_LAYOUT_TRANSITION_LAYER_FADE,
@@ -858,7 +858,6 @@ ivi_layout_transition_fade_layer(
/* FIXME */
fixed_opacity = layer->prop.opacity;
now_opacity = wl_fixed_to_double(fixed_opacity);
-   remain = 0.0;
 
data->is_fade_in = is_fade_in;
data->start_alpha = now_opacity;
-- 
2.7.4

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

[PATCH weston 0/7] Fix source code analyzer findings

[Emre Ucan]

These patches fix some of the findings which are found by
clang source code analyzer.

Emre Ucan (7):
  compositor-drm: remove superfluos get_disable_state call
  compositor-drm: remove dead assigment in drm_fb_create_dumb
  hmi-controller: remove dead assignments in add_launchers
  input: fix use-after-free issue at pointer_cancel
  gl-renderer: set num_images after import_simple_dmabuf
  compositor: initialize ret in repaint_timer_handler
  ivi-shell: remove dead assignments in layer transition

 ivi-shell/hmi-controller.c|  4 
 ivi-shell/ivi-layout-transition.c |  9 -
 libweston/compositor-drm.c|  3 ---
 libweston/compositor.c|  2 +-
 libweston/gl-renderer.c   |  1 +
 libweston/input.c | 12 
 6 files changed, 6 insertions(+), 25 deletions(-)

-- 
2.7.4

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

[PATCH weston 4/7] input: fix use-after-free issue at pointer_cancel

[Emre Ucan]

If the constraint is an one-shot constraint, constraint
is freed in disable_pointer_constraint function.
Therefore, we should not try to read freed memory at
"switch (constraint->lifetime)" statement.

The removed code is anyway superfluous. Because
surface destroy signal is only removed, when constraint
is an one-shot constraint.

(Found by clang source code analyzer)

Signed-off-by: Emre Ucan 
---
 libweston/input.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/libweston/input.c b/libweston/input.c
index 3e91c26..a9d21cb 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -4577,18 +4577,6 @@ confined_pointer_grab_pointer_cancel(struct 
weston_pointer_grab *grab)
container_of(grab, struct weston_pointer_constraint, grab);
 
disable_pointer_constraint(constraint);
-
-   /* If this is a persistent constraint, re-add the surface destroy signal
-* listener only if we are currently not destroying the surface. */
-   switch (constraint->lifetime) {
-   case ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_PERSISTENT:
-   if (constraint->surface->resource)
-   wl_signal_add(>surface->destroy_signal,
- >surface_destroy_listener);
-   break;
-   case ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_ONESHOT:
-   break;
-   }
 }
 
 static const struct weston_pointer_grab_interface
-- 
2.7.4

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

[PATCH weston 3/7] hmi-controller: remove dead assignments in add_launchers

[Emre Ucan]

assigned values of x, y, ret and layout_surface are
never read.

(Found by clang source code analyzer)

Signed-off-by: Emre Ucan 
---
 ivi-shell/hmi-controller.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
index b13936f..0e44df8 100644
--- a/ivi-shell/hmi-controller.c
+++ b/ivi-shell/hmi-controller.c
@@ -1184,10 +1184,6 @@ ivi_hmi_controller_add_launchers(struct hmi_controller 
*hmi_ctrl,
  compare_launcher_info);
 
wl_array_for_each(data, ) {
-   x = 0;
-   y = 0;
-   ret = 0;
-   layout_surface = NULL;
add_surface_id = wl_array_add(_ctrl->ui_widgets,
  sizeof(*add_surface_id));
 
-- 
2.7.4

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

[PATCH weston 1/7] compositor-drm: remove superfluos get_disable_state call

[Emre Ucan]

drm_output_get_disable_state function returns
a duplicated output_state object.

Here we are creating the object, but we are
never using it. Therefore, it is safe to remove.

(Found by clang source code analyzer)

Signed-off-by: Emre Ucan 
---
 libweston/compositor-drm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 81ca67d..f8c13ee 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3860,7 +3860,6 @@ drm_set_dpms(struct weston_output *output_base, enum 
dpms_enum level)
state = drm_pending_state_get_output(pending_state, output);
if (state)
drm_output_state_free(state);
-   state = drm_output_get_disable_state(pending_state, output);
return;
}
 
-- 
2.7.4

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

[PATCH weston 2/7] compositor-drm: remove dead assigment in drm_fb_create_dumb

[Emre Ucan]

ret is overwritten by drmModeAddFB2 call

(Found by clang source code analyzer)

Signed-off-by: Emre Ucan 
---
 libweston/compositor-drm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index f8c13ee..506ded5 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -891,8 +891,6 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int 
height,
fb->height = height;
fb->fd = b->drm.fd;
 
-   ret = -1;
-
handles[0] = fb->handle;
pitches[0] = fb->stride;
offsets[0] = 0;
-- 
2.7.4

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

[PATCH] xdg-shell: add enums for tiled window state to toplevel configure

[Mike Blumenkrantz]

this adds implementation from a related discussion long ago in which
it was decided that it would be useful for clients to know if/where their
windows were tiled so that various behaviors and visuals could be modified
to improve UX

a window which is e.g., tiled on the right side of the screen would set the
right|top|bottom tiled states in configure

Signed-off-by: Mike Blumenkrantz 
---
 stable/xdg-shell/xdg-shell.xml | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/stable/xdg-shell/xdg-shell.xml b/stable/xdg-shell/xdg-shell.xml
index d524ea9..a87527c 100644
--- a/stable/xdg-shell/xdg-shell.xml
+++ b/stable/xdg-shell/xdg-shell.xml
@@ -29,7 +29,7 @@
 DEALINGS IN THE SOFTWARE.
   
 
-  
+  
 
   The xdg_wm_base interface is exposed as a global object enabling clients
   to turn their wl_surfaces into windows in a desktop environment. It
@@ -115,7 +115,7 @@
 
   
 
-  
+  
 
   The xdg_positioner provides a collection of rules for the placement of a
   child surface relative to a parent surface. Rules can be defined to 
ensure
@@ -359,7 +359,7 @@
 
   
 
-  
+  
 
   An interface that may be implemented by a wl_surface, for
   implementations that provide a desktop-style user interface.
@@ -528,7 +528,7 @@
 
   
 
-  
+  
 
   This interface defines an xdg_surface role which allows a surface to,
   among other things, set window-like properties such as maximize,
@@ -750,6 +750,30 @@
  keyboard or pointer focus.

   
+  
+   
+ The window is currently in a tiled layout and the left edge is 
considered to be
+ adjacent to another part of the tiling grid.
+   
+  
+  
+   
+ The window is currently in a tiled layout and the right edge is 
considered to be
+ adjacent to another part of the tiling grid.
+   
+  
+  
+   
+ The window is currently in a tiled layout and the top edge is 
considered to be
+ adjacent to another part of the tiling grid.
+   
+  
+  
+   
+ The window is currently in a tiled layout and the bottom edge is 
considered to be
+ adjacent to another part of the tiling grid.
+   
+  
 
 
 
@@ -989,7 +1013,7 @@
 
   
 
-  
+  
 
   A popup surface is a short-lived, temporary surface. It can be used to
   implement for example menus, popovers, tooltips and other similar user
-- 
2.14.3

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

Re: [PATCH wayland 1/2] wayland-egl-symbols-check: add ARM specific glib entrypoints

[Simon McVittie]

On Tue, 20 Mar 2018 at 11:10:44 +, Emil Velikov wrote:
> Subject: [PATCH wayland 1/2] wayland-egl-symbols-check: add ARM specific
> glib entrypoints

ARM-specific *glibc* entry points, GLib has nothing to do with this :-)

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

On 20 March 2018 at 11:55, Emil Velikov  wrote:
> On 20 March 2018 at 11:46, Daniel Stone  wrote:
>> Sure. As on IRC though, we definitely need to add at least _ftext for
>> MIPS anyway:
>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>
>> And probably some more for ARM toolchains using other linkers:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>
>> Doing a quick check across all the architectures on Debian shows that
>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>> need to respin for those as well, but I think at this point the
>> inconvenience of maintaining a list of every linker's implementation
>> detail on every platform, outweighs the risk of an exported
>> underscore-prefixed symbol slipping through review.
>
> I think you're preemptively worried about this. Plus there's no need
> to add every symbol a search could find.
>
> Even then, ask yourself the question:
> Which is better - updating an ugly looking list once in a blue moon or
> having apps use internal API?
>
> As said above: _if_ it turns out to be labour intensive - fine remove it.
> We've already spend more time researching than what the [expected]
> maintenance for 1 year would be ;-)

It all depends on how you view scope and probability.

I don't agree my concern is preemptive: we've just shipped a beta
where 'make check' fails on a few architectures, and the proposed fix
(adding several symbols to the platform list) will still fail on
architectures Debian ships on, until it also adds _fbss, _fdata and
_ftext. I think my suggestion is reasonable: it fixes the very real
problem, in a way which avoids expanding the scope of our ABI checker
to include the implementation details of every linker/architecture
pair that people use Wayland on. The cost of this is the probability
that someone manages to add a new underscored symbol marked with
WL_EXPORT into a library which is currently 105 LoC, and have no-one
notice that in review. I think that's a reasonable tradeoff.

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 11:46, Daniel Stone  wrote:
> Hi Emil,
>
> On 20 March 2018 at 11:30, Emil Velikov  wrote:
>> On 20 March 2018 at 11:01, Daniel Stone  wrote:
>>> Rather than a hard-coded list of platform symbols, just ignore anything
>>> prefaced with an underscore. This fixes breakage on ARM, which declares
>>> several slightly different platform symbols to x86.
>>
>> FWIW I've explicitly opted against this kind of heuristics for a few reasons:
>>
>>  - projects use single/double underscored symbols for internal API
>>  - other projects have been using such symbols knowingly that it's internal 
>> API
>> Last but not least
>>  - the symbols exposed by the C runtime should stay stable
>>
>> In other words - prevents others from going silly things, only the
>> list might need 1-2 updates.
>> If the latter turns out to be false we can nuke it.
>
> Sure. As on IRC though, we definitely need to add at least _ftext for
> MIPS anyway:
> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>
> And probably some more for ARM toolchains using other linkers:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>
> Doing a quick check across all the architectures on Debian shows that
> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
> need to respin for those as well, but I think at this point the
> inconvenience of maintaining a list of every linker's implementation
> detail on every platform, outweighs the risk of an exported
> underscore-prefixed symbol slipping through review.
>
I think you're preemptively worried about this. Plus there's no need
to add every symbol a search could find.

Even then, ask yourself the question:
Which is better - updating an ugly looking list once in a blue moon or
having apps use internal API?

As said above: _if_ it turns out to be labour intensive - fine remove it.
We've already spend more time researching than what the [expected]
maintenance for 1 year would be ;-)

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

Hi Emil,

On 20 March 2018 at 11:30, Emil Velikov  wrote:
> On 20 March 2018 at 11:01, Daniel Stone  wrote:
>> Rather than a hard-coded list of platform symbols, just ignore anything
>> prefaced with an underscore. This fixes breakage on ARM, which declares
>> several slightly different platform symbols to x86.
>
> FWIW I've explicitly opted against this kind of heuristics for a few reasons:
>
>  - projects use single/double underscored symbols for internal API
>  - other projects have been using such symbols knowingly that it's internal 
> API
> Last but not least
>  - the symbols exposed by the C runtime should stay stable
>
> In other words - prevents others from going silly things, only the
> list might need 1-2 updates.
> If the latter turns out to be false we can nuke it.

Sure. As on IRC though, we definitely need to add at least _ftext for
MIPS anyway:
https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74

And probably some more for ARM toolchains using other linkers:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html

Doing a quick check across all the architectures on Debian shows that
your updated list is also missing _fbss, _fdata, and _ftext. So we'd
need to respin for those as well, but I think at this point the
inconvenience of maintaining a list of every linker's implementation
detail on every platform, outweighs the risk of an exported
underscore-prefixed symbol slipping through review.

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

Re: [PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 11:01, Daniel Stone  wrote:
> Rather than a hard-coded list of platform symbols, just ignore anything
> prefaced with an underscore. This fixes breakage on ARM, which declares
> several slightly different platform symbols to x86.
>
FWIW I've explicitly opted against this kind of heuristics for a few reasons:

 - projects use single/double underscored symbols for internal API
 - other projects have been using such symbols knowingly that it's internal API
Last but not least
 - the symbols exposed by the C runtime should stay stable

In other words - prevents others from going silly things, only the
list might need 1-2 updates.
If the latter turns out to be false we can nuke it.

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

[PATCH wayland 2/2] .gitignore: add wayland-egl-abi-check

[Emil Velikov]

From: Emil Velikov 

Instruct git go ignore the file, in case we've done an in-tree build.

Cc: Derek Foreman 
Signed-off-by: Emil Velikov 
---
I've opted for the shortest fix - add an entry instead of renaming
files, updating build rules, etc.

If people really want to we can opt for that route.
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 8da9861..eadea12 100644
--- a/.gitignore
+++ b/.gitignore
@@ -43,5 +43,6 @@ Makefile
 Makefile.in
 exec-fd-leak-checker
 fixed-benchmark
+/wayland-egl-abi-check
 /wayland-scanner
 protocol/*.[ch]
-- 
2.16.0

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

[PATCH wayland 1/2] wayland-egl-symbols-check: add ARM specific glib entrypoints

[Emil Velikov]

From: Emil Velikov 

Seems like glibc on ARM (32 or 64 bit) exports a few extra symbols.
Add them to the list.

Quick look reveals that those are aliases of the already listed symbols,
and have been added to glibc since day 1 :-\

Fixes: 21b1f22eb05 ("wayland-egl: enhance the symbol test")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105620
Signed-off-by: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 70fe1f4..6e6655e 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -17,7 +17,11 @@ fi
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
 
 # Platform specific symbols.
-PLAT_FUNCS="__bss_start
+PLAT_FUNCS="__bss_end__
+__bss_start
+__bss_start__
+__end__
+_bss_end__
 _edata
 _end
 _fini
-- 
2.16.0

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

[PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Daniel Stone]

Rather than a hard-coded list of platform symbols, just ignore anything
prefaced with an underscore. This fixes breakage on ARM, which declares
several slightly different platform symbols to x86.

Signed-off-by: Daniel Stone 
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105620
Cc: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 70fe1f4c..d04fd042 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -16,14 +16,6 @@ fi
 
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
 
-# Platform specific symbols.
-PLAT_FUNCS="__bss_start
-_edata
-_end
-_fini
-_init
-"
-
 # Official ABI, taken from the header.
 REQ_FUNCS="wl_egl_window_resize
 wl_egl_window_create
@@ -32,8 +24,8 @@ wl_egl_window_get_attached_size
 "
 
 NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
+echo "$func" | grep -q "^_" && continue
 echo "$REQ_FUNCS" | grep -q "^$func$" && continue
-echo "$PLAT_FUNCS" | grep -q "^$func$" && continue
 
 echo $func
 done)
-- 
2.16.2

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

[PATCH weston 3/3] simple-dmabuf-drm: support DRM_FORMAT_LINEAR for NV12 as well

[Guido Günther]

This makes --import-format=NV12 testable on e.g. intel

We only set nv12_format_found to true if we found that format and at
least one understood modifier. Store modifier verbatim instead of using
a boolean flag. Last advertised and supported modifier currently wins.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 54 +++--
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index b9681ca4..7175f2d7 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -75,7 +75,7 @@ struct display {
struct zwp_linux_dmabuf_v1 *dmabuf;
int xrgb_format_found;
int nv12_format_found;
-   int nv12_modifier_found;
+   uint64_t nv12_modifier;
int req_dmabuf_immediate;
int req_dmabuf_modifiers;
 };
@@ -273,7 +273,7 @@ fd_device_destroy(struct buffer *buf)
 #endif /* HAVE_LIBDRM_FREEDRENO */
 
 static void
-fill_content(struct buffer *my_buf)
+fill_content(struct buffer *my_buf, uint64_t modifier)
 {
int x = 0, y = 0;
uint32_t *pix;
@@ -281,11 +281,31 @@ fill_content(struct buffer *my_buf)
assert(my_buf->mmap);
 
if (my_buf->format == DRM_FORMAT_NV12) {
-   pix = (uint32_t *) my_buf->mmap;
-   for (y = 0; y < my_buf->height; y++)
-   memcpy([y * my_buf->width / 4],
-  _tiled[my_buf->width * y / 4],
-  my_buf->width);
+   if (modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE) {
+   pix = (uint32_t *) my_buf->mmap;
+   for (y = 0; y < my_buf->height; y++)
+   memcpy([y * my_buf->width / 4],
+  _tiled[my_buf->width * y / 4],
+  my_buf->width);
+   }
+   else if (modifier == DRM_FORMAT_MOD_LINEAR) {
+   uint8_t *pix8;
+   /* first plane: Y (2/3 of the buffer)   */
+   for (y = 0; y < my_buf->height * 2/3; y++) {
+   pix8 = my_buf->mmap + y * my_buf->stride;
+   for (x = 0; x < my_buf->width; x++)
+   *pix8++ = x % 0xff;
+   }
+   /* second plane (CbCr) is half the size of Y
+  plane (last 1/3 of the buffer) */
+   for (y = my_buf->height * 2/3; y < my_buf->height; y++) 
{
+   pix8 = my_buf->mmap + y * my_buf->stride;
+   for (x = 0; x < my_buf->width; x+=2) {
+   *pix8++ = x % 256;
+   *pix8++ = y % 256;
+   }
+   }
+   }
}
else {
for (y = 0; y < my_buf->height; y++) {
@@ -420,7 +440,6 @@ create_dmabuf_buffer(struct display *display, struct buffer 
*buffer,
/* adjust height for allocation of NV12 Y and UV planes */
buffer->height = height * 3 / 2;
buffer->bpp = 8;
-   modifier = DRM_FORMAT_MOD_SAMSUNG_64_32_TILE;
break;
default:
buffer->height = height;
@@ -437,7 +456,7 @@ create_dmabuf_buffer(struct display *display, struct buffer 
*buffer,
fprintf(stderr, "map_bo failed\n");
goto error2;
}
-   fill_content(buffer);
+   fill_content(buffer, modifier);
drm_dev->unmap_bo(buffer);
 
if (drm_dev->export_bo_to_prime(buffer) != 0) {
@@ -685,10 +704,13 @@ dmabuf_modifiers(void *data, struct zwp_linux_dmabuf_v1 
*zwp_linux_dmabuf,
d->xrgb_format_found = 1;
break;
case DRM_FORMAT_NV12:
-   d->nv12_format_found = 1;
-   if (modifier == DRM_FORMAT_MOD_SAMSUNG_64_32_TILE)
-   d->nv12_modifier_found = 1;
-   break;
+   switch (modifier) {
+   case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
+   case DRM_FORMAT_MOD_LINEAR:
+   d->nv12_format_found = 1;
+   d->nv12_modifier = modifier;
+   break;
+   }
default:
break;
}
@@ -796,12 +818,10 @@ create_display(int opts, int format)
wl_display_roundtrip(display->display);
 
if ((format == DRM_FORMAT_XRGB && !display->xrgb_format_found) 
||
-   (format == DRM_FORMAT_NV12 && (!display->nv12_format_found ||
-   !display->nv12_modifier_found))) {
+   (format == DRM_FORMAT_NV12 && !display->nv12_format_found)) {
fprintf(stderr, "requested format is not available\n");

[PATCH weston 1/3] simple-dmabuf-drm: drop superfluous declaration

[Guido Günther]

variable is defined in simple-dmabuf-drm.h

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 2975f3a5..1c062fad 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -58,7 +58,6 @@
 #include "fullscreen-shell-unstable-v1-client-protocol.h"
 #include "linux-dmabuf-unstable-v1-client-protocol.h"
 
-extern const unsigned nv12_tiled[];
 struct buffer;
 
 /* Possible options that affect the displayed image */
-- 
2.16.1

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

[PATCH weston 0/3] simple-dmabuf-drm: support DRM_FORMAT_LINEAR for NV12

[Guido Günther]

This makes NV12 testable on non freedreno as well. E.g. intel supports it.

Guido Günther (3):
  simple-dmabuf-drm: drop superfluous declaration
  simple-dmabuf-drm: don't exit from create_display
  simple-dmabuf-drm: support DRM_FORMAT_LINEAR for NV12 as well

 clients/simple-dmabuf-drm.c | 63 ++---
 1 file changed, 42 insertions(+), 21 deletions(-)

-- 
2.16.1

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

[PATCH weston 2/3] simple-dmabuf-drm: don't exit from create_display

[Guido Günther]

Only exit from main so control flow is in one place.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 1c062fad..b9681ca4 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -767,7 +767,7 @@ create_display(int opts, int format)
display = malloc(sizeof *display);
if (display == NULL) {
fprintf(stderr, "out of memory\n");
-   exit(1);
+   return NULL;
}
display->display = wl_display_connect(NULL);
assert(display->display);
@@ -790,7 +790,7 @@ create_display(int opts, int format)
wl_display_roundtrip(display->display);
if (display->dmabuf == NULL) {
fprintf(stderr, "No zwp_linux_dmabuf global\n");
-   exit(1);
+   return NULL;
}
 
wl_display_roundtrip(display->display);
@@ -799,7 +799,7 @@ create_display(int opts, int format)
(format == DRM_FORMAT_NV12 && (!display->nv12_format_found ||
!display->nv12_modifier_found))) {
fprintf(stderr, "requested format is not available\n");
-   exit(1);
+   return NULL;
}
 
return display;
@@ -909,6 +909,8 @@ main(int argc, char **argv)
}
 
display = create_display(opts, import_format);
+   if (!display)
+   return 1;
window = create_window(display, 256, 256, import_format, opts);
if (!window)
return 1;
-- 
2.16.1

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

[PATCH weston v5 3/3] simple-dmabuf-drm: support etnaviv drm as well

[Guido Günther]

Reviewed-by: Derek Foreman 
Signed-off-by: Guido Günther 
---
 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 68 +
 configure.ac|  5 +++-
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 69ca6cba..64a8006c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES =   \
 weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
$(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
 weston_simple_dmabuf_drm_LDADD = $(SIMPLE_DMABUF_DRM_CLIENT_LIBS) \
$(LIBDRM_PLATFORM_FREEDRENO_LIBS) \
+   $(LIBDRM_PLATFORM_ETNAVIV_LIBS)   \
$(LIBDRM_PLATFORM_INTEL_LIBS) \
libshared.la
 endif
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index cb04622f..e7a7b0ed 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -49,6 +50,9 @@
 #ifdef HAVE_LIBDRM_FREEDRENO
 #include 
 #endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+#include 
+#endif
 #include 
 
 #include 
@@ -108,6 +112,10 @@ struct buffer {
struct fd_device *fd_dev;
struct fd_bo *fd_bo;
 #endif /* HAVE_LIBDRM_FREEDRENO */
+#if HAVE_LIBDRM_ETNAVIV
+   struct etna_device *etna_dev;
+   struct etna_bo *etna_bo;
+#endif /* HAVE_LIBDRM_ETNAVIV */
 
uint32_t gem_handle;
int dmabuf_fd;
@@ -265,6 +273,56 @@ fd_device_destroy(struct buffer *buf)
fd_device_del(buf->fd_dev);
 }
 #endif /* HAVE_LIBDRM_FREEDRENO */
+#ifdef HAVE_LIBDRM_ETNAVIV
+
+static int
+etna_alloc_bo(struct buffer *buf)
+{
+   int flags = DRM_ETNA_GEM_CACHE_WC;
+   int size;
+
+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size =  buf->stride * buf->height;
+   buf->etna_dev = etna_device_new(buf->drm_fd);
+   buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
+
+   return buf->etna_bo != NULL;
+}
+
+static void
+etna_free_bo(struct buffer *buf)
+{
+   etna_bo_del(buf->etna_bo);
+}
+
+static int
+etna_bo_export_to_prime(struct buffer *buf)
+{
+   buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
+   return buf->dmabuf_fd < 0;
+}
+
+static int
+etna_map_bo(struct buffer *buf)
+{
+   buf->mmap = etna_bo_map(buf->etna_bo);
+   return buf->mmap != NULL;
+}
+
+static void
+etna_unmap_bo(struct buffer *buf)
+{
+   if (munmap(buf->mmap, buf->stride * buf->height) < 0)
+   fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));
+   buf->mmap = NULL;
+}
+
+static void
+etna_device_destroy(struct buffer *buf)
+{
+   etna_device_del(buf->etna_dev);
+}
+#endif /* HAVE_LIBDRM_ENTAVIV */
 
 static void
 fill_content(struct buffer *my_buf)
@@ -332,6 +390,16 @@ drm_device_init(struct buffer *buf)
dev->unmap_bo = fd_unmap_bo;
dev->device_destroy = fd_device_destroy;
}
+#endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+   else if (!strcmp(dev->name, "etnaviv")) {
+   dev->alloc_bo = etna_alloc_bo;
+   dev->free_bo = etna_free_bo;
+   dev->export_bo_to_prime = etna_bo_export_to_prime;
+   dev->map_bo = etna_map_bo;
+   dev->unmap_bo = etna_unmap_bo;
+   dev->device_destroy = etna_device_destroy;
+   }
 #endif
else {
fprintf(stderr, "Error: drm device %s unsupported.\n",
diff --git a/configure.ac b/configure.ac
index 070e61b9..d35f2255 100644
--- a/configure.ac
+++ b/configure.ac
@@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
then
   PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
   AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
   [true])
+  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
+  AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
+  [true])
 
   if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \
  "x$have_simple_dmabuf_libs" = "xno" && \
  test "x$enable_simple_dmabuf_drm_client" = "xyes"; then
-AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
libdrm_freedreno not found])
+AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but none of 
libdrm_{intel,freedreno,etnaviv} found])
   fi
 
   if test "x$have_simple_dmabuf_drm_client" = "xyes" -a 
"x$have_simple_dmabuf_libs" = "xyes"; then
-- 
2.16.1

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

[PATCH weston v5 2/3] simple-dmabuf-drm: simplify fd_map_bo

[Guido Günther]

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 19e8dbb1..cb04622f 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -251,11 +251,7 @@ static int
 fd_map_bo(struct buffer *buf)
 {
buf->mmap = fd_bo_map(buf->fd_bo);
-
-   if (buf->mmap != NULL)
-   return 1;
-
-   return 0;
+   return buf->mmap != NULL;
 }
 
 static void
-- 
2.16.1

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

[PATCH weston v5 1/3] simple-dmabuf-drm: 0 is a valid fd (freedreno)

[Guido Günther]

Reviewed-by: Eric Engestrom 
Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 2975f3a5..19e8dbb1 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -244,10 +244,7 @@ static int
 fd_bo_export_to_prime(struct buffer *buf)
 {
buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo);
-   if (buf->dmabuf_fd > 0)
-   return 0;
-
-   return 1;
+   return buf->dmabuf_fd < 0;
 }
 
 static int
-- 
2.16.1

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

[PATCH weston v5 0/3] simple-dmabuf-drm: Support etnaviv and freedreno cleanups

[Guido Günther]

Former patches 1-2 where already applied thanks Derek.

Changes from v4
- configure: use true if etnaviv drm was not found as with
  other drm backends (avoids an unused variable assignment)
- configure: use (hopefully) correct english
- simplify several if (cond) return 1; return 0; statements
- unmap buffer

Former patches 1-3 where already applied thanks Peka.

Changes from v3:
- Use ALIGN macro by default
- Don't multiply by bpp twice
- unmap buffer

Changes from v2:
- Use stride to calculate buffer size (etnaviv)
- Use stride to calculate buffer size (freedreno)
- Use less generic name for ALIGN macro

Changes from v1:
- Split up changes
- autoconf:
  - don't define unused have_ variables. Use true instead to
prevent abort of PKG_CHECK_MODULES
  - use && instead of -a in test
  - properly check variables
- use vfunc for drm_device_destroy


Guido Günther (3):
  simple-dmabuf-drm: 0 is a valid fd (freedreno)
  simple-dmabuf-drm: simplify fd_map_bo
  simple-dmabuf-drm: support etnaviv drm as well

 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 79 +++--
 configure.ac|  5 ++-
 3 files changed, 75 insertions(+), 10 deletions(-)

-- 
2.16.1

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

Re: [PATCH] xwm: Fix memory leak

[Pekka Paalanen]

On Mon, 19 Mar 2018 18:06:03 -0600
Scott Moreau  wrote:

> Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was
> calling xcb_get_property_reply() without freeing the reply.
> ---
>  xwayland/window-manager.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..24e7213 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -1387,6 +1387,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
> weston_wm_window *window)
>   CAIRO_FORMAT_ARGB32,
>   width, height, width * 4);
>  
> + free(reply);
> +
>   /* Bail out in case anything wrong happened during surface creation. */
>   if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
>   cairo_surface_destroy(new_surface);

Hi,

this is strictly an improvement, so I pushed it:
   dc402462..d2cb711d  master -> master

It does still miss the early error returns the function, though, so
it's only a partial solution.

A few seconds later, I thought of a problem with this patch, so I had
to push a revert immediately (no force-pushing allowed):
   d2cb711d..9fe5d5fa  master -> master

I explained the issue in the revert, and it boils down to
cairo_image_surface_create_for_data() not making a copy of the memory
it is passed in, instead it will just keep using the pointer passed in,
which means we cannot free the data until the cairo surface is
destroyed. So while there is a leak to fix, the fix needs to be more
involved.

This happens when I try to rush things. Sorry.


Thanks,
pq


pgpm1aAQO0Jge.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 v4 4/4] simple-dmabuf-drm: support etnaviv drm as well

[Pekka Paalanen]

On Mon, 19 Mar 2018 15:24:12 -0500
Derek Foreman  wrote:

> On 2018-03-19 11:45 AM, Guido Günther wrote:
> > Signed-off-by: Guido Günther 
> > ---
> >   Makefile.am |  1 +
> >   clients/simple-dmabuf-drm.c | 77 
> > +
> >   configure.ac|  5 ++-
> >   3 files changed, 82 insertions(+), 1 deletion(-)

> > +static int
> > +etna_map_bo(struct buffer *buf)
> > +{
> > +   buf->mmap = etna_bo_map(buf->etna_bo);
> > +
> > +   if (buf->mmap != NULL)
> > +   return 1;
> > +
> > +   return 0;
> > +}
> > +
> > +static void
> > +etna_unmap_bo(struct buffer *buf)
> > +{
> > +   if (munmap(buf->mmap, buf->stride * buf->height) < 0)
> > +   fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno)); 
> >  
> 
> Pekka had said something about cache flushing here in a previous 
> comment, I think.  I think this munmap() is enough, but if it isn't I'd 
> like to know what's required too.

Hi,

well, I was kind of hand-waving there. Surely the unmap function needs
to do *something*, otherwise it's at least a leak, leaves a driver
unmap/flush path untested, and what the flushing comment referred to
specifically, you often need to tell the driver/hardware somehow after
CPU access is over, because cache coherency is usually all lies.

I have no knowledge to review what a proper unmap is for this driver,
so if you say this code gets called and it seems to work ok, that's
enough for me. As long as it's doing something.

If something like etna_bo_unmap() does not exist, while etna_bo_map()
exists, you might want to look into why that is. Such asymmetry is
often an indicator of something missing.

> 
> > +   buf->mmap = NULL;
> > +}

Thanks,
pq


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

Re: [PATCH wayland] wayland-egl: Make symbol test fail on failure

[Pekka Paalanen]

On Mon, 19 Mar 2018 11:59:25 -0500
Derek Foreman  wrote:

> On 2018-03-19 11:20 AM, Eric Engestrom wrote:
> > On Monday, 2018-03-19 16:10:57 +, Daniel Stone wrote:  
> >> Hi,
> >>
> >> On 19 March 2018 at 16:08, Eric Engestrom  
> >> wrote:  
> >>> On Monday, 2018-03-19 15:13:14 +, Daniel Stone wrote:  
>  +if ! test -f "$LIB"; then
>  + echo "Test binary \"$LIB\" does not exist"
>  + exit 99
>  +fi
>  +
>  +if ! test -n "$NM"; then
>  + echo "nm environment variable not set"
>  + exit 99  
> >>>
> >>> 99? Were you looking for the "skip this test" 77?  
> >>
> >> I did mean 99 'some kind of inexplicable internal error happened'
> >> rather than 77 skip, but I have no strong opinion on it and am happy
> >> to change to whatever is suggested.  
> > 
> > I guess "don't have the tools to test this, skipping" would be fine, but
> > I'm not really involved in the wayland project so my opinion isn't the
> > one that matters the most :P  
> 
> Additional review is valuable, thanks. :)
> 
> > "I have no strong feelings one way or the other"  
> 
> In the absence of strong feelings, I've pushed this as-is.
> 
> Along with the recent version of the "pass nm path to check script" patch.

Hi,

I think 99 is the right one to use. It's an ABI breakage check, we
definitely don't want that to be optional.


Thanks,
pq


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