[PATCH weston v2 00/24] Initial Xwayland window positioning, with XWM reordering

[Pekka Paalanen]

On Tue, 17 Jan 2017 15:44:32 +0000
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Pekka,
> 
> On 17 January 2017 at 15:22, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > You can see the state of the current series at:
> > https://patchwork.freedesktop.org/series/17096/
> >
> > And I have a rebased branch at:
> > https://git.collabora.com/cgit/user/pq/weston.git/log/?h=xwm-reorder-WIP
> >
> > In the branch I have squashed "desktop-shell: debug
> > set_position_from_xwayland" into "shell: implement
> > set_xwayland_position".
> >
> > Daniel, I think you can skip looking at all patches already R-b
> > Quentin, except those where the commit message highlights a regression.
> > I'd like some opinions if the regressions are bad and whether we should
> > block things for the last patch.
> >
> > The last patch "xwm: use _XWAYLAND_ALLOW_COMMITS" cannot be merged
> > until the Xwayland patch adding the feature has been accepted.  
> 
> The first four ('add set_toplevel_with_position to internal API' ..
> 'implement set_xwayland_position') seem safe to push and are:
> Reviewed-by: Daniel Stone <daniels at collabora.com>

The four pushed:
   db7b9f3..77a6d95  master -> master


> 'react to geometry changes' makes me a little nervous; is there a way
> you can see that this can be resequenced or moved such that we don't
> have the intermediate regression?

I think it could be left even as the last patch in the series. In its
current point of the series it is needed to make -geometry work, but
after the _XWAYLAND_ALLOW_COMMITS support -geometry should work also
without it.


> 'split weston_wm_window_draw_decoration()', 'move fini near init in
> set_pending_state' and 'split out
> weston_wm_window_set_pending_state_OR()' are also R-b me, and seem
> like they should be safe to push even without reacting to geometry
> changes ... ?

Sure, pushed:
   77a6d95..ed56883  master -> master



> 'schedule repaint from MapRequest' looks OK for the second hunk, but
> the assert added in the first hunk makes the conditional immediately
> above it dead code. Oops.

weston_wm_window_create_frame(window) sets window->frame_id, so the
assert just reminds us that frame_id is now guaranteed to be set.

Suggestions on how to improve it?

> 'debug when weston_surface gets created' and 'explain the
> read_properties() in xserver_map_shell_surface()' are R-b me and safe
> to push.

Pushed the "debug" patch:
   ed56883..9a5fab0  master -> master

I didn't push the "explain" patch though, as it mentions MapRequest to
draw decorations, and it doesn't do that until 'schedule repaint from
MapRequest'.

> With these, we just have the difficult ones left: react to geometry
> changes, schedule repaint from MapRequest / do not draw decor twice on
> map, and _XWAYLAND_ALLOW_COMMITS. I don't have the mental bandwidth to
> deal with those today, but if you're able to gang the easier ones
> together and push them, that at least makes this series very tractable
> indeed.

Very much appreciated. After we agree on 'schedule repaint from
MapRequest' and I push it, I will send a v3 which will be just three
patches.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170118/82a9ee4e/attachment.sig>