[compiz] [PATCH] Resize improvements (Multiple resize modes, better aspect ratio constraining)

[David Reveman]

On Mon, 2007-04-23 at 17:54 +0200, Danny Baumann wrote:
> Hi,
> 
> > 0001-Added-options-for-additional-resize-modes.patch - I think I'll just
> > leave this patch out and add these options once we converted the resize
> > plugin to use the new metadata system.
> 
> Yes, that's ok as it has the same effect ;-)

I've done this. The description for this option mentions that it's the
default resize mode because I think it makes a lot of sense to add match
options that defines what resize mode to use for a specific window. This
allow the user to more dynamically choose which resize mode should be
used. It might also make sense to add specific actions for each resize
mode so the user can set up shortcuts that will always initiate a
specific resize mode.

> 
> > 0002-Added-painting-code-for-additional-resize-modes.patch - I think we
> > want the paintWindow function to paint it's own instance of the window
> > like switcher and scale plugins are doing instead of transforming the
> > core instance. The outline drawing is OK.
> 
> No big deal, will change that (although I still think that for things
> like resizing/minimization we should go with modifying the core instance
> because I don't see why we should add a second fake window instance -
> but we discussed this earlier ;-) ).
> 
> > 0003-Update-resize-logic-to-reflect-additional-resize-mod.patch - I see
> > a lot of calls to damageScreen in this patch when damageScreenRegion
> > should be used in all those cases to avoid redrawing more than we need.
> > Motion events shouldn't cause the window to be move when resizing in
> > stretch-mode and rightEdge, bottomEdge variables can be removed once
> > that's changed.
> 
> So you mean the window should just be translated in stretch mode? That
> makes a lot of sense, right. You're also right about the damageScreen
> calls (I don't think the difference will be huge as these calls are
> executed very rarely, but I get your point).

Let's add one resize mode at a time, starting with the outline mode and
what's necessary to make that work properly. We'll look at the stretch
mode once that's done. Can you provide a patch against head that only
adds the outline mode?

> 
> > 0004-Added-proper-constraining-code.patch - I hope we can avoid
> > including all this code and instead improve the core constraint function
> > to solve any problems that currently exist. Can you provide some details
> > on how the constraining is not currently working properly in the resize
> > plugin so we can discuss how to best solve that?
> 
> The main point the current constraining code is lacking are the
> following:
> 
> 1) ability to apply only a subset of all the constraints (e.g. only
> applying minimum/maximum size constraints) - no big deal with passing a
> bit mask to constrainNewWindowSize

ok, let's fix that. btw, why do we need to only apply a subset of the
constraints?

> 
> 2) abilitity to resize windows with aspect ratio hint set to other
> directions than the lower right 
> The code in the patch (which was taken from Metacity) takes the resize
> direction into account in order to compute the best possible window size
> depending on the resize direction. The problem that I see here is: how
> to pass the direction of resizing to constrainNewWindowSize properly?
> Perhaps we could pass it using an enum and assume some default behaviour
> when there is no clear direction (e.g. when called from
> addWindowSizeChanges).

Sounds good to me.

> 
> > 0005-Warp-pointer-if-resizing-hit-constraints-to-avoid-mo.patch - I had
> > something similar to this in the resize plugin before but removed it as
> > it can't be done properly, it will always look bad as the cursor can
> > never be constrained perfectly. I'd like to avoid this completely.
> 
> Ok.
> 
> > 0006-Added-screen-damages-which-were-missing-if-the-resiz.patch - Again,
> > damageScreen should never have to be called. You want to use
> > damageScreenRegion.
> 
> Right, will update this.
> 
> > 0007-Avoid-resizing-windows-to-negative-sizes.patch - The constrain size
> > code should be fixed to take care of this if it doesn't already.
> 
> It does, but there is some weird effect when only one coordinate is
> negative. In my testing, the size (-1|700) became something like (20|
> 20). I will have a look into this if there is a more appropriate
> solution than that patch.

ok, good.

> 
> > 0008-Avoid-window-flashing-back-to-its-old-size-for-a-sho.patch - Does
> > this have to be a special case? Can't we have the final size change
> > always be the indication that the resizing is done (not only for stretch
> > mode)?
> 
> I'm not sure if I completely got your question, but the problem is as
> follows: 
> -  resize done 
> -> configureXWindow is called 
> -> server information is updated 
> -> ... (server processing) 
> -> (drawing during that time using the not updated w->attrib
> coordinates) 
> -> server processing finished, resizeWindow is called, w->attrib is
> updated

Yes, the resize should finish once server and client processing is done
and the window can be rendered at the new size. Not doing this for
stretched resize mode will look awful but there's also no reason for not
doing this for other resize modes. So all I'm saying is that we
shouldn't make the stretched mode a special case.

- David