Created attachment 234964 [details] [review]
window: Fix get_icon_geometry() annotation

gjs has had support for (out) parameters for quite some time now ...

Created attachment 234965 [details] [review]
window: Cache _NET_WM_ICON_GEOMETRY

Rather than doing a servcer round trip each time when retrieving the
icon geometry, use the existing property mechanism to cache it.

Created attachment 234966 [details] [review]
window: Add set_icon_geometry() method

Using a public method for setting the (cached) icon geometry rather
than accessing the struct members directly allows setting the icon
geometry from extensions.

Created attachment 234967 [details] [review]
windowManager: Respect icon geometry when minimizing

When using a dock or window-list with the shell, it makes sense for
us to minimize to the location requested rather than the activities
button.

Review of attachment 234964 [details] [review]:

indeed

Review of attachment 234965 [details] [review]:

Looks fine. "servcer" typo in the commit message

Review of attachment 234966 [details] [review]:

Yup. Not only from extensions even.

(In reply to comment #7)
> Yup. Not only from extensions even.

Mmmh, you think we'll end up using it in core? Anything else needs to go the XChangeProperty route ...

Review of attachment 234967 [details] [review]:

Do we want to make it scale to the icon width/height instead of 0x0?

(In reply to comment #8)
> (In reply to comment #7)
> > Yup. Not only from extensions even.
> 
> Mmmh, you think we'll end up using it in core? Anything else needs to go the
> XChangeProperty route ...

Which core? I'm just saying that this makes the method generally accessible from instrospection.

Created attachment 234982 [details] [review]
windowManager: Respect icon geometry when minimizing

(In reply to comment #9)
> Do we want to make it scale to the icon width/height instead of 0x0?

Oh, that makes sense to me

(In reply to comment #10)
> Which core?

Oh sorry, I use "core" to refer to gnome-shell (without extensions) ...

Review of attachment 234982 [details] [review]:

::: js/ui/windowManager.js
@@ +244,2 @@
         actor.set_scale(1.0, 1.0);
         actor.move_anchor_point_from_gravity(Clutter.Gravity.CENTER);

Not sure why we had this here but it is messing with the math below. Removing it makes the positioning accurate and doesn't seem to have any bad side-effects. (besides, it's using deprecated clutter API)

(In reply to comment #13)
> Review of attachment 234982 [details] [review]:
> 
actor.move_anchor_point_from_gravity(Clutter.Gravity.CENTER);
> 
> Not sure why we had this here but it is messing with the math below.

I'll try look into it, but off-hand the animation looks more correct to me with the patch in its current form (at least when ignoring that the window-list should probably use a different geometry)

Review of attachment 234982 [details] [review]:

Seems to me that the animation doesn't really end at the retrieved x,y but instead at x+width/2,y+height/2 . But if it looks good, go for it.

Comment on attachment 234982 [details] [review]
windowManager: Respect icon geometry when minimizing

Nevermind, I'm doing something inexplicably weird in the window-list ...

Attachment 234964 [details] pushed as c388ccf - window: Fix get_icon_geometry() annotation
Attachment 234965 [details] pushed as 30bdadb - window: Cache _NET_WM_ICON_GEOMETRY
Attachment 234966 [details] pushed as 4d9d66d - window: Add set_icon_geometry() method

Attachment 234982 [details] pushed as 89a49ce - windowManager: Respect icon geometry when minimizing

Pushed with the call to move_anchor_point removed as originally suggested, sorry for the confusion ...