Merge pull request #1335 from Monsterovich/feat-split-geometry-trigger

wm: split geometry trigger into size and position

man/picom.1.adoc

@@ -509,9 +509,11 @@ animations = ({
 
 	  _decrease-opacity_:: When the opacity of a window is decreased.
 
-	  [[trigger-geometry]]_geometry_:: When the geometry of a window is changed. (EXPERIMENTAL)
+	  _size_, _position_:: When the size or position of a window is changed. If both changed, the position trigger has priority. (EXPERIMENTAL)
+
+	  [[trigger-geometry]]_geometry_:: Alias of size + position.
 +
-WARNING: The _geometry_ trigger is experimental. Using this means you accept the caveat that geometry animations will also trigger when you manually resize or move a window, like when you drag the window around with your mouse.
+WARNING: The _size_ and _position_ triggers are experimental. Using this means you accept the caveat that these animations will also trigger when you manually resize or move a window, like when you drag the window around with your mouse.
 
   _suppressions_:::
 	Which other animations should be suppressed when this animation is running. Normally, if another trigger is activated while an animation is already running, the animation in progress will be interrupted and the new animation will start. If you want to prevent this, you can set the `suppressions` option to a list of triggers that should be suppressed. This is optional, the default value for this is an empty list.

src/config.h

@@ -73,11 +73,18 @@ enum animation_trigger {
 	ANIMATION_TRIGGER_OPEN,
 	/// When a window is closed
 	ANIMATION_TRIGGER_CLOSE,
-	/// When a window's geometry changes
-	ANIMATION_TRIGGER_GEOMETRY,
+	/// When a window's size changes
+	ANIMATION_TRIGGER_SIZE,
+	/// When a window's position changes
+	ANIMATION_TRIGGER_POSITION,
 
 	ANIMATION_TRIGGER_INVALID,
 	ANIMATION_TRIGGER_COUNT = ANIMATION_TRIGGER_INVALID,
+
+	// Aliases are not included in the count
+
+	/// Alias of size + position
+	ANIMATION_TRIGGER_ALIAS_GEOMETRY,
 };
 
 static const char *animation_trigger_names[] attr_unused = {
@@ -87,7 +94,9 @@ static const char *animation_trigger_names[] attr_unused = {
     [ANIMATION_TRIGGER_DECREASE_OPACITY] = "decrease-opacity",
     [ANIMATION_TRIGGER_OPEN] = "open",
     [ANIMATION_TRIGGER_CLOSE] = "close",
-    [ANIMATION_TRIGGER_GEOMETRY] = "geometry",
+    [ANIMATION_TRIGGER_SIZE] = "size",
+    [ANIMATION_TRIGGER_POSITION] = "position",
+    [ANIMATION_TRIGGER_ALIAS_GEOMETRY] = "geometry",
 };
 
 struct script;

src/config_libconfig.c

@@ -227,8 +227,9 @@ static inline void parse_wintype_config(const config_t *cfg, const char *member_
 }
 
 enum animation_trigger parse_animation_trigger(const char *trigger) {
-	for (unsigned i = 0; i < ANIMATION_TRIGGER_COUNT; i++) {
-		if (strcasecmp(trigger, animation_trigger_names[i]) == 0) {
+	for (unsigned i = 0; i < ARR_SIZE(animation_trigger_names); i++) {
+		if (animation_trigger_names[i] != NULL &&
+		    strcasecmp(trigger, animation_trigger_names[i]) == 0) {
 			return i;
 		}
 	}
@@ -260,22 +261,54 @@ compile_win_script(struct win_script *result, config_setting_t *setting, char **
 	return true;
 }
 
-static bool
-set_animation(struct win_script *animations, const enum animation_trigger *triggers,
-              int number_of_triggers, struct win_script animation, unsigned line) {
+/// Return the index of the lowest bit set in `bitflags`, and clear that bit.
+static int next_bit(uint64_t *bitflags) {
+	if (!*bitflags) {
+		return -1;
+	}
+	auto lowbit = *bitflags & (~*bitflags + 1);
+#if defined(__has_builtin) && __has_builtin(__builtin_ctzll)
+	int ret = __builtin_ctzll(lowbit);
+#else
+	int ret = 63;
+	if (lowbit & 0x00000000FFFFFFFF) {
+		ret -= 32;
+	}
+	if (lowbit & 0x0000FFFF0000FFFF) {
+		ret -= 16;
+	}
+	if (lowbit & 0x00FF00FF00FF00FF) {
+		ret -= 8;
+	}
+	if (lowbit & 0x0F0F0F0F0F0F0F0F) {
+		ret -= 4;
+	}
+	if (lowbit & 0x3333333333333333) {
+		ret -= 2;
+	}
+	if (lowbit & 0x5555555555555555) {
+		ret -= 1;
+	}
+#endif
+	*bitflags -= lowbit;
+	return ret;
+}
+
+static bool set_animation(struct win_script *animations, uint64_t triggers,
+                          struct win_script animation, unsigned line) {
 	bool needed = false;
-	for (int i = 0; i < number_of_triggers; i++) {
-		if (triggers[i] == ANIMATION_TRIGGER_INVALID) {
-			log_error("Invalid trigger defined at line %d", line);
-			continue;
+	while (triggers != 0) {
+		int trigger = next_bit(&triggers);
+		if (trigger >= ANIMATION_TRIGGER_INVALID) {
+			break;
 		}
-		if (animations[triggers[i]].script != NULL) {
+		if (animations[trigger].script != NULL) {
 			log_error("Duplicate animation defined for trigger %s at line "
 			          "%d, it will be ignored.",
-			          animation_trigger_names[triggers[i]], line);
+			          animation_trigger_names[trigger], line);
 			continue;
 		}
-		animations[triggers[i]] = animation;
+		animations[trigger] = animation;
 		needed = true;
 	}
 	return needed;
@@ -309,18 +342,37 @@ static bool parse_animation_one(struct win_script *animations,
 		          config_setting_source_line(triggers));
 		return false;
 	}
-	enum animation_trigger *trigger_types =
-	    alloca(sizeof(enum animation_trigger[number_of_triggers]));
+	uint64_t trigger_types = 0;
 	const char *trigger0 = config_setting_get_string(triggers);
 	if (trigger0 == NULL) {
 		for (int i = 0; i < number_of_triggers; i++) {
-			auto trigger_i = config_setting_get_string_elem(triggers, i);
-			trigger_types[i] = trigger_i == NULL
-			                       ? ANIMATION_TRIGGER_INVALID
-			                       : parse_animation_trigger(trigger_i);
+			auto trigger_i_str = config_setting_get_string_elem(triggers, i);
+			auto trigger_i = trigger_i_str == NULL
+			                     ? ANIMATION_TRIGGER_INVALID
+			                     : parse_animation_trigger(trigger_i_str);
+			if (trigger_types & (1 << trigger_i)) {
+				log_warn("Duplicate trigger \"%s\" set at line %d",
+				         trigger_i_str, config_setting_source_line(triggers));
+			}
+			trigger_types |= 1 << trigger_i;
 		}
 	} else {
-		trigger_types[0] = parse_animation_trigger(trigger0);
+		trigger_types = 1 << parse_animation_trigger(trigger0);
+	}
+	if ((trigger_types & (1 << ANIMATION_TRIGGER_INVALID)) != 0) {
+		log_error("Invalid trigger defined at line %d",
+		          config_setting_source_line(triggers));
+	}
+	if ((trigger_types & (1 << ANIMATION_TRIGGER_ALIAS_GEOMETRY)) != 0) {
+		const uint64_t to_set =
+		    (1 << ANIMATION_TRIGGER_SIZE) | (1 << ANIMATION_TRIGGER_POSITION);
+		if ((trigger_types & to_set) != 0) {
+			log_warn("Trigger \"geometry\" is an alias of \"size\" and "
+			         "\"position\", but one or both of them are also set at "
+			         "line %d",
+			         config_setting_source_line(triggers));
+		}
+		trigger_types |= to_set;
 	}
 
 	// script parser shouldn't see this.
@@ -379,7 +431,7 @@ static bool parse_animation_one(struct win_script *animations,
 		return false;
 	}
 
-	bool needed = set_animation(animations, trigger_types, number_of_triggers, result,
+	bool needed = set_animation(animations, trigger_types, result,
 	                            config_setting_source_line(setting));
 	if (!needed) {
 		script_free(result.script);
@@ -434,14 +486,13 @@ void generate_fading_config(struct options *opt) {
 	// overwritten
 	scoped_charp str = NULL;
 	size_t len = 0;
-	enum animation_trigger trigger[2];
 	struct script *scripts[4];
 	unsigned number_of_scripts = 0;
-	int number_of_triggers = 0;
 
 	double duration = 1.0 / opt->fade_in_step * opt->fade_delta / 1000.0;
 	if (!safe_isinf(duration) && !safe_isnan(duration) && duration > 0) {
 		scoped_charp duration_str = NULL;
+		uint64_t triggers = 0;
 		dtostr(duration, &duration_str);
 
 		// Fading in from nothing, i.e. `open` and `show`
@@ -452,12 +503,12 @@ void generate_fading_config(struct options *opt) {
 		BUG_ON(!compile_win_script_from_string(&fade_in1, str));
 		if (opt->animations[ANIMATION_TRIGGER_OPEN].script == NULL &&
 		    !opt->no_fading_openclose) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_OPEN;
+			triggers |= 1 << ANIMATION_TRIGGER_OPEN;
 		}
 		if (opt->animations[ANIMATION_TRIGGER_SHOW].script == NULL) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_SHOW;
+			triggers |= 1 << ANIMATION_TRIGGER_SHOW;
 		}
-		if (set_animation(opt->animations, trigger, number_of_triggers, fade_in1, 0)) {
+		if (set_animation(opt->animations, triggers, fade_in1, 0)) {
 			scripts[number_of_scripts++] = fade_in1.script;
 		} else {
 			script_free(fade_in1.script);
@@ -467,11 +518,11 @@ void generate_fading_config(struct options *opt) {
 		asnprintf(&str, &len, FADING_TEMPLATE_1, duration_str);
 		struct win_script fade_in2 = {.is_generated = true};
 		BUG_ON(!compile_win_script_from_string(&fade_in2, str));
-		number_of_triggers = 0;
+		triggers = 0;
 		if (opt->animations[ANIMATION_TRIGGER_INCREASE_OPACITY].script == NULL) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_INCREASE_OPACITY;
+			triggers |= 1 << ANIMATION_TRIGGER_INCREASE_OPACITY;
 		}
-		if (set_animation(opt->animations, trigger, number_of_triggers, fade_in2, 0)) {
+		if (set_animation(opt->animations, triggers, fade_in2, 0)) {
 			scripts[number_of_scripts++] = fade_in2.script;
 		} else {
 			script_free(fade_in2.script);
@@ -484,22 +535,22 @@ void generate_fading_config(struct options *opt) {
 	duration = 1.0 / opt->fade_out_step * opt->fade_delta / 1000.0;
 	if (!safe_isinf(duration) && !safe_isnan(duration) && duration > 0) {
 		scoped_charp duration_str = NULL;
+		uint64_t triggers = 0;
 		dtostr(duration, &duration_str);
 
 		// Fading out to nothing, i.e. `hide` and `close`
 		asnprintf(&str, &len, FADING_TEMPLATE_1 FADING_TEMPLATE_2, duration_str,
 		          duration_str, 1, 0);
 		struct win_script fade_out1 = {.is_generated = true};
 		BUG_ON(!compile_win_script_from_string(&fade_out1, str));
-		number_of_triggers = 0;
 		if (opt->animations[ANIMATION_TRIGGER_CLOSE].script == NULL &&
 		    !opt->no_fading_openclose) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_CLOSE;
+			triggers |= 1 << ANIMATION_TRIGGER_CLOSE;
 		}
 		if (opt->animations[ANIMATION_TRIGGER_HIDE].script == NULL) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_HIDE;
+			triggers |= 1 << ANIMATION_TRIGGER_HIDE;
 		}
-		if (set_animation(opt->animations, trigger, number_of_triggers, fade_out1, 0)) {
+		if (set_animation(opt->animations, triggers, fade_out1, 0)) {
 			scripts[number_of_scripts++] = fade_out1.script;
 		} else {
 			script_free(fade_out1.script);
@@ -509,11 +560,11 @@ void generate_fading_config(struct options *opt) {
 		asnprintf(&str, &len, FADING_TEMPLATE_1, duration_str);
 		struct win_script fade_out2 = {.is_generated = true};
 		BUG_ON(!compile_win_script_from_string(&fade_out2, str));
-		number_of_triggers = 0;
+		triggers = 0;
 		if (opt->animations[ANIMATION_TRIGGER_DECREASE_OPACITY].script == NULL) {
-			trigger[number_of_triggers++] = ANIMATION_TRIGGER_DECREASE_OPACITY;
+			triggers = 1 << ANIMATION_TRIGGER_DECREASE_OPACITY;
 		}
-		if (set_animation(opt->animations, trigger, number_of_triggers, fade_out2, 0)) {
+		if (set_animation(opt->animations, triggers, fade_out2, 0)) {
 			scripts[number_of_scripts++] = fade_out2.script;
 		} else {
 			script_free(fade_out2.script);

src/wm/win.c

@@ -1715,7 +1715,10 @@ bool win_process_animation_and_state_change(struct session *ps, struct win *w, d
 	bool will_never_render =
 	    (!w->ever_damaged || w->win_image == NULL) && w->state != WSTATE_MAPPED;
 	auto win_ctx = win_script_context_prepare(ps, w);
-	bool geometry_changed = !win_geometry_eq(w->previous.g, w->g);
+
+	bool size_changed = win_size_changed(w->previous.g, w->g);
+	bool position_changed = win_position_changed(w->previous.g, w->g);
+
 	auto old_state = w->previous.state;
 
 	w->previous.state = w->state;
@@ -1726,7 +1729,7 @@ bool win_process_animation_and_state_change(struct session *ps, struct win *w, d
 		// This window won't be rendered, so we don't need to run the animations.
 		bool state_changed = old_state != w->state ||
 		                     win_ctx.opacity_before != win_ctx.opacity ||
-		                     geometry_changed;
+		                     size_changed || position_changed;
 		return state_changed || (w->running_animation_instance != NULL);
 	}
 
@@ -1741,7 +1744,7 @@ bool win_process_animation_and_state_change(struct session *ps, struct win *w, d
 	enum animation_trigger trigger = ANIMATION_TRIGGER_INVALID;
 
 	// Animation trigger priority:
-	//   state > geometry > opacity
+	//   state > position > size > opacity
 	if (old_state != w->state) {
 		// Send D-Bus signal
 		if (ps->o.dbus) {
@@ -1786,9 +1789,12 @@ bool win_process_animation_and_state_change(struct session *ps, struct win *w, d
 			assert(false);
 			return true;
 		}
-	} else if (geometry_changed) {
+	} else if (position_changed) {
+		assert(w->state == WSTATE_MAPPED);
+		trigger = ANIMATION_TRIGGER_POSITION;
+	} else if (size_changed) {
 		assert(w->state == WSTATE_MAPPED);
-		trigger = ANIMATION_TRIGGER_GEOMETRY;
+		trigger = ANIMATION_TRIGGER_SIZE;
 	} else if (win_ctx.opacity_before != win_ctx.opacity) {
 		assert(w->state == WSTATE_MAPPED);
 		trigger = win_ctx.opacity > win_ctx.opacity_before
@@ -1877,7 +1883,7 @@ bool win_process_animation_and_state_change(struct session *ps, struct win *w, d
 			memory[output_indices[WIN_SCRIPT_SAVED_IMAGE_BLEND]] =
 			    1 - memory[output_indices[WIN_SCRIPT_SAVED_IMAGE_BLEND]];
 		}
-		if (geometry_changed) {
+		if (size_changed || position_changed) {
 			// If the window has moved, we need to adjust scripts
 			// outputs so that the window will stay in the same position and
 			// size after applying the animation. This way the window's size

src/wm/win.h

@@ -353,10 +353,19 @@ win_options(const struct win *w) {
 	    win_maybe_options_fold(w->options_override, w->options), *w->options_default);
 }
 
-/// Check if win_geometry `a` and `b` have the same sizes and positions. Border width is
+/// Check if the window has changed in size. Border width is
 /// not considered.
-static inline bool win_geometry_eq(struct win_geometry a, struct win_geometry b) {
-	return a.x == b.x && a.y == b.y && a.width == b.width && a.height == b.height;
+static inline bool win_size_changed(struct win_geometry a, struct win_geometry b) {
+	return a.width != b.width || a.height != b.height;
+}
+
+/// Check if the window position has changed.
+static inline bool win_position_changed(struct win_geometry a, struct win_geometry b) {
+	if (win_size_changed(a, b)) {
+		return false;
+	}
+
+	return a.x != b.x || a.y != b.y;
 }
 
 /// Process pending updates/images flags on a window. Has to be called in X critical