[PATCH v2 2/2] shell: Add set_maximised for wl_shell_surface interface.

[Pekka Paalanen]

On Wed, 11 Jan 2012 23:34:51 +0800
Juan Zhao <juan.j.zhao at linux.intel.com> wrote:

> 
> 
> On Tue, 2012-01-10 at 14:11 +0200, Pekka Paalanen wrote:
> > On Mon,  9 Jan 2012 22:54:20 +0800
> > juan.j.zhao at linux.intel.com wrote:
> > 
> > > From: Alex Wu <zhiwen.wu at linux.intel.com>
> > > 
> > > mainly send a configure event to the client
> > > 
> > > Signed-off-by: Juan Zhao <juan.j.zhao at linux.intel.com>
> > > Signed-off-by: Alex Wu <zhiwen.wu at linux.intel.com>
> > > 
> > > ---
> > >  src/shell.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 48 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/src/shell.c b/src/shell.c
> > > index d12bc1a..b93bfa3 100644
> > > --- a/src/shell.c
> > > +++ b/src/shell.c
> > > @@ -408,6 +408,53 @@ get_default_output(struct weston_compositor *compositor)
> > >  }
> > >  
> > >  static void
> > > +shell_surface_set_maximised(struct wl_client *client,
> > > +			     struct wl_resource *resource)
> > > +{
> > > +	struct shell_surface *shsurf = resource->data;
> > > +	struct weston_surface *es = shsurf->surface;
> > > +	struct weston_surface *es_temp = NULL;
> > > +	struct weston_output *output;
> > > +	uint32_t edges = 0, panel_width = 0, panel_height = 0;
> > > +	struct shell_surface *priv;
> > > +
> > > +	if (reset_shell_surface_type(shsurf))
> > > +		return;
> > > +
> > > +	/* FIXME: set maximised on first output */
> > > +	/* FIXME: Handle output going away */
> > > +	output = get_default_output(es->compositor);
> > > +	es->output = output;
> > 
> > I seem to recall that there was code in compositor.c to find the output
> > the surface is currently in. 

> I would like to do it this way:
> if(!es->output)
> {
> 	/*assign an output by weston_surface_assign_output to assign*/
> }
> Is it OK?

es->output != NULL has implications, so it should not be set without
taking those into account. See the comment in struct weston_surface
definition in compositor.h. It roughly means, that if 'output' is set,
the surface is somehow visible and live, therefore it has been mapped.

But I would not use es->output to infer mapped or not, the surface
could be mapped, but temporarily hidden (or frozen, output==NULL means
also animation is frozen). Or at least the current semantics work that
way.

struct shell_surface has an 'output' member, too. Maybe that could be
used.

I guess set maximised should only set the surface type, and the real
work is done, when buffers are attached to the surface, which is in the
weston_shell::map and configure callbacks.

Perhaps all existing shell_surface_set_*() functions should be fixed to
work that way, plus the possible configure event that needs to be sent
on surface types changes that specifically are supposed to change the
surface size, like maximised.

And now I see the chicken-and-egg problem. To send the configure event,
you need to choose the output, but for unmapped windows, you have
nothing to base that decision on. We should probably start a
new discussion on this particular question, as it is quite non-trivial.

Where does a user expect a fullscreen or maximised window to appear?

I guess you could just punt it for later, and use the first output
there is. That's what the current code does. But for already mapped
surfaces, we could do better even now, and choose the output the
surface is already in. That would be intuitive for the user, right?

> > Another question is, how do we determine
> > the output for a surface that has never been mapped before?
> > 
> > Should probably check for the non-mapped case and defer to mapping time
> > in that case.

> Yeah, good point. Should we add some interface for shell surface to
> record the map status? Or there is already one?

compositor.c uses (es->visual == WESTON_NONE_VISUAL) to determine if a
surface has never been mapped before. Maybe that should be wrapped into
a new weston_surface_is_mapped() function, and use that function
everywhere. Or, maybe we could use shell_surface::output == NULL to
determine something similar, though not quite the same condition.

I'd have to hack on it to see what would be good, and right now I'm
working on the surface transformations, so I cannot say how those should
be done.

- pq