[PATCH weston 1/2] protocol: unify wl_viewport src and dst size rules

[Pekka Paalanen]

From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Let's make the source and destination size rules consistent: neither can
have zero, {-1, -1} disables it, and other negatives are not allowed.

The sanity of allowing zero sized source rectangle as debatable. Now the
minimum becomes 1/256x1/256, and with output_scale the actual samples
may be even smaller. That should be enough.

On not allowed values, raise a protocol error. This should help catch
bugs in clients that accidentally send garbage values.

The old wl_viewport.set request remains the same, and can still produce
zero sized source rectangle.

Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
---
 protocol/scaler.xml | 17 +++++++++++------
 src/compositor.c    | 43 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/protocol/scaler.xml b/protocol/scaler.xml
index 5094616..e21ae5b 100644
--- a/protocol/scaler.xml
+++ b/protocol/scaler.xml
@@ -86,7 +86,8 @@
       dst_width, dst_height. The source (rectangle) is scaled to exactly
       this size. This overrides whatever the attached wl_buffer size is,
       unless the wl_buffer is NULL. If the wl_buffer is NULL, the surface
-      has no content and therefore no size.
+      has no content and therefore no size. Otherwise, the size is always
+      at least 1x1 in surface coordinates.
 
       If the source rectangle is set, it defines what area of the
       wl_buffer is taken as the source. If the source rectangle is set and
@@ -131,7 +132,7 @@
 
     <enum name="error">
       <entry name="bad_value" value="0"
-             summary="negative values in width or height"/>
+             summary="negative or zero values in width or height"/>
     </enum>
 
     <request name="set">
@@ -167,8 +168,10 @@
 	wl_viewport for the description, and relation to the wl_buffer
 	size.
 
-	If width and/or height are negative, the source rectangle is unset
-	instead.
+	If width is -1.0 and height is -1.0, the destination size is unset
+	instead. Any other pair of values for width and height that
+	contains zero or negative values raises the bad_value protocol
+	error.
 
 	The crop and scale state is double-buffered state, and will be
 	applied on the next wl_surface.commit.
@@ -186,8 +189,10 @@
 	wl_viewport for the description, and relation to the wl_buffer
 	size.
 
-	If width and/or height are negative or zero, the destination size
-	is unset instead.
+	If width is -1 and height is -1, the destination size is unset
+	instead. Any other pair of values for width and height that
+	contains zero or negative values raises the bad_value protocol
+	error.
 
 	The crop and scale state is double-buffered state, and will be
 	applied on the next wl_surface.commit.
diff --git a/src/compositor.c b/src/compositor.c
index a298fb8..5439a6b 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3450,16 +3450,27 @@ viewport_set_source(struct wl_client *client,
 
 	assert(surface->viewport_resource != NULL);
 
-	if (wl_fixed_to_double(src_width) < 0 ||
-	    wl_fixed_to_double(src_height) < 0) {
+	if (src_width == wl_fixed_from_int(-1) &&
+	    src_height == wl_fixed_from_int(-1)) {
+		/* unset source size */
 		surface->pending.buffer_viewport.buffer.src_width =
 			wl_fixed_from_int(-1);
-	} else {
-		surface->pending.buffer_viewport.buffer.src_x = src_x;
-		surface->pending.buffer_viewport.buffer.src_y = src_y;
-		surface->pending.buffer_viewport.buffer.src_width = src_width;
-		surface->pending.buffer_viewport.buffer.src_height = src_height;
+		return;
 	}
+
+	if (src_width <= 0 || src_height <= 0) {
+		wl_resource_post_error(resource,
+			WL_VIEWPORT_ERROR_BAD_VALUE,
+			"source size must be positive (%fx%f)",
+			wl_fixed_to_double(src_width),
+			wl_fixed_to_double(src_height));
+		return;
+	}
+
+	surface->pending.buffer_viewport.buffer.src_x = src_x;
+	surface->pending.buffer_viewport.buffer.src_y = src_y;
+	surface->pending.buffer_viewport.buffer.src_width = src_width;
+	surface->pending.buffer_viewport.buffer.src_height = src_height;
 }
 
 static void
@@ -3473,12 +3484,22 @@ viewport_set_destination(struct wl_client *client,
 
 	assert(surface->viewport_resource != NULL);
 
-	if (dst_width <= 0 || dst_height <= 0) {
+	if (dst_width == -1 && dst_height == -1) {
+		/* unset destination size */
 		surface->pending.buffer_viewport.surface.width = -1;
-	} else {
-		surface->pending.buffer_viewport.surface.width = dst_width;
-		surface->pending.buffer_viewport.surface.height = dst_height;
+		return;
+	}
+
+	if (dst_width <= 0 || dst_height <= 0) {
+		wl_resource_post_error(resource,
+			WL_VIEWPORT_ERROR_BAD_VALUE,
+			"destination size must be positive (%dx%d)",
+			dst_width, dst_height);
+		return;
 	}
+
+	surface->pending.buffer_viewport.surface.width = dst_width;
+	surface->pending.buffer_viewport.surface.height = dst_height;
 }
 
 static const struct wl_viewport_interface viewport_interface = {
-- 
1.8.3.2