From f592cabcd778cedd33a82225c2b2357b1beb1bfd Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 13 Oct 2017 11:35:42 +1000 Subject: [PATCH] Replace the resolutions min/max with an explicit list of dpi values Previously, we could get a min/max value but there was no guarantee that it any value in between would actually work on the device. Some devices have a discrete number of resolutions (e.g. 500, 1000, 2000, 2500), most others have at least a minimum step size (so 1234 isn't a valid dpi value). Change the min/max to a discrete list of which resolutions are supported. This allows a UI to adjust its interface dynamically - e.g. a combobox if there are only 3 resolutions or a slider if there are a million of them. This is a DBus API break Signed-off-by: Peter Hutterer --- doc/dbus.rst | 26 ++++++-------- ratbagd/ratbagd-resolution.c | 69 +++++++++++++++++------------------- src/driver-hidpp10.c | 2 +- src/driver-hidpp20.c | 9 +++-- src/driver-logitech-g300.c | 6 ++-- src/driver-test.c | 4 ++- src/libratbag-private.h | 34 +++++++++++++++--- src/libratbag.c | 19 +++++----- src/libratbag.h | 36 +++++++++---------- test/test-device.c | 16 ++++++--- tools/ratbag-command.c | 7 ++-- tools/ratbagd.py | 11 ++---- 12 files changed, 132 insertions(+), 107 deletions(-) diff --git a/doc/dbus.rst b/doc/dbus.rst index ed3cdb0f..8250429d 100644 --- a/doc/dbus.rst +++ b/doc/dbus.rst @@ -264,12 +264,20 @@ org.freedesktop.ratbag1.Resolution uint for the x and y resolution assigned to this entry, respectively. The value for the resolution must be equal to - or between the :js:attr:`Minimum` and :js:attr:`Maximum` of - this resolution. + one of the values in :js:attr:`Resolutions`. If the resolution does not support separate x/y resolutions, x and y must be the same value. + .. js:attribute:: Resolutions + + :type: au + :flags: read-only, constant + + A list of permitted resolutions. Values in this list may be used in + the :js:attr:`Resolution` property. This list is always sorted + ascending, the lowest resolution is the first item in the list. + .. js:attribute:: ReportRate :type: u @@ -281,20 +289,6 @@ org.freedesktop.ratbag1.Resolution capability, changing the report rate on one resolution will change the report rate on all resolutions. - .. js:attribute:: Maximum - - :type: u - :flags: read-only, constant - - uint for the maximum resolution - - .. js:attribute:: Minimum - - :type: u - :flags: read-only, constant - - uint for the minimum possible resolution - .. js:function:: SetDefault() → () Set this resolution to be the default diff --git a/ratbagd/ratbagd-resolution.c b/ratbagd/ratbagd-resolution.c index 11cb9dc7..a0959ba4 100644 --- a/ratbagd/ratbagd-resolution.c +++ b/ratbagd/ratbagd-resolution.c @@ -190,6 +190,38 @@ ratbagd_resolution_is_default(sd_bus *bus, return sd_bus_message_append(reply, "b", is_default); } +static int +ratbagd_resolution_get_resolutions(sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) +{ + struct ratbagd_resolution *resolution = userdata; + struct ratbag_resolution *lib_resolution = resolution->lib_resolution; + unsigned int dpis[300]; + unsigned int ndpis = ARRAY_LENGTH(dpis); + int r; + + r = sd_bus_message_open_container(reply, 'a', "u"); + if (r < 0) + return r; + + ndpis = ratbag_resolution_get_dpi_list(lib_resolution, + dpis, ndpis); + assert(ndpis <= ARRAY_LENGTH(dpis)); + + for (unsigned int i = 0; i < ndpis; i++) { + r = sd_bus_message_append(reply, "u", dpis[i]); + if (r < 0) + return r; + } + + return sd_bus_message_close_container(reply); +} + static int ratbagd_resolution_get_resolution(sd_bus *bus, const char *path, @@ -301,40 +333,6 @@ ratbagd_resolution_set_report_rate(sd_bus *bus, return 0; } -static int -ratbagd_resolution_get_minimum(sd_bus *bus, - const char *path, - const char *interface, - const char *property, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error) -{ - struct ratbagd_resolution *resolution = userdata; - struct ratbag_resolution *lib_resolution = resolution->lib_resolution; - int min; - - min = ratbag_resolution_get_dpi_minimum(lib_resolution); - return sd_bus_message_append(reply, "u", min); -} - -static int -ratbagd_resolution_get_maximum(sd_bus *bus, - const char *path, - const char *interface, - const char *property, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error) -{ - struct ratbagd_resolution *resolution = userdata; - struct ratbag_resolution *lib_resolution = resolution->lib_resolution; - int max; - - max = ratbag_resolution_get_dpi_maximum(lib_resolution); - return sd_bus_message_append(reply, "u", max); -} - const sd_bus_vtable ratbagd_resolution_vtable[] = { SD_BUS_VTABLE_START(0), SD_BUS_PROPERTY("Index", "u", NULL, offsetof(struct ratbagd_resolution, index), SD_BUS_VTABLE_PROPERTY_CONST), @@ -345,12 +343,11 @@ const sd_bus_vtable ratbagd_resolution_vtable[] = { ratbagd_resolution_get_resolution, ratbagd_resolution_set_resolution, 0, SD_BUS_VTABLE_UNPRIVILEGED|SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("Resolutions", "au", ratbagd_resolution_get_resolutions, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_WRITABLE_PROPERTY("ReportRate", "u", ratbagd_resolution_get_report_rate, ratbagd_resolution_set_report_rate, 0, SD_BUS_VTABLE_UNPRIVILEGED|SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("Maximum", "u", ratbagd_resolution_get_maximum, 0, SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("Minimum", "u", ratbagd_resolution_get_minimum, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_METHOD("SetActive", "", "u", ratbagd_resolution_set_active, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("SetDefault", "", "u", ratbagd_resolution_set_default, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_VTABLE_END, diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index 0ae6af83..4f1da8f0 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -437,7 +437,7 @@ hidpp10drv_read_profile(struct ratbag_profile *profile) min = hidpp10_dpi_table_get_min_dpi(hidpp10); max = hidpp10_dpi_table_get_max_dpi(hidpp10); - ratbag_resolution_set_range(res, min, max); + ratbag_resolution_set_dpi_list_from_range(res, min, max); } ratbag_profile_for_each_button(profile, button) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 9814f772..bf800aa4 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -667,7 +667,9 @@ hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) /* FIXME: retrieve the refresh rate */ ratbag_resolution_set_resolution(res, sensor->dpi, sensor->dpi, 0); - ratbag_resolution_set_range(res, sensor->dpi_min, sensor->dpi_max); + ratbag_resolution_set_dpi_list_from_range(res, + sensor->dpi_min, + sensor->dpi_max); /* FIXME: we mark all resolutions as active because * they are from different sensors */ @@ -874,7 +876,10 @@ hidpp20drv_read_profile_8100(struct ratbag_profile *profile) res->is_active = true; } - ratbag_resolution_set_range(res, sensor->dpi_min, sensor->dpi_max); + ratbag_resolution_set_dpi_list_from_range(res, + sensor->dpi_min, + sensor->dpi_max); + } } diff --git a/src/driver-logitech-g300.c b/src/driver-logitech-g300.c index 0c9d2f25..57bc9a45 100644 --- a/src/driver-logitech-g300.c +++ b/src/driver-logitech-g300.c @@ -393,9 +393,9 @@ logitech_g300_read_profile(struct ratbag_profile *profile) resolution->is_default = res->is_default; resolution->is_active = res->is_default; - ratbag_resolution_set_range(resolution, - LOGITECH_G300_DPI_MIN, - LOGITECH_G300_DPI_MAX); + ratbag_resolution_set_dpi_list_from_range(resolution, + LOGITECH_G300_DPI_MIN, + LOGITECH_G300_DPI_MAX); } ratbag_profile_for_each_button(profile, button) diff --git a/src/driver-test.c b/src/driver-test.c index 3277a848..759e7fe7 100644 --- a/src/driver-test.c +++ b/src/driver-test.c @@ -193,7 +193,9 @@ test_read_profile(struct ratbag_profile *profile) assert(res); ratbag_resolution_set_resolution(res, r->xres, r->yres, r->hz); if (r0->dpi_min != 0 && r0->dpi_max != 0) - ratbag_resolution_set_range(res, r0->dpi_min, r0->dpi_max); + ratbag_resolution_set_dpi_list_from_range(res, + r0->dpi_min, + r0->dpi_max); res->is_active = r->active; if (r->active) active_set = true; diff --git a/src/libratbag-private.h b/src/libratbag-private.h index 0801bd4d..c6b9fc64 100644 --- a/src/libratbag-private.h +++ b/src/libratbag-private.h @@ -254,8 +254,10 @@ struct ratbag_resolution { void *userdata; struct list link; unsigned index; - unsigned int dpi_max; /**< max resolution in dpi */ - unsigned int dpi_min; /**< min resolution in dpi */ + + unsigned int dpis[300]; + size_t ndpis; + unsigned int dpi_x; /**< x resolution in dpi */ unsigned int dpi_y; /**< y resolution in dpi */ unsigned int hz; /**< report rate in Hz */ @@ -473,10 +475,32 @@ ratbag_resolution_set_resolution(struct ratbag_resolution *res, } static inline void -ratbag_resolution_set_range(struct ratbag_resolution *res, int min, int max) +ratbag_resolution_set_dpi_list_from_range(struct ratbag_resolution *res, + unsigned int min, unsigned int max) { - res->dpi_min = min; - res->dpi_max = max; + const unsigned int stepsize = 50; + bool maxed_out = false; + + res->ndpis = 0; + + /* FIXME: this should use an exponential approach, because who cares + * about 8200 vs 8250 dpi. */ + while (res->ndpis < ARRAY_LENGTH(res->dpis)) { + unsigned int dpi = min + res->ndpis * stepsize; + + if (dpi > (unsigned)max) { + maxed_out = true; + break; + } + + res->dpis[res->ndpis] = dpi; + res->ndpis++; + } + + if (!maxed_out) + log_bug_libratbag(res->profile->device->ratbag, + "%s: resolution range exceeds available space.\n", + res->profile->device->name); } static inline void diff --git a/src/libratbag.c b/src/libratbag.c index 0064150d..a9d22c1e 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -1122,16 +1122,19 @@ ratbag_resolution_get_dpi(struct ratbag_resolution *resolution) return resolution->dpi_x; } -LIBRATBAG_EXPORT int -ratbag_resolution_get_dpi_maximum(struct ratbag_resolution *resolution) +LIBRATBAG_EXPORT size_t +ratbag_resolution_get_dpi_list(struct ratbag_resolution *resolution, + unsigned int *resolutions, + size_t nres) { - return resolution->dpi_max; -} + _Static_assert(sizeof(*resolutions) == sizeof(*resolution->dpis), "type mismatch"); -LIBRATBAG_EXPORT int -ratbag_resolution_get_dpi_minimum(struct ratbag_resolution *resolution) -{ - return resolution->dpi_min; + assert(nres > 0); + + memcpy(resolutions, resolution->dpis, + sizeof(unsigned int) * min(nres, resolution->ndpis)); + + return resolution->ndpis; } LIBRATBAG_EXPORT int diff --git a/src/libratbag.h b/src/libratbag.h index 2258bc10..992e5f47 100644 --- a/src/libratbag.h +++ b/src/libratbag.h @@ -843,28 +843,26 @@ ratbag_resolution_get_dpi(struct ratbag_resolution *resolution); /** * @ingroup resolution * - * Get the maximum allowed resolution in DPI for the resolution mode. + * Get the number of resolutions in DPI available for this resolution. + * The list of DPI values is sorted in ascending order but may be filtered + * by libratbag and does not necessarily reflect all resolutions supported + * by the physical device. * - * @param resolution A previously initialized ratbag resolution + * This function writes at most nres values but returns the number of + * DPI values available on this resolution. In other words, if it returns a + * number larger than nres, call it again with an array the size of the + * return value. * - * @return The maximum resolution in dpi - * @retval 0 The maximum resolution is unknown to libratbag + * @param[out] resolutions Set to the supported DPI values in ascending order + * @param[in] nres The number of elements in resolutions + * + * @return The number of valid items in resolutions. If the returned value + * is larger than nres, the list was truncated. */ -int -ratbag_resolution_get_dpi_maximum(struct ratbag_resolution *resolution); - -/** - * @ingroup resolution - * - * Get the minimum allowed resolution in DPI for the resolution mode. - * - * @param resolution A previously initialized ratbag resolution - * - * @return The minimum resolution in dpi - * @retval 0 The minimum resolution is unknown to libratbag - */ -int -ratbag_resolution_get_dpi_minimum(struct ratbag_resolution *resolution); +size_t +ratbag_resolution_get_dpi_list(struct ratbag_resolution *resolution, + unsigned int *resolutions, + size_t nres); /** * @ingroup resolution diff --git a/test/test-device.c b/test/test-device.c index bb944e1c..703f81a4 100644 --- a/test/test-device.c +++ b/test/test-device.c @@ -34,6 +34,7 @@ #include #include "libratbag.h" +#include "libratbag-util.h" #include "libratbag-test.h" static void @@ -342,7 +343,7 @@ START_TEST(device_resolutions) struct ratbag_resolution *res; int nprofiles, nresolutions; int i, j; - int xres, yres, rate, dpi_min, dpi_max; + int xres, yres, rate; int device_freed_count = 0; bool is_active; @@ -388,22 +389,27 @@ START_TEST(device_resolutions) ck_assert_int_eq(nresolutions, 3); for (j = 0; j < nresolutions; j++) { + unsigned int dpis[200]; + int ndpis = ARRAY_LENGTH(dpis); + res = ratbag_profile_get_resolution(p, j); xres = ratbag_resolution_get_dpi_x(res); yres = ratbag_resolution_get_dpi_y(res); rate = ratbag_resolution_get_report_rate(res); is_active = ratbag_resolution_is_active(res); - dpi_min = ratbag_resolution_get_dpi_minimum(res); - dpi_max = ratbag_resolution_get_dpi_maximum(res); + + ndpis = ratbag_resolution_get_dpi_list(res, dpis, ndpis); + ck_assert_int_lt(ndpis, ARRAY_LENGTH(dpis)); + ck_assert_int_gt(ndpis, 20); ck_assert_int_eq(xres, i * 1000 + (j + 1) * 100); ck_assert_int_eq(yres, i * 1000 + (j + 1) * 100 + 100); ck_assert_int_eq(xres, ratbag_resolution_get_dpi(res)); ck_assert_int_eq(is_active, (j == 1)); - ck_assert_int_eq(dpi_min, 50); - ck_assert_int_eq(dpi_max, 5000); + ck_assert_int_eq(dpis[0], 50); + ck_assert_int_eq(dpis[ndpis - 1], 5000); ck_assert_int_eq(rate, (i + 1) * 1000); diff --git a/tools/ratbag-command.c b/tools/ratbag-command.c index 6491b0af..4cb08403 100644 --- a/tools/ratbag-command.c +++ b/tools/ratbag-command.c @@ -433,6 +433,8 @@ ratbag_cmd_info(const struct ratbag_cmd *cmd, _cleanup_profile_ struct ratbag_profile *profile = NULL; _cleanup_resolution_ struct ratbag_resolution *res0 = NULL; int dpi, rate; + unsigned int dpis[300]; + int ndpis = ARRAY_LENGTH(dpis); profile = ratbag_device_get_profile(device, i); if (!profile) @@ -447,9 +449,8 @@ ratbag_cmd_info(const struct ratbag_cmd *cmd, printf(" Resolutions:\n"); res0 = ratbag_profile_get_resolution(profile, 0); - printf(" Range: [%d, %d]\n", - ratbag_resolution_get_dpi_minimum(res0), - ratbag_resolution_get_dpi_maximum(res0)); + ndpis = ratbag_resolution_get_dpi_list(res0, dpis, ndpis); + printf(" Range: [%d, %d]\n", dpis[0], dpis[ndpis - 1]); for (unsigned int j = 0; j < ratbag_profile_get_num_resolutions(profile); j++) { _cleanup_resolution_ struct ratbag_resolution *res = NULL; diff --git a/tools/ratbagd.py b/tools/ratbagd.py index dead3b96..9e4000a1 100644 --- a/tools/ratbagd.py +++ b/tools/ratbagd.py @@ -581,14 +581,9 @@ class RatbagdResolution(_RatbagdDBus): self._set_dbus_property("ReportRate", "u", rate) @GObject.Property - def maximum(self): - """The maximum possible resolution.""" - return self._get_dbus_property("Maximum") - - @GObject.Property - def minimum(self): - """The minimum possible resolution.""" - return self._get_dbus_property("Minimum") + def resolutions(self): + """The list of supported DPI values""" + return self._get_dbus_property("Resolutions") @GObject.Property def is_active(self):