Permalink
Browse files

rootston: xdg-shell*: Fix get_size() for newly-mapped views

The user-visible issue is that newly-mapped xdg-shell* windows would
sometimes start with their top-left-corner, rather than their center, in
the center of the screen. This is because get_size() would
conservatively fall back on (width, height) == (0, 0) if both
set_window_geometry() had not been called, and it found
view->wlr_surface to be NULL.

But, view->wlr_surface is only set to non-NULL in view_map(). We call
get_size() before this. Fortunately, the wlr_surface in question is
accessible via view->xdg_shell{,_v6}->surface, so always fall back on
that. We can assert its presence instead of further falling back on
(width, height) == (0, 0).

Signed-off-by: Genki Sky <sky@genki.is>
  • Loading branch information...
Genki Sky authored and SirCmpwn committed May 31, 2018
1 parent d1cf9ac commit 32013abae63f1c31598ac716acd7e73c24fadae1
Showing with 6 additions and 8 deletions.
  1. +3 −4 rootston/xdg_shell.c
  2. +3 −4 rootston/xdg_shell_v6.c
View
@@ -135,11 +135,10 @@ static void get_size(const struct roots_view *view, struct wlr_box *box) {
if (surface->geometry.width > 0 && surface->geometry.height > 0) {
box->width = surface->geometry.width;
box->height = surface->geometry.height;
} else if (view->wlr_surface != NULL) {
box->width = view->wlr_surface->current->width;
box->height = view->wlr_surface->current->height;
} else {
box->width = box->height = 0;
assert(surface->surface);
box->width = surface->surface->current->width;
box->height = surface->surface->current->height;
}
}
View
@@ -136,11 +136,10 @@ static void get_size(const struct roots_view *view, struct wlr_box *box) {
if (surface->geometry.width > 0 && surface->geometry.height > 0) {
box->width = surface->geometry.width;
box->height = surface->geometry.height;
} else if (view->wlr_surface != NULL) {
box->width = view->wlr_surface->current->width;
box->height = view->wlr_surface->current->height;
} else {
box->width = box->height = 0;
assert(surface->surface);
box->width = surface->surface->current->width;
box->height = surface->surface->current->height;
}
}

3 comments on commit 32013ab

@emersion

This comment has been minimized.

Show comment
Hide comment
@emersion

emersion May 31, 2018

Member

We call get_size() before this.

Since we now assert, it means we crash instead?

Member

emersion replied May 31, 2018

We call get_size() before this.

Since we now assert, it means we crash instead?

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn May 31, 2018

Member

Note that it's obtaining the surface reference through a different mechanism, which should always be set here.

Member

SirCmpwn replied May 31, 2018

Note that it's obtaining the surface reference through a different mechanism, which should always be set here.

@emersion

This comment has been minimized.

Show comment
Hide comment
@emersion

emersion May 31, 2018

Member

Then this doesn't change anything, this commit is a no-op. If the wlr_surface is unmapped, width == height == 0.

Member

emersion replied May 31, 2018

Then this doesn't change anything, this commit is a no-op. If the wlr_surface is unmapped, width == height == 0.

Please sign in to comment.