From ef75ba26e0ebe99c3f6f6e1b8287f547c7cdaf2d Mon Sep 17 00:00:00 2001
From: Simon Ser <contact@emersion.fr>
Date: Sun, 24 Nov 2019 13:25:59 +0100
Subject: [PATCH] Don't re-use previous allocation if a layer has been removed

Start from scratch since we may be able to come up with a better allocation
(if the layer made another layer fall back to composition for instance).

Closes: https://github.com/emersion/libliftoff/issues/30
---
 alloc.c             |  6 ++++++
 include/private.h   |  1 +
 layer.c             |  2 ++
 test/meson.build    |  3 ++-
 test/test_dynamic.c | 15 ++++++++++++---
 5 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/alloc.c b/alloc.c
index c595527..ac4550f 100644
--- a/alloc.c
+++ b/alloc.c
@@ -518,6 +518,10 @@ static bool reuse_previous_alloc(struct liftoff_device *device,
 	bool compatible;
 
 	liftoff_list_for_each(output, &device->outputs, link) {
+		if (output->layers_changed) {
+			return false;
+		}
+
 		liftoff_list_for_each(layer, &output->layers, link) {
 			if (layer_needs_realloc(layer)) {
 				return false;
@@ -544,6 +548,8 @@ static void mark_layers_clean(struct liftoff_device *device)
 	struct liftoff_layer *layer;
 
 	liftoff_list_for_each(output, &device->outputs, link) {
+		output->layers_changed = false;
+
 		liftoff_list_for_each(layer, &output->layers, link) {
 			layer_mark_clean(layer);
 		}
diff --git a/include/private.h b/include/private.h
index 22e3302..0bcd154 100644
--- a/include/private.h
+++ b/include/private.h
@@ -30,6 +30,7 @@ struct liftoff_output {
 	struct liftoff_layer *composition_layer;
 
 	struct liftoff_list layers; /* liftoff_layer.link */
+	bool layers_changed; /* layer added or removed */
 };
 
 struct liftoff_layer {
diff --git a/layer.c b/layer.c
index c5f24e1..0ea465e 100644
--- a/layer.c
+++ b/layer.c
@@ -13,11 +13,13 @@ struct liftoff_layer *liftoff_layer_create(struct liftoff_output *output)
 	}
 	layer->output = output;
 	liftoff_list_insert(output->layers.prev, &layer->link);
+	output->layers_changed = true;
 	return layer;
 }
 
 void liftoff_layer_destroy(struct liftoff_layer *layer)
 {
+	layer->output->layers_changed = true;
 	if (layer->output->composition_layer == layer) {
 		layer->output->composition_layer = NULL;
 	}
diff --git a/test/meson.build b/test/meson.build
index dcd3e1f..1a9e9d2 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -44,7 +44,8 @@ tests = {
 	'dynamic': [
 		'same',
 		'fb',
-		'new-layer',
+		'add-layer',
+		'remove-layer',
 	],
 	'priority': [
 		#'basic',
diff --git a/test/test_dynamic.c b/test/test_dynamic.c
index 92a72f7..a4a1143 100644
--- a/test/test_dynamic.c
+++ b/test/test_dynamic.c
@@ -32,7 +32,7 @@ int main(int argc, char *argv[]) {
 	int drm_fd;
 	struct liftoff_device *device;
 	struct liftoff_output *output;
-	struct liftoff_layer *layer;
+	struct liftoff_layer *layer, *other_layer;
 	drmModeAtomicReq *req;
 	bool ok;
 	size_t commit_count;
@@ -56,7 +56,8 @@ int main(int argc, char *argv[]) {
 
 	output = liftoff_output_create(device, liftoff_mock_drm_crtc_id);
 	layer = add_layer(output, 0, 0, 1920, 1080);
-	/* Layer incompatible with all planes */
+	/* Layers incompatible with all planes */
+	other_layer = add_layer(output, 0, 0, 256, 256);
 	add_layer(output, 0, 0, 256, 256);
 
 	liftoff_mock_plane_add_compatible_layer(mock_plane, layer);
@@ -83,9 +84,13 @@ int main(int argc, char *argv[]) {
 	} else if (strcmp(test_name, "fb") == 0) {
 		liftoff_layer_set_property(layer, "FB_ID",
 					   liftoff_mock_drm_create_fb(layer));
-	} else if (strcmp(test_name, "new-layer") == 0) {
+	} else if (strcmp(test_name, "add-layer") == 0) {
 		add_layer(output, 0, 0, 256, 256);
 		want_reuse_prev_alloc = false;
+	} else if (strcmp(test_name, "remove-layer") == 0) {
+		liftoff_layer_destroy(other_layer);
+		other_layer = NULL;
+		want_reuse_prev_alloc = false;
 	} else {
 		fprintf(stderr, "no such test: %s\n", test_name);
 		return 1;
@@ -95,8 +100,12 @@ int main(int argc, char *argv[]) {
 	assert(ok);
 	assert(liftoff_mock_plane_get_layer(mock_plane, req) == layer);
 	if (want_reuse_prev_alloc) {
+		/* The library should perform only one TEST_ONLY commit with the
+		 * previous plane allocation. */
 		assert(liftoff_mock_commit_count == commit_count + 1);
 	} else {
+		/* Since there are at least two planes, the library should
+		 * perform more than one TEST_ONLY commit. */
 		assert(liftoff_mock_commit_count > commit_count + 1);
 	}