[PATCH v2 1/2] shell & compositor: add parameters for set_fullsceen

[Juan Zhao]

Thanks very much for your review.

On Tue, 2012-01-10 at 13:41 +0200, Pekka Paalanen wrote:
> > +     struct wl_list *tlist=(struct wl_list *)ec->surface_list.next;
> > +     int count=0;
> > +
> > +     while((es->type != WESTON_SURFACE_TYPE_GENERAL) && count
> <1024){
> > +             tlist=tlist->next;
> > +             if((tlist == (struct wl_list *)ec->surface_list.next)
> &&
> > +                count!=0)
> > +                     break;
> > +             switch(es->type){
> > +                     case WESTON_SURFACE_TYPE_CURSOR:
> > +                             cursorsurface = es;
> > +                             break;
> > +                     case WESTON_SURFACE_TYPE_PANEL:
> > +                             panelsurface = es;
> > +                             break;
> > +                     default:
> > +                             fprintf(stderr,"unkown surface type!
> \n");
> > +                             return;
> > +             }
> > +             es = container_of(tlist->next, struct weston_surface,
> link);
> > +             count++;
> > +     }
> > +     if(count == 1024) {
> > +             fprintf(stderr,"too many surfaces!\n");
> > +             return -1;
> > +     }
> 
> What is this supposed to do?
add the surface type for cursor/panel/general is to clarify the surface
stack:
-|--cursor
-|--panel
-|--general
-|--general

so that, we can figure out the window surfaces and handle different
request for them.

to differentiate
1.
-|--cursor
-|--panel
-|--general(fullscreen)
-|--general
and
2.
-|--cursor
-|--panel
-|--general
-|--general(fullscreen)

We only hope to only change mode in case 1. So we need to find out the
first general surface. If we only use (es->fs_output=output), we will
seldom go to the fullscreen handling path, because most of the time, the
first surface is that "cursor" type.

> Why is there an arbitrary limit on 'count'?
Just hope to limit the supported surface number, 1024 is just some
number for reference, we can add some macro to define it. Because, in
some case, too many surfaces may cause system too busy to work. 
But if you think it is unnecessary, we can remove this part.
> Are you forgetting there can be several cursors?
Yeah, good concern. We should consider this. Will modify the code.
> What is PANEL doing here?
For the tip fullscreen surface, we will draw only that surface and the
cursor part. Now it is only useful to figure out the general part and
cursor part. And no further usage.
> 
> Maybe all this just stems from the fact that a single list of rendered
> surfaces is not sufficient, and we should use a more appropriate data
> structures supporting layers?
> 

> >  
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 031b7d4..9907abb 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -32,6 +32,15 @@
> >  #include <EGL/egl.h>
> >  #include <EGL/eglext.h>
> >  
> > +#define WESTON_SURFACE_FULLSCREEN_NONE 0
> > +#define WESTON_SURFACE_FULLSCREEN_SCALE 1
> > +#define WESTON_SURFACE_FULLSCREEN_FORCE 2
> > +#define WESTON_SURFACE_FULLSCREEN_FILL 3
> > +#define WESTON_SURFACE_TYPE_CURSOR 0
> > +#define WESTON_SURFACE_TYPE_PANEL 1
> > +#define WESTON_SURFACE_TYPE_GENERAL 2
> 
> Why is PANEL here? Panel as a concept should not exist here, it is a
> shell thing.
We need some type to figure out that is not the general one. For current
usage, it is panel.

And for other parts, agree, will modify the codes.


Thanks a lot,
Juan