Animation part 6 extra: geometry trigger

[yshui]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 31.67260% with 192 lines in your changes missing coverage. Please review.

Project coverage is 53.92%. Comparing base (3df599c) to head (34ecca5).

Files	Patch %	Lines
src/transition/generated/script_templates.c	6.66%	84 Missing ⚠️
src/wm/win.c	51.80%	40 Missing ⚠️
src/renderer/command_builder.c	41.37%	34 Missing ⚠️
src/dbus.c	0.00%	26 Missing ⚠️
src/renderer/renderer.c	0.00%	3 Missing ⚠️
src/picom.c	60.00%	2 Missing ⚠️
src/backend/backend.c	66.66%	1 Missing ⚠️
src/inspect.c	0.00%	1 Missing ⚠️
src/renderer/damage.c	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1310      +/-   ##
==========================================
- Coverage   54.42%   53.92%   -0.50%     
==========================================
  Files          70       70              
  Lines       14902    15135     +233     
==========================================
+ Hits         8110     8162      +52     
- Misses       6792     6973     +181     

Files	Coverage Δ
src/config.h	30.00% <ø> (ø)
src/config_libconfig.c	57.52% <100.00%> (ø)
src/renderer/layout.c	98.64% <100.00%> (+0.03%)	⬆️
src/wm/win.h	93.05% <100.00%> (+0.19%)	⬆️
src/backend/backend.c	48.97% <66.66%> (+0.55%)	⬆️
src/inspect.c	0.00% <0.00%> (ø)
src/renderer/damage.c	89.71% <50.00%> (-0.46%)	⬇️
src/picom.c	64.34% <60.00%> (-0.08%)	⬇️
src/renderer/renderer.c	59.52% <0.00%> (-0.54%)	⬇️
src/dbus.c	21.56% <0.00%> (-0.83%)	⬇️
... and 3 more

... and 1 file with indirect coverage changes

[yshui]

I believe this has everything now, even cross-fading between window contents.

[colonelpanic8]

hey, I believe there is still an issue with how the window-before properties are being populated.

Basically, as far as I can tell, we end up losing the context of where the previous position was, so the properties end up simply matching the current window always.

I actually have a very gross thing (that definitely is not something that can be merged) that does fix the issue here:

https://github.com/colonelpanic8/picom/blob/35fc591aca4de2721d784a9c3498498dd6f85adb/src/transition/script.h#L38

[colonelpanic8]

but yea, basically, I think we need to calculate the win_script_context once (when the animation is created) and continue to use that instead of recalculating it as the animation passes.

[colonelpanic8]

I'm happy to contribute a proper fix if you agree that this is the right way to do it.

Maybe there is a better place to store this pre calculated win_ctx (than https://github.com/colonelpanic8/picom/blob/35fc591aca4de2721d784a9c3498498dd6f85adb/src/transition/script.h#L38)?

[colonelpanic8]

may not be helpful but this is the animation I was using to test:

animations = ({
    triggers = ["geometry"];
    v-timing = {
	    curve = "cubic-bezier(0.17, 0.67, 0.68, 1.03)";
        end = 0;
        duration = 1.0;
        start = 1;
    };
    initial-x-offset = "window-x-before - window-x";
    initial-y-offset = "window-y-before - window-y";
    offset-x = "initial-x-offset * v-timing";
	offset-y = "initial-y-offset * v-timing";
} );

[yshui]

@colonelpanic8 window-*-before is never intended to be used this way. the intention has always been for them to reflect the current state of the window once the animation is started. for example, you cannot use them like this: initial-x-offset = "window-x-before - window-x";

[yshui]

but yeah, I should've made that clear in the documentation.

[colonelpanic8]

@yshui ...okay but I mean there is a need to do something like this if you want to animate a window from where it previously was, right?

[colonelpanic8]

I don't see what the purpose of window-x-before could even possibly be if it is not this.

[colonelpanic8]

also, I think you may be misunderstanding...

in the documentation it says:

  _c_, _window-y-before_, _window-width-before_, _window-height-before_:: The size and coordinates of the window before the animation is triggered. This is only meaningfully different from the normal window geometry variables for the _geometry_ trigger.

it explicitly says BEFORE the animation is triggered.

If this isn't how they're supposed to be used, can you actually describe a situation where window-x-before != window-x?

[yshui]

@colonelpanic8

it explicitly says BEFORE the animation is triggered.

It's a poor choice of words, I have updated it.

if you want to animate a window from where it previously was, right?

yes, so you use them in start and/or end values of a timing function. you can find examples in the presets:

picom/data/animation_presets.conf

Lines 166 to 204 in 0585e31

	geometry-change = {
	scale-x = {
	curve = "cubic-bezier(0.07, 0.65, 0, 1)";
	duration = "placeholder0";
	start = "window-width-before / window-width";
	end = 1;
	};
	scale-y = {
	curve = "cubic-bezier(0.07, 0.65, 0, 1)";
	duration = "placeholder0";
	start = "window-height-before / window-height";
	end = 1;
	};
	shadow-scale-x = "scale-x";
	shadow-scale-y = "scale-y";
	offset-x = {
	curve = "cubic-bezier(0.07, 0.65, 0, 1)";
	duration = "placeholder0";
	start = "window-x-before - window-x";
	end = 0;
	};
	offset-y = {
	curve = "cubic-bezier(0.07, 0.65, 0, 1)";
	duration = "placeholder0";
	start = "window-y-before - window-y";
	end = 0;
	};
	saved-image-blend = {
	duration = "placeholder0";
	start = 1;
	end = 0;
	};
	shadow-offset-x = "offset-x";
	shadow-offset-y = "offset-y";
	*knobs = {
	duration = 0.4;
	};
	*placeholders = ((0, "duration"));
	};

[colonelpanic8]

look at the example that I gave. Its just much easier to write things like that and its pretty surprising that its not possible.

Also, its not hard to just support allowing the definition of constants like this that you may not actually want to be part of a timing curve.

at the very least, if you dont want to allow this usage, it should then be an error to use them in defining a variable.

[yshui]

allowing this does not enable any new functionality. and i think the updated documentation should have made this behavior clear (let me know if you think it can still be improved).

this style of writing animations:

offset-x = "initial-x-offset * v-timing";

makes it difficult to smoothly interrupt an ongoing animations and start a different one. picom automatically adjusts offset and scale so the window doesn't "jump". if you use a generic variable v-timing, picom won't know how to deal with that.

for that reason, i don't think that's something picom should support.

on the other hand i also don't see a reason to prohibit it outright.

[yshui]

makes it difficult to smoothly interrupt an on going animations

to expand on that, if offset-x, offset-y, etc. are all separate timing functions, picom can override their start values so animations are smooth. but picom can't do that if they are expressions.