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 <peter.hutterer@who-t.net>

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

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,

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)

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);
+
 	}
 }
 

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)

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;

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

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

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

test/test-device.c

@@ -34,6 +34,7 @@
 #include <sys/resource.h>
 
 #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);
 

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;

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):