[PATCH weston] shell: Don't draw shadows for maximized windows.

[Kristian Høgsberg]

On Wed, Aug 08, 2012 at 02:43:21PM +0300, Pekka Paalanen wrote:
> On Wed, 8 Aug 2012 04:52:41 -0600
> Scott Moreau <oreaus at gmail.com> wrote:
> 
> > >
> > > This is definitely not right. It is not the compositor's job to cut out
> > > and not show an arbitrary part of the surface. It is the client's job
> > > to not render anything it does not want to show.
> > >
> > > You also break the protocol by lying to client about the dimensions,
> > > and in doing so, you make assumptions that are not based on any
> > > specification, just like you mentioned yourself.
> > >
> > > This must be done in toytoolkit, not here.
> > >
> > >
> > > Sorry,
> > > pq
> > >
> > 
> > The problem I'm having is trying to work out how to do snapping for resize
> > or move requests on maximized surfaces. The patch that works in toytoolkit
> > would require even more dancing in shell to implement snapping. The problem
> > is that the shell doesn't know some specifics about the surface such as
> > input region and/or theme margin, in this case the shadow margin. I think
> > ideally you'd be able to request these specifics from the client. It would
> > also be nice to be able to know the titlebar height so in the snap-off case
> > we can place the surface to where the middle of the titlebar is under the
> > grab cursor. Would it make sense to be able to request certain (theme)
> > attributes such as these from the client?
> 
> Snapping must happen in the server, since the server does all about
> moving surfaces. This is completely different to shadows, which must
> remain a purely client feature.
> 
> I don't think you should need such things as "theme margin" or
> whatever. At most, you could add a snapping region, similar to input
> region, in the protocol. That would be more logical: "this region will
> be used for snapping" instead of "if I set shadow region like this, it
> usually snaps there...".
> 
> Snapping could use the input region, since it is always up-to-date
> with the latest buffer the server has. Except that seems to be a lie,
> because surface::attach resets the input region. Looks like we have
> some protocol to fix here.
> 
> The current protocol works like this, AFAIU:
> 1. wl_surface::attach - input region may get reset
> 2. wl_surface::set_input_region - input region gets properly defined
> again
> 
> This means, that between steps 1 and 2, the server will composite using
> bad data (undefined input region). No, we cannot rely on the
> set_input_region request coming right after the attach, nothing
> guarantees that it will be processed during the current frame.
> 
> This makes snapping on resize to likely fail, since the snapping code
> will be working with an undefined input region, and hence it can only
> ever snap to the surface edge, which due to the shadow will fall far
> off from the "real" window edge.
> 
> Krh, was the the idea of first sending all new surface attributes, and
> then committing those at once on wl_surface::attach rejected, or was
> this part of the protocol just not fixed yet? Or is there some other
> clever mechanism to make this atomic?

It wasn't rejected, just never implemented.  The problem is that in
practice it's not necessary, since typically the protocol buffer will
ensure atomicity.  Even if that gets flushed unexpectedly, most
clients will re-render in response to frame events or input events,
which we send out at the beginning of the frame, giving clients a
(just under) 16ms window to get things done before their requests
might get broken across two frames.

But yes, the fact that it is possible isn't really compatible with
"every frame is perfect".  Mostly I've just been afraid of
overengineering this, but maybe we can just specify that certain
requests are latched until the next surface.attach request.  As a rule
of thumb this would apply to all requests that alter state that
depends on the buffer size or contents.  This would apply to opaque
and input regions, and in fact, wl_shell_surface.set_fullscreen
already works this way.

It's also pretty easy for extensions to tie into this.  They just
document which properties are latched.  For example position of
overlaid surfaces or buffer contents rotation could be done that way.

By the way, the way it works now, we invalidate the input region if we
attach a buffer of a different size, but I'm thinking that that's very
un-wayland-ish and we should just always expect the client to attach a
new input region.

Kristian