Re: [RFC wayland-protocols] Color management protocol

[Graeme Gill]

Carsten Haitzler (The Rasterman) wrote:
> On Sat, 17 Dec 2016 21:16:41 +1100 Graeme Gill  said:

>> That's not a typical situation though, but nothing special would be
>> happening - a new profile may be installed by the user as well,
>> in which case an application should re-render to accommodate
>> the change.
> 
> the user at most should be interacting with compositor settings/tools to
> configure a specific profile for a display (let's assume a fixed profile for
> that screen), so a compositor tool to tell the compositor the profile changed
> (pressed a button on the monitor to change it). when they alter the 
> compositor,
> then compositor can tell applications.

"Compositor tool" == color management application, such as ArgyllCMS "dispwin 
-I"!

>>  > yes. compositors right now work in display colorspace. they do no
>>  > conversions. eventually they SHOULD to display correctly. to do so they
>>  > need a color profile for the display.
>>
>> For enhanced color management yes. But core comes first, and is necessary
>> for many color critical applications, because the compositor will never
>> have the color transformations they require.
> 
> they will have to have them, or then not support that colorspace at all.

There's nothing "have to" about it. It's technically impossible for
a compositor to satisfy every applications color management requirements,
because they could be defined by the application, if it has sufficiently
specialized needs.

And I'm not sure what you mean by "supporting a colorspace".
A source colorspace is supported if a device profile exists for it that the CMM
knows how to handle. But that is not the complete story, because there
is then the details of how that profile is to be used, i.e. the
details of how it should be linked to the output profile.

> not in wayland. not acceptable. the app will never know which windows it is 
> on.
> as i have said - could be wrapped around a sphere or a room full of bunnies
> bouncing about across 8 screens.

It's also not acceptable that color be wrong. So how about trying to
come up with a solution, rather than saying "Wayland hasn't been
designed for that requirement up to now, so it can't be done" ?

> it can't if it doesn't know how a window or buffer is transformed or mapped to
> screens.

So provide mechanism for it to know!

>> The code is already there to do all that in color critical application.
> 
> then they get to pick a colorspace and render to that.

There is no colorspace to pick - a display has a characteristic
behavior. There is no choice in it, short of tweaking it's controls,
altering its calibration, or changing one of its settings.

> attahc that to the
> buffer.

Different display devices have different characteristics, hence
different profiles. So one choice will not work for a surface that
spans multiple displays.

> compositor will do whatever it wants. e.g. if that buffer is on the
> screen it matches it'd do no transform. maybe it does no transforms at all. 
> and
> simply hovers some indicator above the serface telling you if the surface
> colorspace matches the screens colorspace or not. maybe compositor disallows
> the window to be moved off the screen that matches the colorspace. that can be
> configured via the compositor.

The user will want to configure their windows as they see fit, including
moving images from one display to the other. There needs to be a mechanism
to allow color accurate display when this happens.

> and now you just hit "unacceptable in wayland land" space. which is why i tell
> you that this is not acceptable.

Wrong color is unacceptable too. So how do you solve it ?
(And it's a solved problem on all other systems. Why do
you want to cripple Wayland ?)

>> It's damage, just like any other, and color critical users using
>> color critical applications will take "trails" over wrong color
>> anytime. No "trails" and wrong color = a system they can't use.
> 
> and totally unacceptable for wayland space. so a big fat no to that kind of
> design.

So Wayland is a big fat no for anything that requires accurate color ?
Wow. And I though Wayland was intended to ultimately replace X11.
Apparently not!

>> Yes - exactly what I'm suggesting as core color management support.
> 
> but it doesnt have to tell you which screen it is nor tell you which screen
> your window is on. it's an option of 1 of various colorspaces to be able to 
> use
> given a specific compositor.

I don't see how this can work, except for limited color accuracy requirements.
Pick a small colorspace with a common gamut, and the full gamut of all displays
can't be used. Pick a large gamut and clipping policy can't be managed.

>> "Every pixel being perfect" except they are the wrong color, isn't perfect.
> 
> IF the compositor is doing the transformce of colorspaces, then you can 
> acheive
> perfect pixels.

No you can't, if the compositor doesn't have the information or algorithms
to do the 

[PATCH libinput 13/13] filter: tweak the magic slowdown

[Peter Hutterer]

Could be confirmation bias, but it feels better.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/filter.c b/src/filter.c
index ab503cf..d7a1515 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -40,7 +40,7 @@
  * technically correct but subjectively wrong, we expect a touchpad to be a
  * lot slower than a mouse. Apply a magic factor to slow down all movements
  */
-#define TP_MAGIC_SLOWDOWN 0.4 /* unitless factor */
+#define TP_MAGIC_SLOWDOWN 0.37 /* unitless factor */
 
 /* Convert speed/velocity from units/us to units/ms */
 static inline double
-- 
2.9.3

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

[PATCH libinput 11/13] filter: revamp the touchpad's acceleration code

[Peter Hutterer]

The previous code had three main issues:
* acceleration kicked in too early, so even slow movements were accelerated
* acceleration kicked in too quickly, there was only a very narrow window
  where we would have less than the max acceleration factor
* the max accel factor was too low for fast movements, so they still fell
  short of expectations

This patch revamps most of the acceleration though it keeps the basic shape of
the acceleration curve.

* The threshold is increased significantly so that faster movement
  still map to the finger movement. Acceleration doesn't kick in until we get
  to something that's really fast like a flick.
* The incline is dropped, so acceleration kicks in slower than before, i.e.
  the difference between the first speed that is accelerated and the speed
  that reaches the maximum is higher than before.
* The maximum acceleration is increased so ever faster movements get ever
  faster. The max is effectively out of reach now, if you move fast enough to
  hit this speed, your cursor will end up on the moon anyway.

A couple of other changes apply now too, specifically:
* The incline remains the same regardless of the speed
* The max accel factor remains the same regardless of the speed

The caculated factor changes with the speed set so that the base speed changes
with the desired speed.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index 7865c7e..ab503cf 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -135,9 +135,9 @@ filter_get_type(struct motion_filter *filter)
 #define DEFAULT_INCLINE 1.1/* unitless factor */
 
 /* Touchpad acceleration */
-#define TOUCHPAD_DEFAULT_THRESHOLD 40  /* mm/s */
-#define TOUCHPAD_MINIMUM_THRESHOLD 20  /* mm/s */
-#define TOUCHPAD_ACCELERATION 2.0  /* unitless factor */
+#define TOUCHPAD_DEFAULT_THRESHOLD 254 /* mm/s */
+#define TOUCHPAD_THRESHOLD_RANGE 184   /* mm/s */
+#define TOUCHPAD_ACCELERATION 9.0  /* unitless factor */
 #define TOUCHPAD_INCLINE 0.011 /* unitless factor */
 
 /* for the Lenovo x230 custom accel. do not touch */
@@ -521,19 +521,13 @@ touchpad_accelerator_set_speed(struct motion_filter 
*filter,
/* Note: the numbers below are nothing but trial-and-error magic,
   don't read more into them other than "they mostly worked ok" */
 
-   /* delay when accel kicks in */
+   /* adjust when accel kicks in */
accel_filter->threshold = TOUCHPAD_DEFAULT_THRESHOLD -
-   v_ms2us(0.25) * speed_adjustment;
-   if (accel_filter->threshold < TOUCHPAD_MINIMUM_THRESHOLD)
-   accel_filter->threshold = TOUCHPAD_MINIMUM_THRESHOLD;
-
-   /* adjust max accel factor */
-   accel_filter->accel = TOUCHPAD_ACCELERATION + speed_adjustment * 1.5;
-
-   /* higher speed -> faster to reach max */
-   accel_filter->incline = TOUCHPAD_INCLINE + speed_adjustment * 0.75;
-
+   TOUCHPAD_THRESHOLD_RANGE * speed_adjustment;
+   accel_filter->accel = TOUCHPAD_ACCELERATION;
+   accel_filter->incline = TOUCHPAD_INCLINE;
filter->speed_adjustment = speed_adjustment;
+
return true;
 }
 
@@ -805,6 +799,9 @@ touchpad_accel_profile_linear(struct motion_filter *filter,
/* Cap at the maximum acceleration factor */
factor = min(max_accel, factor);
 
+   /* Scale everything depending on the acceleration set */
+   factor *= 1 + 0.5 * filter->speed_adjustment;
+
return factor * TP_MAGIC_SLOWDOWN;
 }
 
-- 
2.9.3

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

[PATCH libinput 10/13] tools: switch the ptraccel-debug printf to use mm/s

[Peter Hutterer]

Signed-off-by: Peter Hutterer 
---
 tools/ptraccel-debug.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/tools/ptraccel-debug.c b/tools/ptraccel-debug.c
index 4ecb7e8..4fcb45f 100644
--- a/tools/ptraccel-debug.c
+++ b/tools/ptraccel-debug.c
@@ -141,19 +141,31 @@ print_ptraccel_sequence(struct motion_filter *filter,
}
 }
 
+/* mm/s → units/µs */
+static inline double
+mmps_to_upus(double mmps, int dpi)
+{
+   return mmps * (dpi/25.4) / 1e6;
+}
+
 static void
-print_accel_func(struct motion_filter *filter, accel_profile_func_t profile)
+print_accel_func(struct motion_filter *filter,
+accel_profile_func_t profile,
+int dpi)
 {
-   double vel;
+   double mmps;
 
printf("# gnuplot:\n");
-   printf("# set xlabel \"speed\"\n");
+   printf("# set xlabel \"speed (mm/s)\"\n");
printf("# set ylabel \"raw accel factor\"\n");
printf("# set style data lines\n");
-   printf("# plot \"gnuplot.data\" using 1:2\n");
-   for (vel = 0.0; vel < 0.004; vel += 0.001) {
-   double result = profile(filter, NULL, vel, 0 /* time */);
-   printf("%.8f\t%.4f\n", vel, result);
+   printf("# plot \"gnuplot.data\" using 1:2 title 'accel factor'\n");
+   printf("#\n");
+   printf("# data: velocity(mm/s) factor velocity(units/us)\n");
+   for (mmps = 0.0; mmps < 300.0; mmps += 1) {
+   double units_per_us = mmps_to_upus(mmps, dpi);
+   double result = profile(filter, NULL, units_per_us, 0 /* time 
*/);
+   printf("%.8f\t%.4f\t%.8f\n", mmps, result, units_per_us);
}
 }
 
@@ -338,7 +350,7 @@ main(int argc, char **argv)
}
 
