[PATCH 2/2] compositor.c: restore surface->plane from destroyed plane to primary plane

[Zhang, Xiong Y]

>There is more work involved in fixing this - it's a somewhat bigger issue.  We do need to move surfaces back to the primary plane if they were on a plane owned by the output as your patch does.  But we also need to reset surface->output if it points to 
>the destroyed output.
>Probably by moving the surface back to one of the remaining outputs and calling weston_surface_update_transform(), which will recompute the current output.
Yes, there is more work to fix this issue. Actually I have been working on this issue for almost one month that moving surface on destroyed output to default output as you said. Currently I have implement this function if system remove the last outpu in output_list, but if system remove the first output, some issue still need be fixed. After I fix these issue, I will send the patch set to maillist. 
But I think we can merge this patch in 1.3 release first, at least with this patch if user remove one output, system won't occur segment fault and mouse is usable, plug in the destroyed output again, everything can be restored normally.
That's why I just send out these two patches firstly when I saw you mentioned this issue on the mail of "Wayland and Weston 1.3 releases are out"


>I'm not quite sure how we'll do that... the cursor surface should be handled in input.c, but all shell surfaces should be moved by shell.c.
>Maybe they can just listen on the output->destroy_signal and move their surfaces when that signal fires.

>compositor-drm.c is still responsible for moving all surfaces out of planes being destroyed, and it can do that just by looping through
>compositor->surface_list.  But I don't want to move them into the
Every client APP has its own cursor surface, if mouse focus isn't on this app's cursor surface, this app's cursor surface won't on cursor_layer and compositor->surface_list. If mouse focus is on destroyed output but isn't on this APP when output is removed, then mouse is moved to this app after removing output, this cursor surface will be repainted and segment fault occur. So looping through compositor->surface_list can't get this kind hiden cursor surface and plane should track all the surface belonged to it.

>primary plane.  When we move a surface to a different output, it may end out in a different overlay plane when we repaint that output.
>Moving it to the primary and then back into an overlay plane means that we generate unecessary damage.  
Yes, this generate unnecessary damage.

>I think we can change weston_surface_damage_below() to just do nothing if surface->plane is NULL.  We can then set surface->plane to NULL for those surface that points to a plane that is owned by an unplugged output.
So you mean that surface-> plane is set to NULL when surface is created and set to NULL also when its output is unplugged, then change weston_surface_damage_below() to just do nothing if surface->plane is NULL.
I think this is great to avoid unnecessary damage 


thanks
-----Original Message-----
From: Kristian Høgsberg [mailto:hoegsberg at gmail.com] 
Sent: Saturday, October 12, 2013 2:05 AM
To: Zhang, Xiong Y
Cc: wayland-devel at lists.freedesktop.org
Subject: Re: [PATCH 2/2] compositor.c: restore surface->plane from destroyed plane to primary plane

On Fri, Oct 11, 2013 at 02:43:08PM +0800, Xiong Zhang wrote:
> In drm backend, the plane pointer of cursor surface point to drm_output->cursor_plane.
> when this output is removed, drm_output->cursor_plane is destroyed, 
> but cursor_surface->plane doesn't restored to primary plane. So once 
> mouse move to this cursor_surface and system will repaint this 
> cursor_surface, segment fault will occure in
> weston_surface_damage_below() function.
> 
> plane should track all the surfaces belonged to it, when plane is 
> destroyed, restroe surface on destroyed plane to primary plane.

There is more work involved in fixing this - it's a somewhat bigger issue.  We do need to move surfaces back to the primary plane if they were on a plane owned by the output as your patch does.  But we also need to reset surface->output if it points to the destroyed output.
Probably by moving the surface back to one of the remaining outputs and calling weston_surface_update_transform(), which will recompute the current output.

I'm not quite sure how we'll do that... the cursor surface should be handled in input.c, but all shell surfaces should be moved by shell.c.
Maybe they can just listen on the output->destroy_signal and move their surfaces when that signal fires.

compositor-drm.c is still responsible for moving all surfaces out of planes being destroyed, and it can do that just by looping through
compositor->surface_list.  But I don't want to move them into the
primary plane.  When we move a surface to a different output, it may end out in a different overlay plane when we repaint that output.
Moving it to the primary and then back into an overlay plane means that we generate unecessary damage.  I think we can change
weston_surface_damage_below() to just do nothing if surface->plane is NULL.  We can then set surface->plane to NULL for those surface that points to a plane that is owned by an unplugged output.

Kristian