if (print_accel)
-   print_accel_func(filter, profile);
+   print_accel_func(filter, profile, dpi);
else if (print_delta)
print_ptraccel_deltas(filter, step);
else if (print_motion)
-- 
2.9.3

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

[PATCH libinput 02/13] filter: drop the dpi_factor in favor of direct calculation

[Peter Hutterer]

This was badly since the factor was the ratio of "dpi:default dpi"

Most devices don't need it, so storing it in all filters event though we only
use it for some devices is confusing. Now that we have the dpi stored
directlyconfusing. Now that we have the dpi stored directly we might as well
use that.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index eecf4ca..06f07af 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -153,7 +153,6 @@ struct pointer_accelerator {
double accel;   /* unitless factor */
double incline; /* incline of the function */
 
-   double dpi_factor;
int dpi;
 };
 
@@ -161,7 +160,6 @@ struct pointer_accelerator_flat {
struct motion_filter base;
 
double factor;
-   double dpi_factor;
int dpi;
 };
 
@@ -382,7 +380,7 @@ accelerator_filter_low_dpi(struct motion_filter *filter,
double accel_value; /* unitless factor */
struct normalized_coords accelerated;
struct normalized_coords unnormalized;
-   double dpi_factor = accel->dpi_factor;
+   double dpi_factor = accel->dpi/(double)DEFAULT_MOUSE_DPI;
 
/* For low-dpi mice, use device units, everything else uses
   1000dpi normalized */
@@ -411,7 +409,7 @@ accelerator_filter_trackpoint(struct motion_filter *filter,
double accel_value; /* unitless factor */
struct normalized_coords accelerated;
struct normalized_coords unnormalized;
-   double dpi_factor = accel->dpi_factor;
+   double dpi_factor = accel->dpi/(double)DEFAULT_MOUSE_DPI;
 
/* trackpoints with a dpi factor have a const accel set, remove that
 * and restore device units. The accel profile takes const accel
@@ -568,8 +566,8 @@ pointer_accel_profile_linear_low_dpi(struct motion_filter 
*filter,
double max_accel = accel_filter->accel; /* unitless factor */
double threshold = accel_filter->threshold; /* units/us */
const double incline = accel_filter->incline;
+   double dpi_factor = accel_filter->dpi/(double)DEFAULT_MOUSE_DPI;
double factor; /* unitless */
-   double dpi_factor = accel_filter->dpi_factor;
 
/* dpi_factor is always < 1.0, increase max_accel, reduce
   the threshold so it kicks in earlier */
@@ -727,8 +725,8 @@ trackpoint_accel_profile(struct motion_filter *filter,
double max_accel = accel_filter->accel; /* unitless factor */
double threshold = accel_filter->threshold; /* units/ms */
const double incline = accel_filter->incline;
+   double dpi_factor = accel_filter->dpi/(double)DEFAULT_MOUSE_DPI;
double factor;
-   double dpi_factor = accel_filter->dpi_factor;
 
/* dpi_factor is always < 1.0, increase max_accel, reduce
   the threshold so it kicks in earlier */
@@ -775,8 +773,6 @@ create_default_filter(int dpi)
filter->threshold = DEFAULT_THRESHOLD;
filter->accel = DEFAULT_ACCELERATION;
filter->incline = DEFAULT_INCLINE;
-
-   filter->dpi_factor = dpi/(double)DEFAULT_MOUSE_DPI;
filter->dpi = dpi;
 
return filter;
@@ -878,8 +874,6 @@ create_pointer_accelerator_filter_lenovo_x230(int dpi)
filter->threshold = X230_THRESHOLD;
filter->accel = X230_ACCELERATION; /* unitless factor */
filter->incline = X230_INCLINE; /* incline of the acceleration function 
*/
-
-   filter->dpi_factor = 1; /* unused for this accel method */
filter->dpi = dpi;
 
return >base;
@@ -923,11 +917,12 @@ accelerator_filter_flat(struct motion_filter *filter,
double factor; /* unitless factor */
struct normalized_coords accelerated;
struct normalized_coords unnormalized;
+   double dpi_factor = accel_filter->dpi/(double)DEFAULT_MOUSE_DPI;
 
/* You want flat acceleration, you get flat acceleration for the
 * device */
-   unnormalized.x = unaccelerated->x * accel_filter->dpi_factor;
-   unnormalized.y = unaccelerated->y * accel_filter->dpi_factor;
+   unnormalized.x = unaccelerated->x * dpi_factor;
+   unnormalized.y = unaccelerated->y * dpi_factor;
factor = accel_filter->factor;
 
accelerated.x = factor * unnormalized.x;
@@ -984,7 +979,6 @@ create_pointer_accelerator_filter_flat(int dpi)
return NULL;
 
filter->base.interface = _interface_flat;
-   filter->dpi_factor = dpi/(double)DEFAULT_MOUSE_DPI;
filter->dpi = dpi;
 
return >base;
-- 
2.9.3

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

[PATCH libinput 06/13] filter: change the filter functions to take raw device coordinates

[Peter Hutterer]

We used to normalize all deltas to equivalents of a 1000dpi mouse before
passing it into the acceleration functions. This has a bunch of drawbacks, not
least that we already have to un-normalize back into device units for a few
devices already (trackpoints, tablet, low-dpi mice).

Switch the filter code over to use device units, relying on the dpi set
earlier during filter creation to convert to normalized. To make things easy,
the output of the filter code is still normalized data, i.e. data ready to be
handed to the libinput caller.

No effective functional changes. For touchpads, we still send normalized
coordinates (for now, anyway). For the various filter methods, we either drop
the places where we unnormalized before or we normalize where needed.

Two possible changes: for trackpoints and low-dpi mice we had a max dpi factor
of 1.0 before - now we don't anymore. This was only the case if a low-dpi
mouse had more than 1000dpi (never true) or a trackpoint had a const accel
lower than 1.0 (yeah, whatever).

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c |  16 +++-
 src/evdev-tablet.c  |   7 +-
 src/evdev.c |   2 +-
 src/filter-private.h|   4 +-
 src/filter.c| 227 ++--
 src/filter.h|  25 +-
 src/libinput-private.h  |   6 ++
 tools/ptraccel-debug.c  |  21 +++--
 8 files changed, 223 insertions(+), 85 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index a5ab4c2..20ba06f 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -51,11 +51,17 @@ tp_filter_motion(struct tp_dispatch *tp,
 const struct normalized_coords *unaccelerated,
 uint64_t time)
 {
+   struct device_float_coords raw;
+
if (normalized_is_zero(*unaccelerated))
return *unaccelerated;
 
+   /* Temporary solution only: convert back to raw coordinates, but
+* make sure we're on the same resolution for both axes */
+   raw = tp_unnormalize_for_xaxis(tp, *unaccelerated);
+
return filter_dispatch(tp->device->pointer.filter,
-  unaccelerated, tp, time);
+  , tp, time);
 }
 
 struct normalized_coords
@@ -63,11 +69,17 @@ tp_filter_motion_unaccelerated(struct tp_dispatch *tp,
   const struct normalized_coords *unaccelerated,
   uint64_t time)
 {
+   struct device_float_coords raw;
+
if (normalized_is_zero(*unaccelerated))
return *unaccelerated;
 
+   /* Temporary solution only: convert back to raw coordinates, but
+* make sure we're on the same resolution for both axes */
+   raw = tp_unnormalize_for_xaxis(tp, *unaccelerated);
+
return filter_dispatch_constant(tp->device->pointer.filter,
-   unaccelerated, tp, time);
+   , tp, time);
 }
 
 static inline void
diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index b840e72..90d3b44 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -392,13 +392,14 @@ tool_process_delta(struct libinput_tablet_tool *tool,
   const struct device_coords *delta,
   uint64_t time)
 {
-   struct normalized_coords accel;
+   const struct normalized_coords zero = { 0.0, 0.0 };
+   struct device_float_coords accel;
 
accel.x = 1.0 * delta->x;
accel.y = 1.0 * delta->y;
 
-   if (normalized_is_zero(accel))
-   return accel;
+   if (device_float_is_zero(accel))
+   return zero;
 
return filter_dispatch(device->pointer.filter,
   ,
diff --git a/src/evdev.c b/src/evdev.c
index 5a3850c..b4de199 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -419,7 +419,7 @@ fallback_flush_relative_motion(struct fallback_dispatch 
*dispatch,
if (device->pointer.filter) {
/* Apply pointer acceleration. */
accel = filter_dispatch(device->pointer.filter,
-   ,
+   ,
device,
time);
} else {
diff --git a/src/filter-private.h b/src/filter-private.h
index 637125a..6ccbf97 100644
--- a/src/filter-private.h
+++ b/src/filter-private.h
@@ -32,11 +32,11 @@ struct motion_filter_interface {
enum libinput_config_accel_profile type;
struct normalized_coords (*filter)(
   struct motion_filter *filter,
-  const struct normalized_coords *unaccelerated,
+  const struct device_float_coords *unaccelerated,
   void *data, uint64_t time);
struct normalized_coords (*filter_constant)(
   struct motion_filter *filter,
-   

[PATCH libinput 09/13] filter: work the touchpad magic slowdown into the various parameters

[Peter Hutterer]

We have everything separate from the mouse now, so having a magic slowdown
isn't needed, we can work this into our parameters. So the acceleration
function now uses everything adjusted, but the factor is still multiplied by
the slowdown in the end.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index f4baa99..7865c7e 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -135,10 +135,10 @@ filter_get_type(struct motion_filter *filter)
 #define DEFAULT_INCLINE 1.1/* unitless factor */
 
 /* Touchpad acceleration */
-#define TOUCHPAD_DEFAULT_THRESHOLD 15.75   /* mm/s */
-#define TOUCHPAD_MINIMUM_THRESHOLD 7.87/* mm/s */
+#define TOUCHPAD_DEFAULT_THRESHOLD 40  /* mm/s */
+#define TOUCHPAD_MINIMUM_THRESHOLD 20  /* mm/s */
 #define TOUCHPAD_ACCELERATION 2.0  /* unitless factor */
-#define TOUCHPAD_INCLINE 0.02794   /* unitless factor */
+#define TOUCHPAD_INCLINE 0.011 /* unitless factor */
 
 /* for the Lenovo x230 custom accel. do not touch */
 #define X230_THRESHOLD v_ms2us(0.4)/* in units/us */
@@ -749,8 +749,6 @@ touchpad_accel_profile_linear(struct motion_filter *filter,
/* Convert to mm/s because that's something one can understand */
speed_in = v_us2s(speed_in) * 25.4/accel_filter->dpi;
 
-   speed_in *= TP_MAGIC_SLOWDOWN;
-
/*
   Our acceleration function calculates a factor to accelerate input
   deltas with. The function is a double incline with a plateau,
@@ -774,10 +772,10 @@ touchpad_accel_profile_linear(struct motion_filter 
*filter,
 
   for speeds up to the lower threshold, we decelerate, down to 30%
   of input speed.
-  hence 1 = a * 2.756 + 0.3
-  0.7 = a * 2.756  => a := 0.254
+  hence 1 = a * 7 + 0.3
+  0.7 = a * 7  => a := 0.1
   deceleration function is thus:
-   y = 0.254x + 0.3
+   y = 0.1x + 0.3
 
  Note:
  * The minimum threshold is a result of trial-and-error and
@@ -785,9 +783,8 @@ touchpad_accel_profile_linear(struct motion_filter *filter,
  * 0.3 is chosen simply because it is above the Nyquist frequency
for subpixel motion within a pixel.
*/
-   if (speed_in < 2.756) {
-   const double incline = 0.254;
-   factor = incline * speed_in + 0.3;
+   if (speed_in < 7.0) {
+   factor = 0.1 * speed_in + 0.3;
/* up to the threshold, we keep factor 1, i.e. 1:1 movement */
} else if (speed_in < threshold) {
factor = 1;
-- 
2.9.3

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

[PATCH libinput 07/13] filter: drop the now-generic trackpoint and low-dpi filter functions

[Peter Hutterer]

The profile is what is still special about those two, the filter itself does
the same as the default filter (calculate velocity, calculate accel factor,
apply to delta).

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 80 ++--
 1 file changed, 7 insertions(+), 73 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index 5da8136..abbfb48 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -403,9 +403,9 @@ calculate_acceleration_factor(struct pointer_accelerator 
*accel,
  * motion
  */
 static struct normalized_coords
-accelerator_filter(struct motion_filter *filter,
-  const struct device_float_coords *unaccelerated,
-  void *data, uint64_t time)
+accelerator_filter_generic(struct motion_filter *filter,
+  const struct device_float_coords *unaccelerated,
+  void *data, uint64_t time)
 {
struct pointer_accelerator *accel =
(struct pointer_accelerator *) filter;
@@ -446,72 +446,6 @@ accelerator_filter_noop(struct motion_filter *filter,
return normalize_for_dpi(unaccelerated, accel->dpi);
 }
 
-/**
- * Low-dpi filter that handles events from devices with less than the
- * default dpi.
- *
- * @param filter The acceleration filter
- * @param unaccelerated The raw delta in the device's dpi
- * @param data Caller-specific data
- * @param time Current time in µs
- *
- * @return An accelerated tuple of coordinates representing normalized
- * motion
- */
-static struct normalized_coords
-accelerator_filter_low_dpi(struct motion_filter *filter,
-  const struct device_float_coords *unaccelerated,
-  void *data, uint64_t time)
-{
-   struct pointer_accelerator *accel =
-   (struct pointer_accelerator *) filter;
-   double accel_value; /* unitless factor */
-   struct normalized_coords accelerated;
-
-   /* Input is already in device-native DPI, nothing else needed */
-   accel_value = calculate_acceleration_factor(accel,
-   unaccelerated,
-   data,
-   time);
-   accelerated.x = accel_value * unaccelerated->x;
-   accelerated.y = accel_value * unaccelerated->y;
-
-   return accelerated;
-}
-
-/**
- * Custom filter that applies the trackpoint's constant acceleration, if any.
- *
- * @param filter The acceleration filter
- * @param unaccelerated The raw delta in the device's dpi
- * @param data Caller-specific data
- * @param time Current time in µs
- *
- * @return An accelerated tuple of coordinates representing normalized
- * motion
- */
-static struct normalized_coords
-accelerator_filter_trackpoint(struct motion_filter *filter,
- const struct device_float_coords *unaccelerated,
- void *data, uint64_t time)
-{
-   struct pointer_accelerator *accel =
-   (struct pointer_accelerator *) filter;
-   double accel_value; /* unitless factor */
-   struct normalized_coords accelerated;
-
-   /* Nothing special to do here, data is already in device dpi */
-   accel_value = calculate_acceleration_factor(accel,
-   unaccelerated,
-   data,
-   time);
-
-   accelerated.x = accel_value * unaccelerated->x;
-   accelerated.y = accel_value * unaccelerated->y;
-
-   return accelerated;
-}
-
 static struct normalized_coords
 accelerator_filter_x230(struct motion_filter *filter,
const struct device_float_coords *raw,
@@ -943,7 +877,7 @@ trackpoint_accel_profile(struct motion_filter *filter,
 
 struct motion_filter_interface accelerator_interface = {
.type = LIBINPUT_CONFIG_ACCEL_PROFILE_ADAPTIVE,
-   .filter = accelerator_filter,
+   .filter = accelerator_filter_generic,
.filter_constant = accelerator_filter_noop,
.restart = accelerator_restart,
.destroy = accelerator_destroy,
@@ -990,7 +924,7 @@ create_pointer_accelerator_filter_linear(int dpi)
 
 struct motion_filter_interface accelerator_interface_low_dpi = {
.type = LIBINPUT_CONFIG_ACCEL_PROFILE_ADAPTIVE,
-   .filter = accelerator_filter_low_dpi,
+   .filter = accelerator_filter_generic,
.filter_constant = accelerator_filter_noop,
.restart = accelerator_restart,
.destroy = accelerator_destroy,
@@ -1014,7 +948,7 @@ create_pointer_accelerator_filter_linear_low_dpi(int dpi)
 
 struct motion_filter_interface accelerator_interface_touchpad = {
.type = LIBINPUT_CONFIG_ACCEL_PROFILE_ADAPTIVE,
-   .filter = accelerator_filter,
+   .filter = accelerator_filter_generic,
.filter_constant = 

[PATCH 00/13] Revamp touchpad acceleration code

[Peter Hutterer]

This patchset is a cleanup and revamp of the touchpad acceleration code.
It doesn't give us perfect acceleration, but it goes from the current rather
abysimal state to one that should at least be good enough most of the time.
More tweaking will come, but meanwhile santa and whatnot.

Right now, we use the mouse acceleration for touchpads with a magic slowdown
applied. The first 10 patches separate the touchpad code from the mouse
acceleration code and switch it to use mm/s as base velocity unit (rather
than the current 1000dpi-mouse-equivalency units). 11 is the main change
that changes the acceleration pattern, mostly to start accelerating at a lot
higher finger speed (found mostly by trial and error). See that patch for
details. 12 is the doc update, 13 is a gratitous change that just seems to
make it feel better, but who knows.

Some pretty graphics and writeup
http://who-t.blogspot.com/2016/12/libinput-touchpad-pointer-acceleration.html

Branch is available here:
https://github.com/whot/libinput/tree/wip/touchpad-pointer-accel-v3
but beware, it spews debug printfs like crazy. Just reset back a few
commits if you're trying this branch.

Cheers,
  Peter

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

[PATCH libinput 08/13] filter: change touchpad accel code to use mm/s

[Peter Hutterer]

That's something human brains can map to because mapping a touchpad to
equivalent units of a 1000dpi mouse requires a lot of mental acrobatics. And
I'm getting older and my physio told me acrobatics is more something for the
youngens, possibly those on my lawn listening to terrible music, etc.

The various numbers are converted either times 25.4/1000 or times 1000/25.4,
depending on the usage. Somewhere I made a mistake or a rounding error or
something, so the acceleration curve is not exactly the same, but it's close
enough that it shouldn't matter. The difference shows up in a gnuplot of the
curve but it may not even perceivable anyway. And these values will be
overhauled soon anyway, so meh.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index abbfb48..f4baa99 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -49,6 +49,12 @@ v_us2ms(double units_per_us)
return units_per_us * 1000.0;
 }
 
+static inline double
+v_us2s(double units_per_us)
+{
+   return units_per_us * 100.0;
+}
+
 /* Convert speed/velocity from units/ms to units/us */
 static inline double
 v_ms2us(double units_per_ms)
@@ -129,10 +135,10 @@ filter_get_type(struct motion_filter *filter)
 #define DEFAULT_INCLINE 1.1/* unitless factor */
 
 /* Touchpad acceleration */
-#define TOUCHPAD_DEFAULT_THRESHOLD v_ms2us(0.4)
-#define TOUCHPAD_MINIMUM_THRESHOLD v_ms2us(0.2)
+#define TOUCHPAD_DEFAULT_THRESHOLD 15.75   /* mm/s */
+#define TOUCHPAD_MINIMUM_THRESHOLD 7.87/* mm/s */
 #define TOUCHPAD_ACCELERATION 2.0  /* unitless factor */
-#define TOUCHPAD_INCLINE 1.1   /* unitless factor */
+#define TOUCHPAD_INCLINE 0.02794   /* unitless factor */
 
 /* for the Lenovo x230 custom accel. do not touch */
 #define X230_THRESHOLD v_ms2us(0.4)/* in units/us */
@@ -740,8 +746,8 @@ touchpad_accel_profile_linear(struct motion_filter *filter,
const double incline = accel_filter->incline;
double factor; /* unitless */
 
-   /* Normalize to 1000dpi, because the rest below relies on that */
-   speed_in = speed_in * DEFAULT_MOUSE_DPI/accel_filter->dpi;
+   /* Convert to mm/s because that's something one can understand */
+   speed_in = v_us2s(speed_in) * 25.4/accel_filter->dpi;
 
speed_in *= TP_MAGIC_SLOWDOWN;
 
@@ -766,21 +772,22 @@ touchpad_accel_profile_linear(struct motion_filter 
*filter,
 a is the incline of acceleration
 b is minimum acceleration factor
 
-  for speeds up to 0.07 u/ms, we decelerate, down to 30% of input
-  speed.
-  hence 1 = a * 0.07 + 0.3
-  0.3 = a * 0.07  => a := 10
+  for speeds up to the lower threshold, we decelerate, down to 30%
+  of input speed.
+  hence 1 = a * 2.756 + 0.3
+  0.7 = a * 2.756  => a := 0.254
   deceleration function is thus:
-   y = 10x + 0.3
+   y = 0.254x + 0.3
 
  Note:
- * 0.07u/ms as threshold is a result of trial-and-error and
+ * The minimum threshold is a result of trial-and-error and
has no other intrinsic meaning.
  * 0.3 is chosen simply because it is above the Nyquist frequency
for subpixel motion within a pixel.
*/
-   if (v_us2ms(speed_in) < 0.07) {
-   factor = 10 * v_us2ms(speed_in) + 0.3;
+   if (speed_in < 2.756) {
+   const double incline = 0.254;
+   factor = incline * speed_in + 0.3;
/* up to the threshold, we keep factor 1, i.e. 1:1 movement */
} else if (speed_in < threshold) {
factor = 1;
@@ -795,7 +802,7 @@ touchpad_accel_profile_linear(struct motion_filter *filter,
hence 1 = ax' + 1
=> x' := (x - T)
 */
-   factor = incline * v_us2ms(speed_in - threshold) + 1;
+   factor = incline * (speed_in - threshold) + 1;
}
 
/* Cap at the maximum acceleration factor */
-- 
2.9.3

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

[PATCH libinput 04/13] touchpad: init the device's dpi correctly

[Peter Hutterer]

This has no real effect just yet because we don't use a touchpad's dpi
anywhere in the touchpad code. Only the acceleration code wants it but all
touchpads use the same acceleration method, and that one doesn't care about
the dpi.

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 26b65de..a5ab4c2 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -2257,6 +2257,10 @@ tp_init(struct tp_dispatch *tp,
   EV_ABS,
   ABS_MT_DISTANCE);
 
+   /* Set the dpi to that of the x axis, because that's what we normalize
+  to when needed*/
+   device->dpi = device->abs.absinfo_x->resolution * 25.4;
+
tp_init_hysteresis(tp);
 
if (!tp_init_accel(tp))
-- 
2.9.3

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

[PATCH libinput 01/13] filter: store the raw dpi value in the filter

[Peter Hutterer]

Currently unused, will be used in the future.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/filter.c b/src/filter.c
index 4b15c30..eecf4ca 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -154,6 +154,7 @@ struct pointer_accelerator {
double incline; /* incline of the function */
 
double dpi_factor;
+   int dpi;
 };
 
 struct pointer_accelerator_flat {
@@ -161,6 +162,7 @@ struct pointer_accelerator_flat {
 
double factor;
double dpi_factor;
+   int dpi;
 };
 
 struct tablet_accelerator_flat {
@@ -775,6 +777,7 @@ create_default_filter(int dpi)
filter->incline = DEFAULT_INCLINE;
 
filter->dpi_factor = dpi/(double)DEFAULT_MOUSE_DPI;
+   filter->dpi = dpi;
 
return filter;
 }
@@ -877,6 +880,7 @@ create_pointer_accelerator_filter_lenovo_x230(int dpi)
filter->incline = X230_INCLINE; /* incline of the acceleration function 
*/
 
filter->dpi_factor = 1; /* unused for this accel method */
+   filter->dpi = dpi;
 
return >base;
 }
@@ -904,6 +908,7 @@ create_pointer_accelerator_filter_trackpoint(int dpi)
filter->threshold = DEFAULT_THRESHOLD;
filter->accel = DEFAULT_ACCELERATION;
filter->incline = DEFAULT_INCLINE;
+   filter->dpi = dpi;
 
return >base;
 }
@@ -980,6 +985,7 @@ create_pointer_accelerator_filter_flat(int dpi)
 
filter->base.interface = _interface_flat;
filter->dpi_factor = dpi/(double)DEFAULT_MOUSE_DPI;
+   filter->dpi = dpi;
 
return >base;
 }
-- 
2.9.3

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

[PATCH libinput 03/13] filter: duplicate pointer accel for touchpads

[Peter Hutterer]

This duplicates the code so we can change it for touchpads without affecting
mice.

Signed-off-by: Peter Hutterer 
---
 src/filter.c | 103 ---
 1 file changed, 98 insertions(+), 5 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index 06f07af..6f06ab9 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -117,6 +117,12 @@ filter_get_type(struct motion_filter *filter)
 #define DEFAULT_ACCELERATION 2.0   /* unitless factor */
 #define DEFAULT_INCLINE 1.1/* unitless factor */
 
+/* Touchpad acceleration */
+#define TOUCHPAD_DEFAULT_THRESHOLD v_ms2us(0.4)
+#define TOUCHPAD_MINIMUM_THRESHOLD v_ms2us(0.2)
+#define TOUCHPAD_ACCELERATION 2.0  /* unitless factor */
+#define TOUCHPAD_INCLINE 1.1   /* unitless factor */
+
 /* for the Lenovo x230 custom accel. do not touch */
 #define X230_THRESHOLD v_ms2us(0.4)/* in units/us */
 #define X230_ACCELERATION 2.0  /* unitless factor */
@@ -470,6 +476,34 @@ accelerator_filter_constant_x230(struct motion_filter 
*filter,
return normalized;
 }
 
+static bool
+touchpad_accelerator_set_speed(struct motion_filter *filter,
+ double speed_adjustment)
+{
+   struct pointer_accelerator *accel_filter =
+   (struct pointer_accelerator *)filter;
+
+   assert(speed_adjustment >= -1.0 && speed_adjustment <= 1.0);
+
+   /* Note: the numbers below are nothing but trial-and-error magic,
+  don't read more into them other than "they mostly worked ok" */
+
+   /* delay when accel kicks in */
+   accel_filter->threshold = TOUCHPAD_DEFAULT_THRESHOLD -
+   v_ms2us(0.25) * speed_adjustment;
+   if (accel_filter->threshold < TOUCHPAD_MINIMUM_THRESHOLD)
+   accel_filter->threshold = TOUCHPAD_MINIMUM_THRESHOLD;
+
+   /* adjust max accel factor */
+   accel_filter->accel = TOUCHPAD_ACCELERATION + speed_adjustment * 1.5;
+
+   /* higher speed -> faster to reach max */
+   accel_filter->incline = TOUCHPAD_INCLINE + speed_adjustment * 0.75;
+
+   filter->speed_adjustment = speed_adjustment;
+   return true;
+}
+
 static struct normalized_coords
 touchpad_constant_filter(struct motion_filter *filter,
 const struct normalized_coords *unaccelerated,
@@ -662,15 +696,74 @@ pointer_accel_profile_linear(struct motion_filter *filter,
 
 double
 touchpad_accel_profile_linear(struct motion_filter *filter,
-  void *data,
-  double speed_in, /* units/us */
-  uint64_t time)
+ void *data,
+ double speed_in, /* 1000-dpi normalized */
+ uint64_t time)
 {
+   struct pointer_accelerator *accel_filter =
+   (struct pointer_accelerator *)filter;
+   const double max_accel = accel_filter->accel; /* unitless factor */
+   const double threshold = accel_filter->threshold; /* units/us */
+   const double incline = accel_filter->incline;
double factor; /* unitless */
 
speed_in *= TP_MAGIC_SLOWDOWN;
 
-   factor = pointer_accel_profile_linear(filter, data, speed_in, time);
+   /*
+  Our acceleration function calculates a factor to accelerate input
+  deltas with. The function is a double incline with a plateau,
+  with a rough shape like this:
+
+ accel
+factor
+  ^
+  |/
+  |  _/
+  | /
+  |/
+  +-> speed in
+
+  The two inclines are linear functions in the form
+  y = ax + b
+  where y is speed_out
+x is speed_in
+a is the incline of acceleration
+b is minimum acceleration factor
+
+  for speeds up to 0.07 u/ms, we decelerate, down to 30% of input
+  speed.
+  hence 1 = a * 0.07 + 0.3
+  0.3 = a * 0.07  => a := 10
+  deceleration function is thus:
+   y = 10x + 0.3
+
+ Note:
+ * 0.07u/ms as threshold is a result of trial-and-error and
+   has no other intrinsic meaning.
+ * 0.3 is chosen simply because it is above the Nyquist frequency
+   for subpixel motion within a pixel.
+   */
+   if (v_us2ms(speed_in) < 0.07) {
+   factor = 10 * v_us2ms(speed_in) + 0.3;
+   /* up to the threshold, we keep factor 1, i.e. 1:1 movement */
+   } else if (speed_in < threshold) {
+   factor = 1;
+   } else {
+   /* Acceleration function above the threshold:
+   y = ax' + b
+   where T is threshold
+ x is speed_in
+ x' is speed
+  

[PATCH libinput 05/13] Add device_float_get_direction

[Peter Hutterer]

With some upcoming changes we need this function for device float coordinates
as well.

Signed-off-by: Peter Hutterer 
---
 src/libinput-private.h | 37 ++---
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 4625880..f80efdc 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -701,28 +701,28 @@ enum directions {
 };
 
 static inline uint32_t
-normalized_get_direction(struct normalized_coords norm)
+xy_get_direction(double x, double y)
 {
uint32_t dir = UNDEFINED_DIRECTION;
int d1, d2;
double r;
 
-   if (fabs(norm.x) < 2.0 && fabs(norm.y) < 2.0) {
-   if (norm.x > 0.0 && norm.y > 0.0)
+   if (fabs(x) < 2.0 && fabs(y) < 2.0) {
+   if (x > 0.0 && y > 0.0)
dir = S | SE | E;
-   else if (norm.x > 0.0 && norm.y < 0.0)
+   else if (x > 0.0 && y < 0.0)
dir = N | NE | E;
-   else if (norm.x < 0.0 && norm.y > 0.0)
+   else if (x < 0.0 && y > 0.0)
dir = S | SW | W;
-   else if (norm.x < 0.0 && norm.y < 0.0)
+   else if (x < 0.0 && y < 0.0)
dir = N | NW | W;
-   else if (norm.x > 0.0)
+   else if (x > 0.0)
dir = NE | E | SE;
-   else if (norm.x < 0.0)
+   else if (x < 0.0)
dir = NW | W | SW;
-   else if (norm.y > 0.0)
+   else if (y > 0.0)
dir = SE | S | SW;
-   else if (norm.y < 0.0)
+   else if (y < 0.0)
dir = NE | N | NW;
} else {
/* Calculate r within the interval  [0 to 8)
@@ -731,7 +731,7 @@ normalized_get_direction(struct normalized_coords norm)
 * d_f = r / 2π  ([0 .. 1))
 * d_8 = 8 * d_f
 */
-   r = atan2(norm.y, norm.x);
+   r = atan2(y, x);
r = fmod(r + 2.5*M_PI, 2*M_PI);
r *= 4*M_1_PI;
 
@@ -745,4 +745,19 @@ normalized_get_direction(struct normalized_coords norm)
return dir;
 }
 
+static inline uint32_t
+normalized_get_direction(struct normalized_coords norm)
+{
+   return xy_get_direction(norm.x, norm.y);
+}
+
+/**
+ * Get the direction for the given set of coordinates.
+ * assumption: coordinates are normalized to one axis resolution.
+ */
+static inline uint32_t
+device_float_get_direction(struct device_float_coords coords)
+{
+   return xy_get_direction(coords.x, coords.y);
+}
 #endif /* LIBINPUT_PRIVATE_H */
-- 
2.9.3

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

Re: [RFC wayland-protocols] Color management protocol

[The Rasterman]

On Sat, 17 Dec 2016 21:16:41 +1100 Graeme Gill  said:

> Carsten Haitzler (The Rasterman) wrote:
>  > On Tue, 13 Dec 2016 17:46:25 +1100 Graeme Gill 
>  > said:
> 
>  > a display may not have a single native colorspace. it may be able to
>  > switch. embedded devices can do this as the display panel may have extra
>  > control lines for switching to a different display gammut/profile. it may
>  > be done at the gfx card output level too... so it can change on the fly.
> 
> That's not a typical situation though, but nothing special would be
> happening - a new profile may be installed by the user as well,
> in which case an application should re-render to accommodate
> the change.

the user at most should be interacting with compositor settings/tools to
configure a specific profile for a display (let's assume a fixed profile for
that screen), so a compositor tool to tell the compositor the profile changed
(pressed a button on the monitor to change it). when they alter the compositor,
then compositor can tell applications.

>  > yes. compositors right now work in display colorspace. they do no
>  > conversions. eventually they SHOULD to display correctly. to do so they
>  > need a color profile for the display.
> 
> For enhanced color management yes. But core comes first, and is necessary
> for many color critical applications, because the compositor will never
> have the color transformations they require.

they will have to have them, or then not support that colorspace at all.

>  > it may be that a window spans 8 different screens all with different
>  > profiles. then what?
> 
> As I've explained several times, what happens is that the application
> is aware of this, and transforms each region appropriately - just
> as they currently do on X11/OS X/MSWin systems.

not in wayland. not acceptable. the app will never know which windows it is on.
as i have said - could be wrapped around a sphere or a room full of bunnies
bouncing about across 8 screens.

>  > with a
>  > proper color correcting compositor it can make them all look the same.
> 
> As will a color aware application given appropriate color management support.

it can't if it doesn't know how a window or buffer is transformed or mapped to
screens.

> The code is already there to do all that in color critical application.

then they get to pick a colorspace and render to that. attahc that to the
buffer. compositor will do whatever it wants. e.g. if that buffer is on the
screen it matches it'd do no transform. maybe it does no transforms at all. and
simply hovers some indicator above the serface telling you if the surface
colorspace matches the screens colorspace or not. maybe compositor disallows
the window to be moved off the screen that matches the colorspace. that can be
configured via the compositor.

>  > if i mmove the
>  > window around the client has drawn different parts of its buffer with
>  > different colorspaces/profiles in mind and then has to keep redrawing to
>  > adjust as it moves.
> 
> Yes.

and now you just hit "unacceptable in wayland land" space. which is why i tell
you that this is not acceptable.

>  > you'll be ablew to see "trails" of incorrect coloring around the
>  > boundaries of the screens untl the client catches up.
> 
> It's damage, just like any other, and color critical users using
> color critical applications will take "trails" over wrong color
> anytime. No "trails" and wrong color = a system they can't use.

and totally unacceptable for wayland space. so a big fat no to that kind of
design.

>  > the compositor SHOULD do any color correction needed at this point.
> 
> Not at all. That's a way to do it under some circumstances yes, but
> it's not satisfactory for all.

it is up to the compositor. if the goal is to make colors visibly as uniform as
is possible given the displays that exist then it should. not all compositors
will. they may do things differently like above. maybe limit location of a
window/surface or place an indicator above a window when it is fully on the
correct screen for its color profile, and/or they may transform colorspaces.

>  > if you want
>  > PROPER color correction the compositor at a MINIMUM needs to be able to
>  > report the color profile of a screen even if it does no correcting.
> 
> Yes - exactly what I'm suggesting as core color management support.

but it doesnt have to tell you which screen it is nor tell you which screen
your window is on. it's an option of 1 of various colorspaces to be able to use
given a specific compositor.

>  > yes you may have
>  > multiple screens. i really dislike the above scenario of incorrect pixel
>  > tails because this goes against the whole philosophy of "every frame is
>  > perfect".
> 
> "Every pixel being perfect" except they are the wrong color, isn't perfect.

IF the compositor is doing the transformce of colorspaces, then you can acheive
perfect pixels.

> There are multiple ways of 

Re: [RFC wayland-protocols] Add the color-management protocol

[The Rasterman]

On Sat, 19 Nov 2016 17:29:20 +0100 Niels Ole Salscheider
 said:

> Signed-off-by: Niels Ole Salscheider 
> ---
>  Makefile.am|   1 +
>  unstable/color-management/README   |   4 +
>  .../color-management-unstable-v1.xml   | 136 
> + 3 files changed, 141 insertions(+)
>  create mode 100644 unstable/color-management/README
>  create mode 100644 unstable/color-management/color-management-unstable-v1.xml
> 
> diff --git a/Makefile.am b/Makefile.am
> index e693afa..ff435d5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -12,6 +12,7 @@ unstable_protocols
> = \
> unstable/tablet/tablet-unstable-v2.xml
> \ unstable/xdg-foreign/xdg-foreign-unstable-v1.xml\
> unstable/idle-inhibit/idle-inhibit-unstable-v1.xml\
> +
> unstable/color-management/color-management-unstable-v1.xml\
> $(NULL) 
>  stable_protocols
> = \ diff --git
> a/unstable/color-management/README b/unstable/color-management/README new
> file mode 100644 index 000..3bd3e6c
> --- /dev/null
> +++ b/unstable/color-management/README
> @@ -0,0 +1,4 @@
> +Color management protocol
> +
> +Maintainers:
> +Niels Ole Salscheider 
> diff --git a/unstable/color-management/color-management-unstable-v1.xml
> b/unstable/color-management/color-management-unstable-v1.xml new file mode
> 100644 index 000..f2bb3f6
> --- /dev/null
> +++ b/unstable/color-management/color-management-unstable-v1.xml
> @@ -0,0 +1,136 @@
> +
> +
> +
> +  
> +Copyright © 2014-2016 Niels Ole Salscheider
> +
> +Permission to use, copy, modify, distribute, and sell this
> +software and its documentation for any purpose is hereby granted
> +without fee, provided that the above copyright notice appear in
> +all copies and that both that copyright notice and this permission
> +notice appear in supporting documentation, and that the name of
> +the copyright holders not be used in advertising or publicity
> +pertaining to distribution of the software without specific,
> +written prior permission.  The copyright holders make no
> +representations about the suitability of this software for any
> +purpose.  It is provided "as is" without express or implied
> +warranty.
> +
> +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +THIS SOFTWARE.
> +  
> +
> +  
> +
> +  This interface allows attaching a color space to a wl_surface. The
> +  compositor uses this information to display the colors correctly.
> +  For this, the compositor converts any attached surfaces to the blending
> +  color space before performing the blending operations. After blending,
> +  the output surface is converted to the color space of the output
> device.
> +  This interface also provides requests for the sRGB and the blending
> color
> +  space. It further allows creation of a color space from an ICC profile.
> +  The client is informed by an event if the color space of one of the
> +  outputs changes.
> +
> +
> +
> +  
> +With this request, the color space of a wl_surface can be set.
> +The sRGB colorspace is attached to a surface before set_colorspace is
> +called for the first time.
> +  
> +  
> +  
> +
> +
> +
> +  
> +This request allows to create a zwp_colorspace_v1 object from an ICC
> +profile. The fd argument is the file descriptor to the ICC profile
> (ICC
> +V2 or V4).
> +  
> +  
> +  
> +
> +
> +
> +  
> +This request returns a zwp_colorspace_v1 object for the requested
> +output. A client can use this when it does not want its surfaces to
> be
> +color-corrected. In this case it can attach the color space of its
> main
> +output to its surfaces.
> +  
> +  
> +  
> +

this above i wonder if this shoiuld be done this way. does it not expose too
much about an output? what happens when the output changes its colorspace?
imagine it has software controls where it can chnage its mapping or hardware
buttons etc.? shouldnt we just have a list of colorspaces available (that may
change at any time so some kind of event indicating a change would be needed)?

> +
> +  
> +

Re: [RFC wayland-protocols] Color management protocol

[Graeme Gill]

Niels Ole Salscheider wrote:
> I feel like the discussion drifts off a bit. You (Graeme) obviously know much 
> more about color management than I do. But as Pekka already pointed out there 
> are a few constraints that originate in the design decisions of wayland and 
> are quite different to these of X11. We can't change these constraints but 
> have to find a solution that works well with them:

> - A normal application CANNOT control the hardware directly (it can't program 
> LUTs, for example).

Right, so it is not a normal application, it's a management application
that has the privileges to do so.

> - A normal application CANNOT alter global settings of the compositor (like 
> setting color profiles for the outputs). This can only be done by the 
> compositor or a few trusted applications.

Right - and therefore those trusted applications need to include
color management applications.

> The user will just have to use the 
> settings dialog provided with the compositor. Because of that it does not 
> matter if this is implementation dependent.

Hmm. I'm not sure what you mean by "settings dialog provided with the 
compositor".
Sounds like an application. For color management, the user often needs
specialist tools to create calibrations, profiles, and to manage the
system color configuration. That's what standardized API's are
meant to provide, a means of mixing and matching tools so that
systems don't have to be monolithic.

> - You DO NOT know which parts of a surface are shown on which screen.

So that needs to be fixed for color aware applications to be able to provide
display colorspace values.

> - We aim to be pixel-perfect.

Great. But perfect means the correct color as well. So mechanisms to
provide correct color are needed if you are to achieve perfection.

> I think these constraints mean that we must let the compositor take part in 
> the color correction, at least if more than one screen is involved.

I don't think this is either desirable, adequate, or necessary. For
some levels of color management I agree it will be desirable and
adequate, which is what my "Enhanced" extension sketched out, as
a variation of your extension. But the reality is that no compositor
can have all the required mechanisms to convert colors in the way
that demanding color critical applications may require. So it
shouldn't handicap Wayland in ways that none of the alternatives
are handicapped, but provide a mechanism for applications
to do it the way they need. It's shouldn't be hard. There are only two
things asked of the compositor :- provide the application the Surface
to Output region information it already has internally, and convey
the color values to the display(s).

> If we do 
> so, we should also be able to expect that the compositor can handle a bit 
> more 
> complicated cases (e. g. an arbitrary number of different surfaces with 
> different color profiles).

Providing source color profiles may satisfy many simple color management
requirements, but it won't satisfy all. This is only a problem if
there is no "escape" mechanism to work around such limitations.

> When I proposed this protocol my focus was on applications that may not be 
> color managed currently. I thought for example about web browsers or simple 
> image viewers where I would view (but not edit) photos.

Yes, I understand. This is an important class of color management usage.

> Your focus is obviously on professional applications. I think both use cases 
> are equally important and we should not treat one as an afterthought of the 
> other.

Right. I agree, but technically I think one builds on the other.

> I would be glad if we could come up with a solution that works for both under 
> these constraints.

I sketched out two extensions. Please critique and/or refine and/or contrast
them with other alternatives

Graeme Gill.

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

Re: [RFC wayland-protocols] Color management protocol

[Graeme Gill]

Pekka Paalanen wrote:
> On Wed, 14 Dec 2016 18:49:14 +1100
> Graeme Gill  wrote:

>> Please read my earlier posts. No (sane) compositor can implement CMM
>> capabilities to a color critical applications requirements,
>> so color management without any participation of a compositor
>> is a core requirement.

> that is a very strange requirement, and architecturally it is a big
> step backwards.

Since Wayland doesn't currently implement any color management support,
I'm not following how it can be a step backwards.

> Applications are never in direct control of the display hardware.

But applications have to be in control over the display content.
Content includes color.

> If
> they need to be, they need to be written directly on DRM/KMS, not
> Wayland.

I'm not sure why you think that, unless you are telling me
that applications can't send rasters to the display via Wayland.

> That way you truly get what you want: no compositor to mess
> with your colors, and no fighting between different apps demanding
> different display confgurations.

Messing with colors is not a requirement of the compositor. It can
leave them alone, just like it currently does. And I've
not mentioned display configurations.

> If you really have applications this
> color critical, then you'd better do exactly this or provide/audit the
> compositor yourself.

That's an extreme position to take - no other graphics system
(i.e. X11, OS X, MSWindows) has such a requirement for supporting
color managed applications.

> In every other case, there is a display server *with a compositor*
> between the application and the display. The compositor will
> necessarily copy and translate pixel values from one format to another,
> if not from color space to another.

And I'm pointing out that you cannot assume that a compositor will be capable
of translating from one colorspace to another in a way that the application
requires, so a means of allowing the application to provide color
values in display colorspace is a requirement. It's not that difficult,
the compositor just has to avoid messing with them, as well as
working as a communication point for display profile information.

> The compositor is also in control of the display hardware: color lookup
> tables, color transformation matrices, hardware overlays and direct
> scanout of client buffers at opportunity. Scanout hardware can often do
> color transformations on its own, with a method that might be unknown
> to anyone but the vendor. All of these depend on the hardware whether
> and which of them are available.

Which is why protocols for controlling the hardware should
ideally go via Wayland, so that applications who's job
is to setup that hardware can work.

> What you are proposing necessarily
> leads to moving the control of all of these into Wayland clients.

Yes - that's their job. Wayland or operating system management tools
don't have the expertise to manage such things - they don't know how
to create calibration curves or profiles etc.

> It
> may have been so with X11 where any client can change any display
> setting any time it wants, but I cannot see that happening on Wayland.

Wayland is useless unless there is a way to manage it. Which means
that it should have channels for administrative tools to configure it.

> If the pixels go through a display server, you have no choice but to
> trust the display server to do the right thing. So the real question
> is: "How do you let the compositor do the right thing?"
> Not: "How do I bypass the compositor?"

I didn't ask to bypass the compositor - you are the one assuming that.

I merely asked that core color management provide a means hat allows the
application to provide display colorspace values, and that the compositor
not mess with them.

> Here are some other points:
> 
> - Blending will only be an issue if a color-managed window has
>   non-opaque pixels. If the application cares about the end result, it
>   should not have non-opaque pixels, because not only it cannot know
>   *how* they will be blended, it also cannot know *what* they will be
>   blended with.

Yes, agreed. I pointed this out in a previous email.

> - You do not know which parts of a window are shown on which outputs.

Right. So that information needs to be communicated to the application,
just like it currently is on systems like X11, OS X and MSWin.

>   Some parts may be shown on multiple outputs at the same time.

Right, so since there is no hardware path for providing different
color managed value to each display, the best that can be done
in such hardware mirroring situations is to prioritize the color
management to the user designated display of highest priority.

>   There
>   is no provision for clients to provide a buffer separately for each
>   output (you could add that as an extension).

None is needed.

>   Therefore the same
>   buffer content must be applicable to any and all outputs in the
>   system.

This doesn't 

Re: [RFC wayland-protocols] Color management protocol

[The Rasterman]

On Sun, 18 Dec 2016 16:25:13 +0100 Niels Ole Salscheider
 said:

> I feel like the discussion drifts off a bit. You (Graeme) obviously know much 
> more about color management than I do. But as Pekka already pointed out there 
> are a few constraints that originate in the design decisions of wayland and 
> are quite different to these of X11. We can't change these constraints but 
> have to find a solution that works well with them:
> 
> - A normal application CANNOT control the hardware directly (it can't program 
> LUTs, for example).
> 
> - A normal application CANNOT alter global settings of the compositor (like 
> setting color profiles for the outputs). This can only be done by the 
> compositor or a few trusted applications. The user will just have to use the 
> settings dialog provided with the compositor. Because of that it does not 
> matter if this is implementation dependent.
> 
> - You DO NOT know which parts of a surface are shown on which screen.
> 
> - We aim to be pixel-perfect.
> 
> I think these constraints mean that we must let the compositor take part in 
> the color correction, at least if more than one screen is involved. If we do 
> so, we should also be able to expect that the compositor can handle a bit
> more complicated cases (e. g. an arbitrary number of different surfaces with 
> different color profiles).

bingo. and while we're at it we should solve our yuv colorspace issues too
(same as rgb. currently bt601, bt709 and bt2020 need supporting).

> When I proposed this protocol my focus was on applications that may not be 
> color managed currently. I thought for example about web browsers or simple 
> image viewers where I would view (but not edit) photos.
> Your focus is obviously on professional applications. I think both use cases 
> are equally important and we should not treat one as an afterthought of the 
> other.
> 
> I would be glad if we could come up with a solution that works for both under 
> these constraints.
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


-- 
- Codito, ergo sum - "I code, therefore I am" --
The Rasterman (Carsten Haitzler)ras...@rasterman.com

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

Re: [RFC wayland-protocols] Color management protocol

[James Feeney]

> as Pekka already pointed out there 
> are a few constraints that originate in the design decisions of wayland and 
> are quite different to [those] of X11. We can't change these constraints but 
> have to find a solution that works well with them: ...

I'm more of a bystander to this discussion.  It would be really nice to have a
shared working document, to follow along, showing
1) a list of Wayland design requirements and constraints for color, and
2) a complete list steps needed to process an image, from source to display,
3) distinguishing issues like color gamut from color encoding.

From there, it would be much easier for people to see where "standard interface"
lines are being drawn, to see what steps are being handled by the compositor vs
an application, to see what steps are automatic and which steps must be manually
configured, and to distinguish what *should be* happening, and to see whether
anything was "missing", in the processing chain.

It is unfortunate that we are all still "stuck in the stone age", exchanging
black and white text documents with email, without the tools needed to draw and
view nice graphics for block diagrams and such.  It is what it is.  Still, some
clever use of ordered lists can go a long way to creating an understandable
sequence with groupings.

Would someone knowledgeable be willing to draft a "Wayland Pixel Perfect Color
Processing Chain" working document and post it somewhere conspicuously?  I'm
thinking that someone already knows enough to just rattle this off, from memory?

Or, is that document already posted somewhere?
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC wayland-protocols] Color management protocol

[Niels Ole Salscheider]

I feel like the discussion drifts off a bit. You (Graeme) obviously know much 
more about color management than I do. But as Pekka already pointed out there 
are a few constraints that originate in the design decisions of wayland and 
are quite different to these of X11. We can't change these constraints but 
have to find a solution that works well with them:

- A normal application CANNOT control the hardware directly (it can't program 
LUTs, for example).

- A normal application CANNOT alter global settings of the compositor (like 
setting color profiles for the outputs). This can only be done by the 
compositor or a few trusted applications. The user will just have to use the 
settings dialog provided with the compositor. Because of that it does not 
matter if this is implementation dependent.

- You DO NOT know which parts of a surface are shown on which screen.

- We aim to be pixel-perfect.

I think these constraints mean that we must let the compositor take part in 
the color correction, at least if more than one screen is involved. If we do 
so, we should also be able to expect that the compositor can handle a bit more 
complicated cases (e. g. an arbitrary number of different surfaces with 
different color profiles).

When I proposed this protocol my focus was on applications that may not be 
color managed currently. I thought for example about web browsers or simple 
image viewers where I would view (but not edit) photos.
Your focus is obviously on professional applications. I think both use cases 
are equally important and we should not treat one as an afterthought of the 
other.

I would be glad if we could come up with a solution that works for both under 
these constraints.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v2 0/5] libweston common modules preparation

[Quentin Glidic]

From: Quentin Glidic 

Here is the second take at my common modules series, or rather at the 
preparation
work.

The real feature will come in a second series, with README update, and a 
complete
design explanation.

This series is merely a split and reordering of the first two patches frome the
old series, with two modifications, as asked by reviewers:
- dropped the old "module_init" entirely
- maitained "modules=xwayland.so" compatibility

Thanks,


Quentin Glidic (5):
  libweston: Properly namespace backends entrypoint
  libweston: Properly namespace modules entrypoint
  weston: Properly namespace modules entrypoint
  weston: Make the shell entrypoint specific
  weston: Add a specific option to load XWayland

 compositor/cms-colord.c |  5 +--
 compositor/cms-static.c |  4 +--
 compositor/main.c   | 66 +
 compositor/screen-share.c   |  4 +--
 compositor/systemd-notify.c |  5 +--
 compositor/weston.h | 16 -
 desktop-shell/shell.c   |  4 +--
 fullscreen-shell/fullscreen-shell.c |  5 +--
 ivi-shell/ivi-layout.c  |  4 ++-
 ivi-shell/ivi-shell.c   |  4 +--
 libweston/compositor-drm.c  |  4 +--
 libweston/compositor-fbdev.c|  4 +--
 libweston/compositor-headless.c |  4 +--
 libweston/compositor-rdp.c  |  4 +--
 libweston/compositor-wayland.c  |  4 +--
 libweston/compositor-x11.c  |  4 +--
 libweston/compositor.c  | 10 +++---
 libweston/compositor.h  |  7 ++--
 man/weston.ini.man  |  7 ++--
 man/weston.man  |  7 ++--
 tests/plugin-registry-test.c|  4 ++-
 tests/surface-global-test.c |  4 ++-
 tests/surface-screenshot.c  |  5 +--
 tests/surface-test.c|  4 ++-
 tests/weston-test.c |  4 +--
 tests/weston-tests-env  |  7 ++--
 weston.ini.in   |  3 +-
 xwayland/launcher.c |  3 +-
 28 files changed, 137 insertions(+), 69 deletions(-)

-- 
2.10.0

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

[PATCH weston v2 1/5] libweston: Properly namespace backends entrypoint

[Quentin Glidic]

From: Quentin Glidic 

This prevents loading a backend as a simple module. This will avoid
messing up with backends when we will introduce libweston common
modules.

Signed-off-by: Quentin Glidic 
---
 libweston/compositor-drm.c  | 4 ++--
 libweston/compositor-fbdev.c| 4 ++--
 libweston/compositor-headless.c | 4 ++--
 libweston/compositor-rdp.c  | 4 ++--
 libweston/compositor-wayland.c  | 4 ++--
 libweston/compositor-x11.c  | 4 ++--
 libweston/compositor.c  | 2 +-
 libweston/compositor.h  | 4 ++--
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 70514ea..529662f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3300,8 +3300,8 @@ config_init_to_defaults(struct weston_drm_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct drm_backend *b;
struct weston_drm_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
index 0c45e98..44f0cf5 100644
--- a/libweston/compositor-fbdev.c
+++ b/libweston/compositor-fbdev.c
@@ -782,8 +782,8 @@ config_init_to_defaults(struct weston_fbdev_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct fbdev_backend *b;
struct weston_fbdev_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
index e7fc397..a1aec6d 100644
--- a/libweston/compositor-headless.c
+++ b/libweston/compositor-headless.c
@@ -316,8 +316,8 @@ config_init_to_defaults(struct 
weston_headless_backend_config *config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct headless_backend *b;
struct weston_headless_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
index 223382c..4674612 100644
--- a/libweston/compositor-rdp.c
+++ b/libweston/compositor-rdp.c
@@ -1370,8 +1370,8 @@ config_init_to_defaults(struct weston_rdp_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct rdp_backend *b;
struct weston_rdp_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index d1e387d..b53b170 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -2557,8 +2557,8 @@ config_init_to_defaults(struct 
weston_wayland_backend_config *config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct wayland_backend *b;
struct wayland_parent_output *poutput;
diff --git a/libweston/compositor-x11.c b/libweston/compositor-x11.c
index 34ef854..92033be 100644
--- a/libweston/compositor-x11.c
+++ b/libweston/compositor-x11.c
@@ -1761,8 +1761,8 @@ config_init_to_defaults(struct weston_x11_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct x11_backend *b;
struct weston_x11_backend_config config = {{ 0, }};
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 508c2a9..6226810 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -5347,7 +5347,7 @@ weston_compositor_load_backend(struct weston_compositor 
*compositor,
if (backend >= ARRAY_LENGTH(backend_map))
return -1;
 
-   backend_init = weston_load_module(backend_map[backend], "backend_init");
+   backend_init = weston_load_module(backend_map[backend], 
"weston_backend_init");
if (!backend_init)
return -1;
 
diff --git a/libweston/compositor.h b/libweston/compositor.h
index ce3d9ab..faeea3a 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1751,8 

[PATCH weston v2 5/5] weston: Add a specific option to load XWayland

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 compositor/main.c  | 22 +-
 man/weston.ini.man |  7 +--
 man/weston.man |  7 +--
 tests/weston-tests-env |  7 ---
 weston.ini.in  |  3 ++-
 5 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index 74b404b..f9614f5 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -845,7 +845,7 @@ wet_load_shell(struct weston_compositor *compositor,
 
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
-int *argc, char *argv[])
+int *argc, char *argv[], int32_t *xwayland)
 {
const char *p, *end;
char buffer[256];
@@ -859,8 +859,10 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
 
if (strstr(buffer, "xwayland.so")) {
-   if (wet_load_xwayland(ec) < 0)
-   return -1;
+   weston_log("Old Xwayland module loading detected:"
+  "Please use --xwayland command line option"
+  "or weston.ini xwayland=true\n");
+   *xwayland = 1;
} else {
if (wet_load_module(ec, buffer, argc, argv) < 0)
return -1;
@@ -1756,6 +1758,7 @@ int main(int argc, char *argv[])
int i, fd;
char *backend = NULL;
char *shell = NULL;
+   int32_t xwayland = 0;
char *modules = NULL;
char *option_modules = NULL;
char *log = NULL;
@@ -1780,6 +1783,7 @@ int main(int argc, char *argv[])
{ WESTON_OPTION_STRING, "shell", 0,  },
{ WESTON_OPTION_STRING, "socket", 'S', _name },
{ WESTON_OPTION_INTEGER, "idle-time", 'i', _time },
+   { WESTON_OPTION_BOOLEAN, "xwayland", 0,  },
{ WESTON_OPTION_STRING, "modules", 0, _modules },
{ WESTON_OPTION_STRING, "log", 0,  },
{ WESTON_OPTION_BOOLEAN, "help", 'h',  },
@@ -1914,12 +1918,20 @@ int main(int argc, char *argv[])
goto out;
 
weston_config_section_get_string(section, "modules", , "");
-   if (load_modules(ec, modules, , argv) < 0)
+   if (load_modules(ec, modules, , argv, ) < 0)
goto out;
 
-   if (load_modules(ec, option_modules, , argv) < 0)
+   if (load_modules(ec, option_modules, , argv, ) < 0)
goto out;
 
+   if (!xwayland)
+   weston_config_section_get_bool(section, "xwayland", ,
+  false);
+   if (xwayland) {
+   if (wet_load_xwayland(ec) < 0)
+   goto out;
+   }
+
section = weston_config_get_section(config, "keyboard", NULL, NULL);
weston_config_section_get_bool(section, "numlock-on", _on, 0);
if (numlock_on) {
diff --git a/man/weston.ini.man b/man/weston.ini.man
index 2eac098..429dcdd 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -106,14 +106,17 @@ directory are:
 .fi
 .RE
 .TP 7
-.BI "modules=" xwayland.so,cms-colord.so
+.BI "xwayland=" true
+ask Weston to load the XWayland module (boolean).
+.RE
+.TP 7
+.BI "modules=" cms-colord.so,screen-share.so
 specifies the modules to load (string). Available modules in the
 .IR "__weston_modules_dir__"
 directory are:
 .PP
 .RS 10
 .nf
-.BR xwayland.so
 .BR cms-colord.so
 .BR screen-share.so
 .fi
diff --git a/man/weston.man b/man/weston.man
index 0c3e8dc..face229 100644
--- a/man/weston.man
+++ b/man/weston.man
@@ -83,7 +83,7 @@ the X server. XWayland provides backwards compatibility to X 
applications in a
 Wayland stack.
 
 XWayland is activated by instructing
-.BR weston " to load " xwayland.so " module, see " EXAMPLES .
+.BR weston " to load the XWayland module, see " EXAMPLES .
 Weston starts listening on a new X display socket, and exports it in the
 environment variable
 .BR DISPLAY .
@@ -143,6 +143,9 @@ Append log messages to the file
 .I file.log
 instead of writing them to stderr.
 .TP
+\fB\-\-xwayland\fR
+Ask Weston to load the XWayland module.
+.TP
 \fB\-\-modules\fR=\fImodule1.so,module2.so\fR
 Load the comma-separated list of modules. Only used by the test
 suite. The file is searched for in
@@ -326,7 +329,7 @@ http://wayland.freedesktop.org/
 .IP "Launch Weston with the DRM backend on a VT"
 weston-launch
 .IP "Launch Weston with the DRM backend and XWayland support"
-weston-launch -- --modules=xwayland.so
+weston-launch -- --xwayland
 .IP "Launch Weston (wayland-1) nested in another Weston instance (wayland-0)"
 WAYLAND_DISPLAY=wayland-0 weston -Swayland-1
 .IP "From an X terminal, launch Weston with the x11 backend"
diff --git a/tests/weston-tests-env b/tests/weston-tests-env

[PATCH weston v2 4/5] weston: Make the shell entrypoint specific

[Quentin Glidic]

From: Quentin Glidic 

This avoids loading a shell as a module, so we are sure to have only one
shell loaded at a time.

Signed-off-by: Quentin Glidic 
---
 compositor/main.c   | 17 -
 compositor/weston.h |  3 +++
 desktop-shell/shell.c   |  4 ++--
 fullscreen-shell/fullscreen-shell.c |  4 ++--
 ivi-shell/ivi-shell.c   |  4 ++--
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index af093f1..74b404b 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -828,6 +828,21 @@ wet_load_module(struct weston_compositor *compositor,
return 0;
 }
 
+static int
+wet_load_shell(struct weston_compositor *compositor,
+  const char *name, int *argc, char *argv[])
+{
+   int (*shell_init)(struct weston_compositor *ec,
+ int *argc, char *argv[]);
+
+   shell_init = wet_load_module_entrypoint(name, "wet_shell_init");
+   if (!shell_init)
+   return -1;
+   if (shell_init(compositor, argc, argv) < 0)
+   return -1;
+   return 0;
+}
+
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
 int *argc, char *argv[])
@@ -1895,7 +1910,7 @@ int main(int argc, char *argv[])
weston_config_section_get_string(section, "shell", ,
 "desktop-shell.so");
 
-   if (load_modules(ec, shell, , argv) < 0)
+   if (wet_load_shell(ec, shell, , argv) < 0)
goto out;
 
weston_config_section_get_string(section, "modules", , "");
diff --git a/compositor/weston.h b/compositor/weston.h
index 6229b34..5708aca 100644
--- a/compositor/weston.h
+++ b/compositor/weston.h
@@ -64,6 +64,9 @@ void *
 wet_load_module_entrypoint(const char *name, const char *entrypoint);
 
 int
+wet_shell_init(struct weston_compositor *ec,
+  int *argc, char *argv[]);
+int
 wet_module_init(struct weston_compositor *ec,
int *argc, char *argv[]);
 int
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 47f7e16..a5e32bd 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4875,8 +4875,8 @@ handle_seat_created(struct wl_listener *listener, void 
*data)
 }
 
 WL_EXPORT int
-wet_module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *ec,
+  int *argc, char *argv[])
 {
struct weston_seat *seat;
struct desktop_shell *shell;
diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index fcecc8d..2037f1c 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -897,8 +897,8 @@ bind_fullscreen_shell(struct wl_client *client, void *data, 
uint32_t version,
 }
 
 WL_EXPORT int
-wet_module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *compositor,
+  int *argc, char *argv[])
 {
struct fullscreen_shell *shell;
struct weston_seat *seat;
diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
index 6095ff7..efaa889 100644
--- a/ivi-shell/ivi-shell.c
+++ b/ivi-shell/ivi-shell.c
@@ -492,8 +492,8 @@ shell_add_bindings(struct weston_compositor *compositor,
  * Initialization of ivi-shell.
  */
 WL_EXPORT int
-wet_module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *compositor,
+  int *argc, char *argv[])
 {
struct ivi_shell *shell;
struct ivi_shell_setting setting = { };
-- 
2.10.0

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

[PATCH weston v2 3/5] weston: Properly namespace modules entrypoint

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 compositor/cms-colord.c |  4 ++--
 compositor/cms-static.c |  4 ++--
 compositor/main.c   | 27 +++
 compositor/screen-share.c   |  4 ++--
 compositor/systemd-notify.c |  4 ++--
 compositor/weston.h |  9 -
 desktop-shell/shell.c   |  4 ++--
 fullscreen-shell/fullscreen-shell.c |  4 ++--
 ivi-shell/ivi-layout.c  |  4 +++-
 ivi-shell/ivi-shell.c   |  4 ++--
 tests/plugin-registry-test.c|  3 ++-
 tests/surface-global-test.c |  3 ++-
 tests/surface-screenshot.c  |  4 ++--
 tests/surface-test.c|  3 ++-
 tests/weston-test.c |  4 ++--
 15 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
index ae3ef25..0daa2a7 100644
--- a/compositor/cms-colord.c
+++ b/compositor/cms-colord.c
@@ -496,8 +496,8 @@ colord_cms_output_destroy(gpointer data)
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *ec,
+   int *argc, char *argv[])
 {
gboolean ret;
GError *error = NULL;
diff --git a/compositor/cms-static.c b/compositor/cms-static.c
index a6bbfd4..e24501b 100644
--- a/compositor/cms-static.c
+++ b/compositor/cms-static.c
@@ -91,8 +91,8 @@ cms_notifier_destroy(struct wl_listener *listener, void *data)
 
 
 WL_EXPORT int
-module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *ec,
+   int *argc, char *argv[])
 {
struct cms_static *cms;
struct weston_output *output;
diff --git a/compositor/main.c b/compositor/main.c
index 2aa4936..af093f1 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -761,7 +761,7 @@ weston_create_listening_socket(struct wl_display *display, 
const char *socket_na
 }
 
 WL_EXPORT void *
-wet_load_module(const char *name, const char *entrypoint)
+wet_load_module_entrypoint(const char *name, const char *entrypoint)
 {
const char *builddir = getenv("WESTON_BUILD_DIR");
char path[PATH_MAX];
@@ -812,14 +812,28 @@ wet_load_module(const char *name, const char *entrypoint)
return init;
 }
 
+
+WL_EXPORT int
+wet_load_module(struct weston_compositor *compositor,
+   const char *name, int *argc, char *argv[])
+{
+   int (*module_init)(struct weston_compositor *ec,
+  int *argc, char *argv[]);
+
+   module_init = wet_load_module_entrypoint(name, "wet_module_init");
+   if (!module_init)
+   return -1;
+   if (module_init(compositor, argc, argv) < 0)
+   return -1;
+   return 0;
+}
+
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
 int *argc, char *argv[])
 {
const char *p, *end;
char buffer[256];
-   int (*module_init)(struct weston_compositor *ec,
-  int *argc, char *argv[]);
 
if (modules == NULL)
return 0;
@@ -833,16 +847,13 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
if (wet_load_xwayland(ec) < 0)
return -1;
} else {
-   module_init = wet_load_module(buffer, "module_init");
-   if (!module_init)
-   return -1;
-   if (module_init(ec, argc, argv) < 0)
+   if (wet_load_module(ec, buffer, argc, argv) < 0)
return -1;
}
+
p = end;
while (*p == ',')
p++;
-
}
 
return 0;
diff --git a/compositor/screen-share.c b/compositor/screen-share.c
index 0db0203..bcb9def 100644
--- a/compositor/screen-share.c
+++ b/compositor/screen-share.c
@@ -1106,8 +1106,8 @@ share_output_binding(struct weston_keyboard *keyboard, 
uint32_t time, uint32_t k
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[])
 {
struct screen_share *ss;
struct weston_config *config = wet_get_config(compositor);
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index ce18ede..50f03cb 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -115,8 +115,8 @@ weston_compositor_destroy_listener(struct wl_listener 
*listener, void *data)
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[])
 {
char 

[PATCH weston v2 2/5] libweston: Properly namespace modules entrypoint

[Quentin Glidic]

From: Quentin Glidic 

Use different functions so we cannot load a libweston common module in
weston directly or the other way around.

Signed-off-by: Quentin Glidic 
---
 compositor/cms-colord.c | 1 +
 compositor/systemd-notify.c | 1 +
 compositor/weston.h | 4 
 fullscreen-shell/fullscreen-shell.c | 1 +
 libweston/compositor.c  | 8 +++-
 libweston/compositor.h  | 3 +--
 tests/plugin-registry-test.c| 1 +
 tests/surface-global-test.c | 1 +
 tests/surface-screenshot.c  | 1 +
 tests/surface-test.c| 1 +
 xwayland/launcher.c | 3 +--
 11 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
index 152a734..ae3ef25 100644
--- a/compositor/cms-colord.c
+++ b/compositor/cms-colord.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "weston.h"
 #include "cms-helper.h"
 #include "shared/helpers.h"
 
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index 49e51f4..ce18ede 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -34,6 +34,7 @@
 #include "shared/string-helpers.h"
 #include "shared/zalloc.h"
 #include "compositor.h"
+#include "weston.h"
 
 struct systemd_notifier {
int watchdog_time;
diff --git a/compositor/weston.h b/compositor/weston.h
index bb04002..2e0417c 100644
--- a/compositor/weston.h
+++ b/compositor/weston.h
@@ -63,6 +63,10 @@ wet_get_config(struct weston_compositor *compositor);
 void *
 wet_load_module(const char *name, const char *entrypoint);
 
+int
+module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[]);
+
 int
 wet_load_xwayland(struct weston_compositor *comp);
 
diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index b3083d8..dab429d 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "fullscreen-shell-unstable-v1-server-protocol.h"
 #include "shared/helpers.h"
 
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 6226810..d00a25a 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -5357,14 +5357,12 @@ weston_compositor_load_backend(struct weston_compositor 
*compositor,
 WL_EXPORT int
 weston_compositor_load_xwayland(struct weston_compositor *compositor)
 {
-   int (*module_init)(struct weston_compositor *ec,
-  int *argc, char *argv[]);
-   int argc = 0;
+   int (*module_init)(struct weston_compositor *ec);
 
-   module_init = weston_load_module("xwayland.so", "module_init");
+   module_init = weston_load_module("xwayland.so", "weston_module_init");
if (!module_init)
return -1;
-   if (module_init(compositor, , NULL) < 0)
+   if (module_init(compositor) < 0)
return -1;
return 0;
 }
diff --git a/libweston/compositor.h b/libweston/compositor.h
index faeea3a..d59e622 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1754,8 +1754,7 @@ int
 weston_backend_init(struct weston_compositor *c,
struct weston_backend_config *config_base);
 int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[]);
+weston_module_init(struct weston_compositor *compositor);
 
 void
 weston_transformed_coord(int width, int height,
diff --git a/tests/plugin-registry-test.c b/tests/plugin-registry-test.c
index 7fc88f3..2a32e01 100644
--- a/tests/plugin-registry-test.c
+++ b/tests/plugin-registry-test.c
@@ -28,6 +28,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "plugin-registry.h"
 
 static void
diff --git a/tests/surface-global-test.c b/tests/surface-global-test.c
index 55899c6..20d99ce 100644
--- a/tests/surface-global-test.c
+++ b/tests/surface-global-test.c
@@ -29,6 +29,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 
 static void
 surface_to_from_global(void *data)
diff --git a/tests/surface-screenshot.c b/tests/surface-screenshot.c
index 332b5e3..6ff2bfc 100644
--- a/tests/surface-screenshot.c
+++ b/tests/surface-screenshot.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "file-util.h"
 
 static char *
diff --git a/tests/surface-test.c b/tests/surface-test.c
index 243f8dc..4463061 100644
--- a/tests/surface-test.c
+++ b/tests/surface-test.c
@@ -29,6 +29,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 
 static void
 surface_transform(void *data)
diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index 276795a..0ecdb20 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -351,8 +351,7 @@ const struct weston_xwayland_api api = {
 extern const struct