Discussion:
[PATCH 2/2 weston] toytoolkit: Don't draw shadows for maximized windows.
(too old to reply)
Scott Moreau
2012-08-07 06:32:23 UTC
Permalink
This effectively fixes a bug where maximized windows appear to not maximize
fully bacause of the shadow margin. Instead, we now maximize the window to
the understood input region.
---
clients/window.c | 60 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 17 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 30a6167..8f5cf9b 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -181,6 +181,7 @@ struct widget {
void *user_data;
int opaque;
int tooltip_count;
+ int shadow_margin;
};

struct input {
@@ -921,6 +922,7 @@ widget_create(struct window *window, void *data)
widget->opaque = 0;
widget->tooltip = NULL;
widget->tooltip_count = 0;
+ widget->shadow_margin = window->display->theme->margin;

return widget;
}
@@ -1246,7 +1248,38 @@ frame_resize_handler(struct widget *widget,
int decoration_width, decoration_height;
int opaque_margin;

- if (widget->window->type != TYPE_FULLSCREEN) {
+ switch (widget->window->type) {
+ case TYPE_FULLSCREEN:
+ decoration_width = 0;
+ decoration_height = 0;
+
+ allocation.x = 0;
+ allocation.y = 0;
+ allocation.width = width;
+ allocation.height = height;
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 0;
+ break;
+ case TYPE_MAXIMIZED:
+ widget->shadow_margin = t->margin;
+ t->margin = 0;
+ decoration_width = t->width * 2;
+ decoration_height = t->width + t->titlebar_height;
+
+ allocation.x = t->width;
+ allocation.y = t->titlebar_height;
+ allocation.width = width - decoration_width;
+ allocation.height = height - decoration_height;
+
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 1;
+ break;
+ default:
+ t->margin = widget->shadow_margin;
decoration_width = (t->width + t->margin) * 2;
decoration_height = t->width +
t->titlebar_height + t->margin * 2;
@@ -1260,18 +1293,7 @@ frame_resize_handler(struct widget *widget,

wl_list_for_each(button, &frame->buttons_list, link)
button->widget->opaque = 1;
- } else {
- decoration_width = 0;
- decoration_height = 0;
-
- allocation.x = 0;
- allocation.y = 0;
- allocation.width = width;
- allocation.height = height;
- opaque_margin = 0;
-
- wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 0;
+ break;
}

widget_set_allocation(child, allocation.x, allocation.y,
@@ -1289,10 +1311,14 @@ frame_resize_handler(struct widget *widget,
if (widget->window->type != TYPE_FULLSCREEN) {
widget->window->input_region =
wl_compositor_create_region(display->compositor);
- wl_region_add(widget->window->input_region,
- t->margin, t->margin,
- width - 2 * t->margin,
- height - 2 * t->margin);
+ if (widget->window->type == TYPE_MAXIMIZED)
+ wl_region_add(widget->window->input_region,
+ t->margin, t->margin, width, height);
+ else
+ wl_region_add(widget->window->input_region,
+ t->margin, t->margin,
+ width - 2 * t->margin,
+ height - 2 * t->margin);
}

widget_set_allocation(widget, 0, 0, width, height);
--
1.7.11.2
Ander Conselvan de Oliveira
2012-08-07 08:57:07 UTC
Permalink
What exactly does this fix? Your commit message doesn't make it that
obvious.

Weston commit 010f98b0 added the opaque field to struct widget to "track
and report [...] opaque regions", i.e., the region of a surface whose
alpha channel is 1. It seems to me the frame button code is abusing this
field to hide the buttons in fullscreen mode. IMO, either way you define
a visibility value based on the opacity value results in improper usage.

Cheers,
Ander
---
clients/window.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clients/window.c b/clients/window.c
index d0b7a7d..30a6167 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1259,7 +1259,7 @@ frame_resize_handler(struct widget *widget,
opaque_margin = t->margin + t->frame_radius;
wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 0;
+ button->widget->opaque = 1;
} else {
decoration_width = 0;
decoration_height = 0;
@@ -1271,7 +1271,7 @@ frame_resize_handler(struct widget *widget,
opaque_margin = 0;
wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 1;
+ button->widget->opaque = 0;
}
widget_set_allocation(child, allocation.x, allocation.y,
@@ -1416,7 +1416,7 @@ frame_button_redraw_handler(struct widget *widget, void *data)
return;
if (!height)
return;
- if (widget->opaque)
+ if (!widget->opaque)
return;
cr = cairo_create(window->cairo_surface);
Scott Moreau
2012-08-07 11:03:50 UTC
Permalink
Hi Ander,

On Tue, Aug 7, 2012 at 2:57 AM, Ander Conselvan de Oliveira <
Post by Ander Conselvan de Oliveira
What exactly does this fix? Your commit message doesn't make it that
obvious.
I assumed the problem was obvious. The definition of the word opaque makes
it clear. In frame_button_redraw_handler() the code returns (i.e. does not
draw the buttons) if widget->opaque is set. It should be the other way
around.
Post by Ander Conselvan de Oliveira
Weston commit 010f98b0 added the opaque field to struct widget to "track
and report [...] opaque regions", i.e., the region of a surface whose alpha
channel is 1.
I don't think this commit is relevant here. What is relevant is the meaning
of opaque. (since cgit.fd.o is down at the moment I will post some
snippets) For widget_set_transparent() you have

widget_set_transparent(struct widget *widget, int transparent)
{
widget->opaque = !transparent;
}
Post by Ander Conselvan de Oliveira
It seems to me the frame button code is abusing this field to hide the
buttons in fullscreen mode.
This may be the case but see window.c~:1300 frame_resize_handler()


if (child->opaque) {
widget->window->opaque_region =
wl_compositor_create_region(display->compositor);
wl_region_add(widget->window->opaque_region,
opaque_margin, opaque_margin,
widget->allocation.width - 2 * opaque_margin,
widget->allocation.height - 2 * opaque_margin);
}

it adds the region if opaque is set.
Post by Ander Conselvan de Oliveira
IMO, either way you define a visibility value based on the opacity value
results in improper usage.
I did not write this code, I'm just building on top of it while trying to
improve it. If you have a better idea, I would be interested to know what
you think.



Regards,

Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120807/ad57a19d/attachment.html>
Ander Conselvan de Oliveira
2012-08-07 13:54:25 UTC
Permalink
Post by Scott Moreau
Hi Ander,
On Tue, Aug 7, 2012 at 2:57 AM, Ander Conselvan de Oliveira
What exactly does this fix? Your commit message doesn't make it that
obvious.
I assumed the problem was obvious. The definition of the word opaque
makes it clear. In frame_button_redraw_handler() the code returns (i.e.
does not draw the buttons) if widget->opaque is set. It should be the
other way around.
Weston commit 010f98b0 added the opaque field to struct widget to
"track and report [...] opaque regions", i.e., the region of a
surface whose alpha channel is 1.
I don't think this commit is relevant here. What is relevant is the
meaning of opaque.
My point exactly. The reason I pointed this commit out is because it
introduced the opaque field with the purpose of tracking the opaque
region of a window.

The protocol defines the opaque region as "the region of the surface
that contain opaque content. The opaque region is an optimization hint
for the compositor that lets it optimize out redrawing of content behind
opaque regions. Setting an opaque region is not required for correct
behaviour, but marking transparent content as opaque will result in
repaint artifacts."

So if the opaque field is 0 for a given widget it does *not* mean that
this widget is hidden or invisible. It only means that the contents
might contain pixels whose alpha component is different than one. In
other words, if a surface is opaque its contents obscure whatever is
below them while otherwise alpha blending is needed.

Later the frame_button code started to use opaque as a visibility field.

(since cgit.fd.o is down at the moment I will post
Post by Scott Moreau
some snippets) For widget_set_transparent() you have
widget_set_transparent(struct widget *widget, int transparent)
{
widget->opaque = !transparent;
}
It seems to me the frame button code is abusing this field to hide
the buttons in fullscreen mode.
This may be the case but see window.c~:1300 frame_resize_handler()
if (child->opaque) {
widget->window->opaque_region =
wl_compositor_create_region(display->compositor);
wl_region_add(widget->window->opaque_region,
opaque_margin, opaque_margin,
widget->allocation.width - 2 * opaque_margin,
widget->allocation.height - 2 * opaque_margin);
}
it adds the region if opaque is set.
IMO, either way you define a visibility value based on the opacity
value results in improper usage.
I did not write this code, I'm just building on top of it while trying
to improve it.
My point was that since opacity and visibility are not the same thing,
defining visibility as opaque == 0 or opaque == 1 is equally wrong. Your
patch doesn't change any behavior, it only changes if we go with the
former or the latter.
Post by Scott Moreau
If you have a better idea, I would be interested to know
what you think.
One simple solution is to add a visibility field to struct frame_button.
Or have a general visibility switch on every widget.


Best regards,
Ander
Ander Conselvan de Oliveira
2012-08-07 09:22:05 UTC
Permalink
Post by Scott Moreau
This effectively fixes a bug where maximized windows appear to not maximize
fully bacause of the shadow margin. Instead, we now maximize the window to
the understood input region.
---
clients/window.c | 60 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 17 deletions(-)
diff --git a/clients/window.c b/clients/window.c
index 30a6167..8f5cf9b 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -181,6 +181,7 @@ struct widget {
void *user_data;
int opaque;
int tooltip_count;
+ int shadow_margin;
};
struct input {
@@ -921,6 +922,7 @@ widget_create(struct window *window, void *data)
widget->opaque = 0;
widget->tooltip = NULL;
widget->tooltip_count = 0;
+ widget->shadow_margin = window->display->theme->margin;
return widget;
}
@@ -1246,7 +1248,38 @@ frame_resize_handler(struct widget *widget,
int decoration_width, decoration_height;
int opaque_margin;
- if (widget->window->type != TYPE_FULLSCREEN) {
+ switch (widget->window->type) {
+ decoration_width = 0;
+ decoration_height = 0;
+
+ allocation.x = 0;
+ allocation.y = 0;
+ allocation.width = width;
+ allocation.height = height;
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 0;
+ break;
+ widget->shadow_margin = t->margin;
+ t->margin = 0;
+ decoration_width = t->width * 2;
+ decoration_height = t->width + t->titlebar_height;
+
+ allocation.x = t->width;
+ allocation.y = t->titlebar_height;
+ allocation.width = width - decoration_width;
+ allocation.height = height - decoration_height;
+
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 1;
+ break;
+ t->margin = widget->shadow_margin;
You shouldn't change the theme data. With your patch applied, run
weston-terminal, press ctrl-alt-n, maximize the new terminal window and
then use mod-alt to switch back to the other terminal. The frame is not
drawn properly anymore.

Cheers,
Ander
Post by Scott Moreau
decoration_width = (t->width + t->margin) * 2;
decoration_height = t->width +
t->titlebar_height + t->margin * 2;
@@ -1260,18 +1293,7 @@ frame_resize_handler(struct widget *widget,
wl_list_for_each(button, &frame->buttons_list, link)
button->widget->opaque = 1;
- } else {
- decoration_width = 0;
- decoration_height = 0;
-
- allocation.x = 0;
- allocation.y = 0;
- allocation.width = width;
- allocation.height = height;
- opaque_margin = 0;
-
- wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 0;
+ break;
}
widget_set_allocation(child, allocation.x, allocation.y,
@@ -1289,10 +1311,14 @@ frame_resize_handler(struct widget *widget,
if (widget->window->type != TYPE_FULLSCREEN) {
widget->window->input_region =
wl_compositor_create_region(display->compositor);
- wl_region_add(widget->window->input_region,
- t->margin, t->margin,
- width - 2 * t->margin,
- height - 2 * t->margin);
+ if (widget->window->type == TYPE_MAXIMIZED)
+ wl_region_add(widget->window->input_region,
+ t->margin, t->margin, width, height);
+ else
+ wl_region_add(widget->window->input_region,
+ t->margin, t->margin,
+ width - 2 * t->margin,
+ height - 2 * t->margin);
}
widget_set_allocation(widget, 0, 0, width, height);
Scott Moreau
2012-08-07 11:06:14 UTC
Permalink
This post might be inappropriate. Click to display it.
Scott Moreau
2012-08-07 12:16:14 UTC
Permalink
This effectively fixes a bug where maximized windows appear to not maximize fully
bacause of the shadow margin. Instead, we maximize the window to the input region.
---

v3:

* Applied the same logic to both instances of theme_get_location() so the resize
cursors show properly for maximized windows
* Fixed window-manager.c flags
* Fixed a small indentation mishap
* Simplified the assignment case in theme_get_location()

---
clients/window.c | 75 +++++++++++++++++++++++++++++--------------
shared/cairo-util.c | 51 +++++++++++++++++------------
shared/cairo-util.h | 7 ++--
src/xwayland/window-manager.c | 4 +--
4 files changed, 88 insertions(+), 49 deletions(-)

diff --git a/clients/window.c b/clients/window.c
index 30a6167..05f1e85 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1244,9 +1244,37 @@ frame_resize_handler(struct widget *widget,
struct theme *t = display->theme;
int x_l, x_r, y, w, h;
int decoration_width, decoration_height;
- int opaque_margin;
+ int opaque_margin, shadow_margin;

- if (widget->window->type != TYPE_FULLSCREEN) {
+ switch (widget->window->type) {
+ case TYPE_FULLSCREEN:
+ decoration_width = 0;
+ decoration_height = 0;
+
+ allocation.x = 0;
+ allocation.y = 0;
+ allocation.width = width;
+ allocation.height = height;
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 0;
+ break;
+ case TYPE_MAXIMIZED:
+ decoration_width = t->width * 2;
+ decoration_height = t->width + t->titlebar_height;
+
+ allocation.x = t->width;
+ allocation.y = t->titlebar_height;
+ allocation.width = width - decoration_width;
+ allocation.height = height - decoration_height;
+
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 1;
+ break;
+ default:
decoration_width = (t->width + t->margin) * 2;
decoration_height = t->width +
t->titlebar_height + t->margin * 2;
@@ -1260,18 +1288,7 @@ frame_resize_handler(struct widget *widget,

wl_list_for_each(button, &frame->buttons_list, link)
button->widget->opaque = 1;
- } else {
- decoration_width = 0;
- decoration_height = 0;
-
- allocation.x = 0;
- allocation.y = 0;
- allocation.width = width;
- allocation.height = height;
- opaque_margin = 0;
-
- wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 0;
+ break;
}

widget_set_allocation(child, allocation.x, allocation.y,
@@ -1286,13 +1303,15 @@ frame_resize_handler(struct widget *widget,
width = child->allocation.width + decoration_width;
height = child->allocation.height + decoration_height;

+ shadow_margin = widget->window->type == TYPE_MAXIMIZED ? 0 : t->margin;
+
if (widget->window->type != TYPE_FULLSCREEN) {
widget->window->input_region =
wl_compositor_create_region(display->compositor);
wl_region_add(widget->window->input_region,
- t->margin, t->margin,
- width - 2 * t->margin,
- height - 2 * t->margin);
+ shadow_margin, shadow_margin,
+ width - 2 * shadow_margin,
+ height - 2 * shadow_margin);
}

widget_set_allocation(widget, 0, 0, width, height);
@@ -1307,9 +1326,9 @@ frame_resize_handler(struct widget *widget,
}

/* frame internal buttons */
- x_r = frame->widget->allocation.width - t->width - t->margin;
- x_l = t->width + t->margin;
- y = t->width + t->margin;
+ x_r = frame->widget->allocation.width - t->width - shadow_margin;
+ x_l = t->width + shadow_margin;
+ y = t->width + shadow_margin;
wl_list_for_each(button, &frame->buttons_list, link) {
const int button_padding = 4;
w = cairo_image_surface_get_width(button->icon);
@@ -1504,6 +1523,8 @@ frame_redraw_handler(struct widget *widget, void *data)

if (window->keyboard_device)
flags |= THEME_FRAME_ACTIVE;
+ if (window->type == TYPE_MAXIMIZED)
+ flags |= THEME_FRAME_NO_SHADOW;
theme_render_frame(t, cr, widget->allocation.width,
widget->allocation.height, window->title, flags);

@@ -1514,11 +1535,14 @@ static int
frame_get_pointer_image_for_location(struct frame *frame, struct input *input)
{
struct theme *t = frame->widget->window->display->theme;
+ struct window *window = frame->widget->window;
int location;

location = theme_get_location(t, input->sx, input->sy,
frame->widget->allocation.width,
- frame->widget->allocation.height);
+ frame->widget->allocation.height,
+ window->type == TYPE_MAXIMIZED ?
+ THEME_FRAME_NO_SHADOW : 0);

switch (location) {
case THEME_LOCATION_RESIZING_TOP:
@@ -1610,7 +1634,9 @@ frame_button_handler(struct widget *widget,

location = theme_get_location(display->theme, input->sx, input->sy,
frame->widget->allocation.width,
- frame->widget->allocation.height);
+ frame->widget->allocation.height,
+ window->type == TYPE_MAXIMIZED ?
+ THEME_FRAME_NO_SHADOW : 0);

if (window->display->shell && button == BTN_LEFT &&
state == WL_POINTER_BUTTON_STATE_PRESSED) {
@@ -1700,11 +1726,12 @@ frame_set_child_size(struct widget *widget, int child_width, int child_height)
struct theme *t = display->theme;
int decoration_width, decoration_height;
int width, height;
+ int margin = widget->window->type == TYPE_MAXIMIZED ? 0 : t->margin;

if (widget->window->type != TYPE_FULLSCREEN) {
- decoration_width = (t->width + t->margin) * 2;
+ decoration_width = (t->width + margin) * 2;
decoration_height = t->width +
- t->titlebar_height + t->margin * 2;
+ t->titlebar_height + margin * 2;

width = child_width + decoration_width;
height = child_height + decoration_height;
diff --git a/shared/cairo-util.c b/shared/cairo-util.c
index c64ace2..7b4e537 100644
--- a/shared/cairo-util.c
+++ b/shared/cairo-util.c
@@ -373,23 +373,28 @@ theme_destroy(struct theme *t)
}

void
-theme_render_frame(struct theme *t,
+theme_render_frame(struct theme *t,
cairo_t *cr, int width, int height,
const char *title, uint32_t flags)
{
cairo_text_extents_t extents;
cairo_font_extents_t font_extents;
cairo_surface_t *source;
- int x, y;
+ int x, y, margin;

cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
cairo_set_source_rgba(cr, 0, 0, 0, 0);
cairo_paint(cr);

- cairo_set_source_rgba(cr, 0, 0, 0, 0.45);
- tile_mask(cr, t->shadow,
- 2, 2, width + 8, height + 8,
- 64, 64);
+ if (flags & THEME_FRAME_NO_SHADOW)
+ margin = 0;
+ else {
+ cairo_set_source_rgba(cr, 0, 0, 0, 0.45);
+ tile_mask(cr, t->shadow,
+ 2, 2, width + 8, height + 8,
+ 64, 64);
+ margin = t->margin;
+ }

if (flags & THEME_FRAME_ACTIVE)
source = t->active_frame;
@@ -397,12 +402,12 @@ theme_render_frame(struct theme *t,
source = t->inactive_frame;

tile_source(cr, source,
- t->margin, t->margin,
- width - t->margin * 2, height - t->margin * 2,
+ margin, margin,
+ width - margin * 2, height - margin * 2,
t->width, t->titlebar_height);

- cairo_rectangle (cr, t->margin + t->width, t->margin,
- width - (t->margin + t->width) * 2,
+ cairo_rectangle (cr, margin + t->width, margin,
+ width - (margin + t->width) * 2,
t->titlebar_height - t->width);
cairo_clip(cr);

@@ -414,7 +419,7 @@ theme_render_frame(struct theme *t,
cairo_text_extents(cr, title, &extents);
cairo_font_extents (cr, &font_extents);
x = (width - extents.width) / 2;
- y = t->margin +
+ y = margin +
(t->titlebar_height -
font_extents.ascent - font_extents.descent) / 2 +
font_extents.ascent;
@@ -434,29 +439,33 @@ theme_render_frame(struct theme *t,
}

enum theme_location
-theme_get_location(struct theme *t, int x, int y, int width, int height)
+theme_get_location(struct theme *t, int x, int y,
+ int width, int height, int flags)
{
int vlocation, hlocation, location;
const int grip_size = 8;
+ int margin;
+
+ margin = (flags & THEME_FRAME_NO_SHADOW) ? 0 : t->margin;

- if (x < t->margin)
+ if (x < margin)
hlocation = THEME_LOCATION_EXTERIOR;
- else if (t->margin <= x && x < t->margin + grip_size)
+ else if (margin <= x && x < margin + grip_size)
hlocation = THEME_LOCATION_RESIZING_LEFT;
- else if (x < width - t->margin - grip_size)
+ else if (x < width - margin - grip_size)
hlocation = THEME_LOCATION_INTERIOR;
- else if (x < width - t->margin)
+ else if (x < width - margin)
hlocation = THEME_LOCATION_RESIZING_RIGHT;
else
hlocation = THEME_LOCATION_EXTERIOR;

- if (y < t->margin)
+ if (y < margin)
vlocation = THEME_LOCATION_EXTERIOR;
- else if (t->margin <= y && y < t->margin + grip_size)
+ else if (margin <= y && y < margin + grip_size)
vlocation = THEME_LOCATION_RESIZING_TOP;
- else if (y < height - t->margin - grip_size)
+ else if (y < height - margin - grip_size)
vlocation = THEME_LOCATION_INTERIOR;
- else if (y < height - t->margin)
+ else if (y < height - margin)
vlocation = THEME_LOCATION_RESIZING_BOTTOM;
else
vlocation = THEME_LOCATION_EXTERIOR;
@@ -465,7 +474,7 @@ theme_get_location(struct theme *t, int x, int y, int width, int height)
if (location & THEME_LOCATION_EXTERIOR)
location = THEME_LOCATION_EXTERIOR;
if (location == THEME_LOCATION_INTERIOR &&
- y < t->margin + t->titlebar_height)
+ y < margin + t->titlebar_height)
location = THEME_LOCATION_TITLEBAR;
else if (location == THEME_LOCATION_INTERIOR)
location = THEME_LOCATION_CLIENT_AREA;
diff --git a/shared/cairo-util.h b/shared/cairo-util.h
index 2fec389..16d7fb4 100644
--- a/shared/cairo-util.h
+++ b/shared/cairo-util.h
@@ -58,7 +58,10 @@ theme_create(void);
void
theme_destroy(struct theme *t);

-#define THEME_FRAME_ACTIVE 1
+enum {
+ THEME_FRAME_ACTIVE = 1,
+ THEME_FRAME_NO_SHADOW,
+};

void
theme_render_frame(struct theme *t,
@@ -82,6 +85,6 @@ enum theme_location {
};

enum theme_location
-theme_get_location(struct theme *t, int x, int y, int width, int height);
+theme_get_location(struct theme *t, int x, int y, int width, int height, int flags);

#endif
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index d724d47..6ca7694 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -998,7 +998,7 @@ weston_wm_destroy_cursors(struct weston_wm *wm)
static int
get_cursor_for_location(struct theme *t, int width, int height, int x, int y)
{
- int location = theme_get_location(t, x, y, width, height);
+ int location = theme_get_location(t, x, y, width, height, 0);

switch (location) {
case THEME_LOCATION_RESIZING_TOP:
@@ -1064,7 +1064,7 @@ weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
location = theme_get_location(t,
button->event_x,
button->event_y,
- width, height);
+ width, height, 0);

switch (location) {
case THEME_LOCATION_TITLEBAR:
--
1.7.11.2
Scott Moreau
2012-08-08 02:27:55 UTC
Permalink
The more I think about it, the more I think this functionality might fit
better into shell.c, but this would mean shell would have to know the input
region of the surface. Is there a way to get the input region set by the
client? The region is set in toytoolkit here
http://cgit.freedesktop.org/wayland/weston/tree/clients/window.c#n1288 but
I don't see a way to get it from shell.c.


Thanks,

Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120807/99371ef1/attachment.html>
Scott Moreau
2012-08-08 02:45:21 UTC
Permalink
I found it now in weston_surface->input. I think I want to try rewriting
this patch in shell.c because I think it makes more sense. So please hold
off on this particular version for now.


Thanks,

Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120807/2ef65541/attachment.html>
Scott Moreau
2012-08-08 03:39:37 UTC
Permalink
This effectively fixes a bug where maximized windows appear to not maximize
fully bacause of the shadow margin. Instead, we now maximize the window to
the understood input region.
---

This problem is much more easily and appropriately fixed in shell.c but it makes
the assumption that the input_region is the same for top/bottom and left/right.
We can't calulate bottom or right margin in shell_surface_set_maximized()
because the input region hasn't been updated and there's no way to know in
advance before the configure event. So we have the client resize its surface
accounting for the (shadow) margin, which in this case, is a constant margin for
all sides, derived from the theme margin. We then position the surface taking
into consideration the same.

src/shell.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/shell.c b/src/shell.c
index 6c810ff..0580aeb 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1362,8 +1362,11 @@ shell_surface_set_maximized(struct wl_client *client,
edges = WL_SHELL_SURFACE_RESIZE_TOP|WL_SHELL_SURFACE_RESIZE_LEFT;

shsurf->client->send_configure(shsurf->surface, edges,
- shsurf->output->current->width,
- shsurf->output->current->height - panel_height);
+ shsurf->output->current->width +
+ shsurf->surface->input.extents.x1 * 2,
+ shsurf->output->current->height -
+ panel_height +
+ shsurf->surface->input.extents.y1 * 2);

shsurf->next_type = SHELL_SURFACE_MAXIMIZED;
}
@@ -2673,9 +2676,10 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
break;
case SHELL_SURFACE_MAXIMIZED:
/* setting x, y and using configure to change that geometry */
- surface->geometry.x = surface->output->x;
+ surface->geometry.x = surface->output->x - surface->input.extents.x1;
surface->geometry.y = surface->output->y +
- get_output_panel_height(shell,surface->output);
+ get_output_panel_height(shell,surface->output -
+ surface->input.extents.y1);
break;
case SHELL_SURFACE_TOPLEVEL:
break;
--
1.7.11.2
Pekka Paalanen
2012-08-08 09:31:57 UTC
Permalink
On Tue, 7 Aug 2012 21:39:37 -0600
Post by Scott Moreau
This effectively fixes a bug where maximized windows appear to not maximize
fully bacause of the shadow margin. Instead, we now maximize the window to
the understood input region.
---
This problem is much more easily and appropriately fixed in shell.c but it makes
the assumption that the input_region is the same for top/bottom and left/right.
We can't calulate bottom or right margin in shell_surface_set_maximized()
because the input region hasn't been updated and there's no way to know in
advance before the configure event. So we have the client resize its surface
accounting for the (shadow) margin, which in this case, is a constant margin for
all sides, derived from the theme margin. We then position the surface taking
into consideration the same.
src/shell.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/shell.c b/src/shell.c
index 6c810ff..0580aeb 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1362,8 +1362,11 @@ shell_surface_set_maximized(struct wl_client *client,
edges = WL_SHELL_SURFACE_RESIZE_TOP|WL_SHELL_SURFACE_RESIZE_LEFT;
shsurf->client->send_configure(shsurf->surface, edges,
- shsurf->output->current->width,
- shsurf->output->current->height - panel_height);
+ shsurf->output->current->width +
+ shsurf->surface->input.extents.x1 * 2,
+ shsurf->output->current->height -
+ panel_height +
+ shsurf->surface->input.extents.y1 * 2);
shsurf->next_type = SHELL_SURFACE_MAXIMIZED;
}
@@ -2673,9 +2676,10 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
break;
/* setting x, y and using configure to change that geometry */
- surface->geometry.x = surface->output->x;
+ surface->geometry.x = surface->output->x - surface->input.extents.x1;
surface->geometry.y = surface->output->y +
- get_output_panel_height(shell,surface->output);
+ get_output_panel_height(shell,surface->output -
+ surface->input.extents.y1);
break;
break;
This is definitely not right. It is not the compositor's job to cut out
and not show an arbitrary part of the surface. It is the client's job
to not render anything it does not want to show.

You also break the protocol by lying to client about the dimensions,
and in doing so, you make assumptions that are not based on any
specification, just like you mentioned yourself.

This must be done in toytoolkit, not here.


Sorry,
pq
Scott Moreau
2012-08-08 10:52:41 UTC
Permalink
Post by Pekka Paalanen
This is definitely not right. It is not the compositor's job to cut out
and not show an arbitrary part of the surface. It is the client's job
to not render anything it does not want to show.
You also break the protocol by lying to client about the dimensions,
and in doing so, you make assumptions that are not based on any
specification, just like you mentioned yourself.
This must be done in toytoolkit, not here.
Sorry,
pq
The problem I'm having is trying to work out how to do snapping for resize
or move requests on maximized surfaces. The patch that works in toytoolkit
would require even more dancing in shell to implement snapping. The problem
is that the shell doesn't know some specifics about the surface such as
input region and/or theme margin, in this case the shadow margin. I think
ideally you'd be able to request these specifics from the client. It would
also be nice to be able to know the titlebar height so in the snap-off case
we can place the surface to where the middle of the titlebar is under the
grab cursor. Would it make sense to be able to request certain (theme)
attributes such as these from the client?


Regards,

Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120808/80202709/attachment.html>
Pekka Paalanen
2012-08-08 11:43:21 UTC
Permalink
On Wed, 8 Aug 2012 04:52:41 -0600
Post by Scott Moreau
Post by Pekka Paalanen
This is definitely not right. It is not the compositor's job to cut out
and not show an arbitrary part of the surface. It is the client's job
to not render anything it does not want to show.
You also break the protocol by lying to client about the dimensions,
and in doing so, you make assumptions that are not based on any
specification, just like you mentioned yourself.
This must be done in toytoolkit, not here.
Sorry,
pq
The problem I'm having is trying to work out how to do snapping for resize
or move requests on maximized surfaces. The patch that works in toytoolkit
would require even more dancing in shell to implement snapping. The problem
is that the shell doesn't know some specifics about the surface such as
input region and/or theme margin, in this case the shadow margin. I think
ideally you'd be able to request these specifics from the client. It would
also be nice to be able to know the titlebar height so in the snap-off case
we can place the surface to where the middle of the titlebar is under the
grab cursor. Would it make sense to be able to request certain (theme)
attributes such as these from the client?
Snapping must happen in the server, since the server does all about
moving surfaces. This is completely different to shadows, which must
remain a purely client feature.

I don't think you should need such things as "theme margin" or
whatever. At most, you could add a snapping region, similar to input
region, in the protocol. That would be more logical: "this region will
be used for snapping" instead of "if I set shadow region like this, it
usually snaps there...".

Snapping could use the input region, since it is always up-to-date
with the latest buffer the server has. Except that seems to be a lie,
because surface::attach resets the input region. Looks like we have
some protocol to fix here.

The current protocol works like this, AFAIU:
1. wl_surface::attach - input region may get reset
2. wl_surface::set_input_region - input region gets properly defined
again

This means, that between steps 1 and 2, the server will composite using
bad data (undefined input region). No, we cannot rely on the
set_input_region request coming right after the attach, nothing
guarantees that it will be processed during the current frame.

This makes snapping on resize to likely fail, since the snapping code
will be working with an undefined input region, and hence it can only
ever snap to the surface edge, which due to the shadow will fall far
off from the "real" window edge.

Krh, was the the idea of first sending all new surface attributes, and
then committing those at once on wl_surface::attach rejected, or was
this part of the protocol just not fixed yet? Or is there some other
clever mechanism to make this atomic?


Thanks,
pq
Kristian Høgsberg
2012-08-09 16:44:43 UTC
Permalink
Post by Pekka Paalanen
On Wed, 8 Aug 2012 04:52:41 -0600
Post by Scott Moreau
Post by Pekka Paalanen
This is definitely not right. It is not the compositor's job to cut out
and not show an arbitrary part of the surface. It is the client's job
to not render anything it does not want to show.
You also break the protocol by lying to client about the dimensions,
and in doing so, you make assumptions that are not based on any
specification, just like you mentioned yourself.
This must be done in toytoolkit, not here.
Sorry,
pq
The problem I'm having is trying to work out how to do snapping for resize
or move requests on maximized surfaces. The patch that works in toytoolkit
would require even more dancing in shell to implement snapping. The problem
is that the shell doesn't know some specifics about the surface such as
input region and/or theme margin, in this case the shadow margin. I think
ideally you'd be able to request these specifics from the client. It would
also be nice to be able to know the titlebar height so in the snap-off case
we can place the surface to where the middle of the titlebar is under the
grab cursor. Would it make sense to be able to request certain (theme)
attributes such as these from the client?
Snapping must happen in the server, since the server does all about
moving surfaces. This is completely different to shadows, which must
remain a purely client feature.
I don't think you should need such things as "theme margin" or
whatever. At most, you could add a snapping region, similar to input
region, in the protocol. That would be more logical: "this region will
be used for snapping" instead of "if I set shadow region like this, it
usually snaps there...".
Snapping could use the input region, since it is always up-to-date
with the latest buffer the server has. Except that seems to be a lie,
because surface::attach resets the input region. Looks like we have
some protocol to fix here.
1. wl_surface::attach - input region may get reset
2. wl_surface::set_input_region - input region gets properly defined
again
This means, that between steps 1 and 2, the server will composite using
bad data (undefined input region). No, we cannot rely on the
set_input_region request coming right after the attach, nothing
guarantees that it will be processed during the current frame.
This makes snapping on resize to likely fail, since the snapping code
will be working with an undefined input region, and hence it can only
ever snap to the surface edge, which due to the shadow will fall far
off from the "real" window edge.
Krh, was the the idea of first sending all new surface attributes, and
then committing those at once on wl_surface::attach rejected, or was
this part of the protocol just not fixed yet? Or is there some other
clever mechanism to make this atomic?
It wasn't rejected, just never implemented. The problem is that in
practice it's not necessary, since typically the protocol buffer will
ensure atomicity. Even if that gets flushed unexpectedly, most
clients will re-render in response to frame events or input events,
which we send out at the beginning of the frame, giving clients a
(just under) 16ms window to get things done before their requests
might get broken across two frames.

But yes, the fact that it is possible isn't really compatible with
"every frame is perfect". Mostly I've just been afraid of
overengineering this, but maybe we can just specify that certain
requests are latched until the next surface.attach request. As a rule
of thumb this would apply to all requests that alter state that
depends on the buffer size or contents. This would apply to opaque
and input regions, and in fact, wl_shell_surface.set_fullscreen
already works this way.

It's also pretty easy for extensions to tie into this. They just
document which properties are latched. For example position of
overlaid surfaces or buffer contents rotation could be done that way.

By the way, the way it works now, we invalidate the input region if we
attach a buffer of a different size, but I'm thinking that that's very
un-wayland-ish and we should just always expect the client to attach a
new input region.

Kristian
Pekka Paalanen
2012-08-10 07:43:38 UTC
Permalink
On Thu, 9 Aug 2012 12:44:43 -0400
Post by Kristian Høgsberg
Post by Pekka Paalanen
Krh, was the the idea of first sending all new surface attributes, and
then committing those at once on wl_surface::attach rejected, or was
this part of the protocol just not fixed yet? Or is there some other
clever mechanism to make this atomic?
It wasn't rejected, just never implemented. The problem is that in
practice it's not necessary, since typically the protocol buffer will
ensure atomicity. Even if that gets flushed unexpectedly, most
clients will re-render in response to frame events or input events,
which we send out at the beginning of the frame, giving clients a
(just under) 16ms window to get things done before their requests
might get broken across two frames.
Errm...
Post by Kristian Høgsberg
But yes, the fact that it is possible isn't really compatible with
"every frame is perfect". Mostly I've just been afraid of
overengineering this, but maybe we can just specify that certain
requests are latched until the next surface.attach request. As a rule
of thumb this would apply to all requests that alter state that
depends on the buffer size or contents. This would apply to opaque
and input regions, and in fact, wl_shell_surface.set_fullscreen
already works this way.
It's also pretty easy for extensions to tie into this. They just
document which properties are latched. For example position of
overlaid surfaces or buffer contents rotation could be done that way.
By the way, the way it works now, we invalidate the input region if we
attach a buffer of a different size, but I'm thinking that that's very
un-wayland-ish and we should just always expect the client to attach a
new input region.
Yes! :-)

I've actually pondered, if we should implement a Weston "synchronous"
debug mode, where after handling every single protocol request it would
force a repaint (not damage) and make sure the repaint hits the screen.
Maybe even add a delay to let possible glitches stay longer on screen.
Considering we don't run thousands of requests per second with a few
clients, it shouldn't be unbearably slow, just slow.


Thanks,
pq
Andreas Ericsson
2012-08-08 12:12:42 UTC
Permalink
Post by Scott Moreau
Post by Pekka Paalanen
This is definitely not right. It is not the compositor's job to cut out
and not show an arbitrary part of the surface. It is the client's job
to not render anything it does not want to show.
You also break the protocol by lying to client about the dimensions,
and in doing so, you make assumptions that are not based on any
specification, just like you mentioned yourself.
This must be done in toytoolkit, not here.
Sorry,
pq
The problem I'm having is trying to work out how to do snapping for resize
or move requests on maximized surfaces. The patch that works in toytoolkit
would require even more dancing in shell to implement snapping. The problem
is that the shell doesn't know some specifics about the surface such as
input region and/or theme margin, in this case the shadow margin.
Personally, I've always felt it's really weird when ethereal entities such as
shadows interact with physical entities such as the plastic edge framing the
LCD part of my screen. In any sane faking of the real world that plastic edge
would simply cover the shadow, so it makes perfect sense to ignore it
completely when snapping windows.
--
Andreas Ericsson andreas.ericsson at op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
Kristian Høgsberg
2012-08-09 15:59:47 UTC
Permalink
Post by Scott Moreau
This effectively fixes a bug where maximized windows appear to not maximize fully
bacause of the shadow margin. Instead, we maximize the window to the input region.
---
* Applied the same logic to both instances of theme_get_location() so the resize
cursors show properly for maximized windows
* Fixed window-manager.c flags
* Fixed a small indentation mishap
* Simplified the assignment case in theme_get_location()
What we need here is to tell the theme code "draw maximized window
decorations" not a "no shadow" flag.

Kristian
Post by Scott Moreau
---
clients/window.c | 75 +++++++++++++++++++++++++++++--------------
shared/cairo-util.c | 51 +++++++++++++++++------------
shared/cairo-util.h | 7 ++--
src/xwayland/window-manager.c | 4 +--
4 files changed, 88 insertions(+), 49 deletions(-)
diff --git a/clients/window.c b/clients/window.c
index 30a6167..05f1e85 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -1244,9 +1244,37 @@ frame_resize_handler(struct widget *widget,
struct theme *t = display->theme;
int x_l, x_r, y, w, h;
int decoration_width, decoration_height;
- int opaque_margin;
+ int opaque_margin, shadow_margin;
- if (widget->window->type != TYPE_FULLSCREEN) {
+ switch (widget->window->type) {
+ decoration_width = 0;
+ decoration_height = 0;
+
+ allocation.x = 0;
+ allocation.y = 0;
+ allocation.width = width;
+ allocation.height = height;
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 0;
+ break;
+ decoration_width = t->width * 2;
+ decoration_height = t->width + t->titlebar_height;
+
+ allocation.x = t->width;
+ allocation.y = t->titlebar_height;
+ allocation.width = width - decoration_width;
+ allocation.height = height - decoration_height;
+
+ opaque_margin = 0;
+
+ wl_list_for_each(button, &frame->buttons_list, link)
+ button->widget->opaque = 1;
+ break;
decoration_width = (t->width + t->margin) * 2;
decoration_height = t->width +
t->titlebar_height + t->margin * 2;
@@ -1260,18 +1288,7 @@ frame_resize_handler(struct widget *widget,
wl_list_for_each(button, &frame->buttons_list, link)
button->widget->opaque = 1;
- } else {
- decoration_width = 0;
- decoration_height = 0;
-
- allocation.x = 0;
- allocation.y = 0;
- allocation.width = width;
- allocation.height = height;
- opaque_margin = 0;
-
- wl_list_for_each(button, &frame->buttons_list, link)
- button->widget->opaque = 0;
+ break;
}
widget_set_allocation(child, allocation.x, allocation.y,
@@ -1286,13 +1303,15 @@ frame_resize_handler(struct widget *widget,
width = child->allocation.width + decoration_width;
height = child->allocation.height + decoration_height;
+ shadow_margin = widget->window->type == TYPE_MAXIMIZED ? 0 : t->margin;
+
if (widget->window->type != TYPE_FULLSCREEN) {
widget->window->input_region =
wl_compositor_create_region(display->compositor);
wl_region_add(widget->window->input_region,
- t->margin, t->margin,
- width - 2 * t->margin,
- height - 2 * t->margin);
+ shadow_margin, shadow_margin,
+ width - 2 * shadow_margin,
+ height - 2 * shadow_margin);
}
widget_set_allocation(widget, 0, 0, width, height);
@@ -1307,9 +1326,9 @@ frame_resize_handler(struct widget *widget,
}
/* frame internal buttons */
- x_r = frame->widget->allocation.width - t->width - t->margin;
- x_l = t->width + t->margin;
- y = t->width + t->margin;
+ x_r = frame->widget->allocation.width - t->width - shadow_margin;
+ x_l = t->width + shadow_margin;
+ y = t->width + shadow_margin;
wl_list_for_each(button, &frame->buttons_list, link) {
const int button_padding = 4;
w = cairo_image_surface_get_width(button->icon);
@@ -1504,6 +1523,8 @@ frame_redraw_handler(struct widget *widget, void *data)
if (window->keyboard_device)
flags |= THEME_FRAME_ACTIVE;
+ if (window->type == TYPE_MAXIMIZED)
+ flags |= THEME_FRAME_NO_SHADOW;
theme_render_frame(t, cr, widget->allocation.width,
widget->allocation.height, window->title, flags);
@@ -1514,11 +1535,14 @@ static int
frame_get_pointer_image_for_location(struct frame *frame, struct input *input)
{
struct theme *t = frame->widget->window->display->theme;
+ struct window *window = frame->widget->window;
int location;
location = theme_get_location(t, input->sx, input->sy,
frame->widget->allocation.width,
- frame->widget->allocation.height);
+ frame->widget->allocation.height,
+ window->type == TYPE_MAXIMIZED ?
+ THEME_FRAME_NO_SHADOW : 0);
switch (location) {
@@ -1610,7 +1634,9 @@ frame_button_handler(struct widget *widget,
location = theme_get_location(display->theme, input->sx, input->sy,
frame->widget->allocation.width,
- frame->widget->allocation.height);
+ frame->widget->allocation.height,
+ window->type == TYPE_MAXIMIZED ?
+ THEME_FRAME_NO_SHADOW : 0);
if (window->display->shell && button == BTN_LEFT &&
state == WL_POINTER_BUTTON_STATE_PRESSED) {
@@ -1700,11 +1726,12 @@ frame_set_child_size(struct widget *widget, int child_width, int child_height)
struct theme *t = display->theme;
int decoration_width, decoration_height;
int width, height;
+ int margin = widget->window->type == TYPE_MAXIMIZED ? 0 : t->margin;
if (widget->window->type != TYPE_FULLSCREEN) {
- decoration_width = (t->width + t->margin) * 2;
+ decoration_width = (t->width + margin) * 2;
decoration_height = t->width +
- t->titlebar_height + t->margin * 2;
+ t->titlebar_height + margin * 2;
width = child_width + decoration_width;
height = child_height + decoration_height;
diff --git a/shared/cairo-util.c b/shared/cairo-util.c
index c64ace2..7b4e537 100644
--- a/shared/cairo-util.c
+++ b/shared/cairo-util.c
@@ -373,23 +373,28 @@ theme_destroy(struct theme *t)
}
void
-theme_render_frame(struct theme *t,
+theme_render_frame(struct theme *t,
cairo_t *cr, int width, int height,
const char *title, uint32_t flags)
{
cairo_text_extents_t extents;
cairo_font_extents_t font_extents;
cairo_surface_t *source;
- int x, y;
+ int x, y, margin;
cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
cairo_set_source_rgba(cr, 0, 0, 0, 0);
cairo_paint(cr);
- cairo_set_source_rgba(cr, 0, 0, 0, 0.45);
- tile_mask(cr, t->shadow,
- 2, 2, width + 8, height + 8,
- 64, 64);
+ if (flags & THEME_FRAME_NO_SHADOW)
+ margin = 0;
+ else {
+ cairo_set_source_rgba(cr, 0, 0, 0, 0.45);
+ tile_mask(cr, t->shadow,
+ 2, 2, width + 8, height + 8,
+ 64, 64);
+ margin = t->margin;
+ }
if (flags & THEME_FRAME_ACTIVE)
source = t->active_frame;
@@ -397,12 +402,12 @@ theme_render_frame(struct theme *t,
source = t->inactive_frame;
tile_source(cr, source,
- t->margin, t->margin,
- width - t->margin * 2, height - t->margin * 2,
+ margin, margin,
+ width - margin * 2, height - margin * 2,
t->width, t->titlebar_height);
- cairo_rectangle (cr, t->margin + t->width, t->margin,
- width - (t->margin + t->width) * 2,
+ cairo_rectangle (cr, margin + t->width, margin,
+ width - (margin + t->width) * 2,
t->titlebar_height - t->width);
cairo_clip(cr);
@@ -414,7 +419,7 @@ theme_render_frame(struct theme *t,
cairo_text_extents(cr, title, &extents);
cairo_font_extents (cr, &font_extents);
x = (width - extents.width) / 2;
- y = t->margin +
+ y = margin +
(t->titlebar_height -
font_extents.ascent - font_extents.descent) / 2 +
font_extents.ascent;
@@ -434,29 +439,33 @@ theme_render_frame(struct theme *t,
}
enum theme_location
-theme_get_location(struct theme *t, int x, int y, int width, int height)
+theme_get_location(struct theme *t, int x, int y,
+ int width, int height, int flags)
{
int vlocation, hlocation, location;
const int grip_size = 8;
+ int margin;
+
+ margin = (flags & THEME_FRAME_NO_SHADOW) ? 0 : t->margin;
- if (x < t->margin)
+ if (x < margin)
hlocation = THEME_LOCATION_EXTERIOR;
- else if (t->margin <= x && x < t->margin + grip_size)
+ else if (margin <= x && x < margin + grip_size)
hlocation = THEME_LOCATION_RESIZING_LEFT;
- else if (x < width - t->margin - grip_size)
+ else if (x < width - margin - grip_size)
hlocation = THEME_LOCATION_INTERIOR;
- else if (x < width - t->margin)
+ else if (x < width - margin)
hlocation = THEME_LOCATION_RESIZING_RIGHT;
else
hlocation = THEME_LOCATION_EXTERIOR;
- if (y < t->margin)
+ if (y < margin)
vlocation = THEME_LOCATION_EXTERIOR;
- else if (t->margin <= y && y < t->margin + grip_size)
+ else if (margin <= y && y < margin + grip_size)
vlocation = THEME_LOCATION_RESIZING_TOP;
- else if (y < height - t->margin - grip_size)
+ else if (y < height - margin - grip_size)
vlocation = THEME_LOCATION_INTERIOR;
- else if (y < height - t->margin)
+ else if (y < height - margin)
vlocation = THEME_LOCATION_RESIZING_BOTTOM;
else
vlocation = THEME_LOCATION_EXTERIOR;
@@ -465,7 +474,7 @@ theme_get_location(struct theme *t, int x, int y, int width, int height)
if (location & THEME_LOCATION_EXTERIOR)
location = THEME_LOCATION_EXTERIOR;
if (location == THEME_LOCATION_INTERIOR &&
- y < t->margin + t->titlebar_height)
+ y < margin + t->titlebar_height)
location = THEME_LOCATION_TITLEBAR;
else if (location == THEME_LOCATION_INTERIOR)
location = THEME_LOCATION_CLIENT_AREA;
diff --git a/shared/cairo-util.h b/shared/cairo-util.h
index 2fec389..16d7fb4 100644
--- a/shared/cairo-util.h
+++ b/shared/cairo-util.h
@@ -58,7 +58,10 @@ theme_create(void);
void
theme_destroy(struct theme *t);
-#define THEME_FRAME_ACTIVE 1
+enum {
+ THEME_FRAME_ACTIVE = 1,
+ THEME_FRAME_NO_SHADOW,
+};
void
theme_render_frame(struct theme *t,
@@ -82,6 +85,6 @@ enum theme_location {
};
enum theme_location
-theme_get_location(struct theme *t, int x, int y, int width, int height);
+theme_get_location(struct theme *t, int x, int y, int width, int height, int flags);
#endif
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index d724d47..6ca7694 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -998,7 +998,7 @@ weston_wm_destroy_cursors(struct weston_wm *wm)
static int
get_cursor_for_location(struct theme *t, int width, int height, int x, int y)
{
- int location = theme_get_location(t, x, y, width, height);
+ int location = theme_get_location(t, x, y, width, height, 0);
switch (location) {
@@ -1064,7 +1064,7 @@ weston_wm_handle_button(struct weston_wm *wm, xcb_generic_event_t *event)
location = theme_get_location(t,
button->event_x,
button->event_y,
- width, height);
+ width, height, 0);
switch (location) {
--
1.7.11.2
_______________________________________________
wayland-devel mailing list
wayland-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Bill Spitzak
2012-08-11 01:47:33 UTC
Permalink
This looks really messy and still makes the assumption that the only
reason for "edges" to be removed is because of "maximize". It is also
completely different than how "fullscreen" is done, even though that
should be identical except the "titlebar" is also removed for fullscreen.

Based on the latest changes to the shell api, I feel the following must
be added:

1. The "content" region. This is the part of the window that does not
include the "edges". However it *does* include the titlebar. Imagine the
"part of the window you see when it is maximized". This is different and
smaller than the input region and should be specified by clients using a
new api very similar to how the input and opaque regions are specified.
Shells need this information to property implement snap-to-edge (where
the edge is of another window, a panel, or an output), and to implement
maximizes other than the full-screen maximize (ie vertically-only). It
also means "maximize" is not a special state, instead the shell just
resizes the surface so that the contents fill the output.

2. When the shell tells a client to configure a window, it should
communicate both a size for the surface image, and the bounding box of
the content region in this new size. This bounding box will always be
the same distance or closer to the edges of the surface (ie it is only
used to clip edges, not to make them thicker). This allows clients to
know that edges are clipped so they do not place any important controls
there, and they can adjust their graphics to take into account the
clipping (for instance not drawing rounded-corner shading for a
clipped-off rounded corner). It will also save some memory and rendering
time by clipping these off the allocated image buffer.

Ideally this new box should be added to the configure event and we
should just require clients to obey it. If this is not allowed due to
the api being frozen, it could be a different event, but it suffers from
the ugly fact that the size sent with configure is not the actual size
the surface should be (because that would make a client that does not
clip the edges make the window too small when maximized).

3. Fullscreen: somewhat unrelated, but the shell should be able to
directly tell clients to "remove all decorations". Currently this is
tied into the fullscreen request from a client. This is necessary so
shells can force a client to fullscreen. It is also needed for embedding
one wayland client in another. It may also be useful for displaying
wayland clients on legacy window systems that make it very difficult to
remove their own decorations.
Scott Moreau
2012-08-11 05:55:58 UTC
Permalink
Post by Bill Spitzak
This looks really messy and still makes the assumption that the only
reason for "edges" to be removed is because of "maximize". It is also
completely different than how "fullscreen" is done, even though that should
be identical except the "titlebar" is also removed for fullscreen.
Based on the latest changes to the shell api, I feel the following must be
1. The "content" region. This is the part of the window that does not
include the "edges". However it *does* include the titlebar. Imagine the
"part of the window you see when it is maximized". This is different and
smaller than the input region
No it isn't. The input region and your theoretical content region are
exactly the same.
Post by Bill Spitzak
and should be specified by clients using a new api very similar to how the
input and opaque regions are specified. Shells need this information to
property implement snap-to-edge (where the edge is of another window, a
panel, or an output),
No it doesn't. We can do snap-to-edge for any edge with the input and
surface areas known already.
Post by Bill Spitzak
and to implement maximizes other than the full-screen maximize (ie
vertically-only). It also means "maximize" is not a special state, instead
the shell just resizes the surface so that the contents fill the output.
This doesn't really have anything to do with what we're talking about here.
Post by Bill Spitzak
2. When the shell tells a client to configure a window, it should
communicate both a size for the surface image, and the bounding box of the
content region in this new size.
It already does this and it's called the input region.
Post by Bill Spitzak
This bounding box will always be the same distance or closer to the edges
of the surface (ie it is only used to clip edges, not to make them
thicker). This allows clients to know that edges are clipped so they do not
place any important controls there, and they can adjust their graphics to
take into account the clipping (for instance not drawing rounded-corner
shading for a clipped-off rounded corner). It will also save some memory
and rendering time by clipping these off the allocated image buffer.
You are over-thinking this far too much. We don't need any of this.
Post by Bill Spitzak
Ideally this new box should be added to the configure event and we should
just require clients to obey it. If this is not allowed due to the api
being frozen, it could be a different event, but it suffers from the ugly
fact that the size sent with configure is not the actual size the surface
should be (because that would make a client that does not clip the edges
make the window too small when maximized).
It's the same as the input region which we already have.
Post by Bill Spitzak
3. Fullscreen: somewhat unrelated, but the shell should be able to
directly tell clients to "remove all decorations". Currently this is tied
into the fullscreen request from a client. This is necessary so shells can
force a client to fullscreen. It is also needed for embedding one wayland
client in another. It may also be useful for displaying wayland clients on
legacy window systems that make it very difficult to remove their own
decorations.
This sounds like a bunch of stuff we don't need. The client will have
special cases for decoration flags such as maximized and none.


To make your point more specific, why not submit a patch implementing what
you think should happen? I'm not really sure why you're doing this whole
write up without any code to show. Further, we should strive to make
initial implementations as simple as possible and not try to over
complicate it. That said, there is a bug where the input or opaque region
may be invalid at any given time. My snap implementation built on top of
the v3 patch here works around the issue but we still need to restructure
the way input and opaque regions are handled before advancing on this.



Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120810/85546da8/attachment.html>
Bill Spitzak
2012-08-13 16:54:28 UTC
Permalink
Post by Bill Spitzak
1. The "content" region. This is the part of the window that does
not include the "edges". However it *does* include the titlebar.
Imagine the "part of the window you see when it is maximized". This
is different and smaller than the input region
No it isn't. The input region and your theoretical content region are
exactly the same.
The input region includes the window edges, which is outside the region
I am talking about.

There may be a failure to understand exactly what pixels I am talking
about. Lets zoom way in on the right edge of a Windows95-style window
which is filled with solid green. Here are the (approximate) colors of
the right 4 pixels:

green light-gray darker-gray black

The green pixel is inside my proposed region. The light and dark gray
pixels are inside the input region (because the user can click on them
to resize the window). Thus my region is smaller than the input region.

(The black pixel probably is inside the input region as well, though I
think it should be outside to properly get snap-to-outside-edge
behavior, it can be considered a very dense shadow).

If you maximize the window, you can see that my region is what is lined
up with the edges of the output and with any panels around the screen.
Thus this region has meaning. A "maximized" bit is insufficient as it
does not describe other states such as maximized-vertically.
Post by Bill Spitzak
and should be specified by clients using a new api very similar to
how the input and opaque regions are specified. Shells need this
information to property implement snap-to-edge (where the edge is of
another window, a panel, or an output),
No it doesn't. We can do snap-to-edge for any edge with the input and
surface areas known already.
You can only snap to the *OUTSIDE* of the edge. There is no indication
as to where the inside of the edge is. This information is needed to get
proper snap to inside edge and to snap windows to each other or behind
panels.
Post by Bill Spitzak
2. When the shell tells a client to configure a window, it should
communicate both a size for the surface image, and the bounding box
of the content region in this new size.
It already does this and it's called the input region.
Sorry, I do not see any shell event saying what the input region size
should be. The client can tell the shell, but not the other way around.

If this was added it would work, though it has the unfortunate fact that
it would have a negative top-left corner for a maximized window. That is
why I would prefer that the shell communicate the contents region.

It does not remove the need for the shell to know the contents region,
since the shell cannot correctly do snap to inside edges without it.
Post by Bill Spitzak
To make your point more specific, why not submit a patch implementing
what you think should happen? I'm not really sure why you're doing this
whole write up without any code to show.
I have been trying for a few months to successfully compile Wayland. It
used to work but has failed ever since the api freeze. I did upgrade the
machines being used to the newest Ubuntu but this hardly made things better.

At the moment the display I have is skewed unless the surfaces are a
multiple of 16 pixels wide. It is obvious that some stride requirement
is not being communicated correctly from the driver to the client, but I
did not want to annoy the list until I tried to figure out how this
information should be sent.

It also is dreadfully slow because, despite changes to the driver and 2
updates to the entire os, and changing the window manager to 4 different
ones (both compositing and non-compositing), and following suggestions
that I alter compositing settings) I still have the problem that
x11_compositor only sees one event and then insists on redrawing the
screen. Does anybody else have this problem?
Scott Moreau
2012-08-14 01:22:25 UTC
Permalink
Post by Bill Spitzak
1. The "content" region. This is the part of the window that does
Post by Bill Spitzak
not include the "edges". However it *does* include the titlebar.
Imagine the "part of the window you see when it is maximized". This
is different and smaller than the input region
No it isn't. The input region and your theoretical content region are
exactly the same.
The input region includes the window edges, which is outside the region I
am talking about.
I can't think of a reason why the compositor would need to know this
information. It is most certainly not what you snap to.
Post by Bill Spitzak
There may be a failure to understand exactly what pixels I am talking
about. Lets zoom way in on the right edge of a Windows95-style window which
is filled with solid green. Here are the (approximate) colors of the right
green light-gray darker-gray black
The green pixel is inside my proposed region. The light and dark gray
pixels are inside the input region (because the user can click on them to
resize the window). Thus my region is smaller than the input region.
(The black pixel probably is inside the input region as well, though I
think it should be outside to properly get snap-to-outside-edge behavior,
it can be considered a very dense shadow).
If you maximize the window, you can see that my region is what is lined up
with the edges of the output and with any panels around the screen. Thus
this region has meaning. A "maximized" bit is insufficient as it does not
describe other states such as maximized-vertically.
You're using Windows 95 as a base example? First of all, I don't know why
you'd assume that everyone knows how that interface looked. Second, I don't
even want to remember how it looked. Third, we're not trying to recreate
windows starting from '95. I really think this is a very poor choice for an
example.
Post by Bill Spitzak
and should be specified by clients using a new api very similar to
Post by Bill Spitzak
how the input and opaque regions are specified. Shells need this
information to property implement snap-to-edge (where the edge is of
another window, a panel, or an output),
No it doesn't. We can do snap-to-edge for any edge with the input and
surface areas known already.
You can only snap to the *OUTSIDE* of the edge. There is no indication as
to where the inside of the edge is. This information is needed to get
proper snap to inside edge and to snap windows to each other or behind
panels.
No, that's not how this works. Since the client draws it's own decorations,
it will change them based on state. For instance if it's fullscreen, it
draws none. For maximize, it shouldn't draw the shadow border. If it's your
client, you can do whatever; and that's where this needs to happen. The
client draws it's contents and decoration, then tells the compositor what
the input region is. The client can choose to draw anything or set any
input rect it wants to. This is the way it works. We do not need to know
the content area on compositor side.
Post by Bill Spitzak
2. When the shell tells a client to configure a window, it should
Post by Bill Spitzak
communicate both a size for the surface image, and the bounding box
of the content region in this new size.
It already does this and it's called the input region.
Sorry, I do not see any shell event saying what the input region size
should be. The client can tell the shell, but not the other way around.
It does not need to go backward.
Post by Bill Spitzak
If this was added it would work, though it has the unfortunate fact that
it would have a negative top-left corner for a maximized window. That is
why I would prefer that the shell communicate the contents region.
It does not remove the need for the shell to know the contents region,
since the shell cannot correctly do snap to inside edges without it.
Snapping will work fine. You do not snap to the inside edge of the
decoration, you snap to the input region. The client can set the input
region to whatever it wants.
Post by Bill Spitzak
To make your point more specific, why not submit a patch implementing
Post by Bill Spitzak
what you think should happen? I'm not really sure why you're doing this
whole write up without any code to show.
I have been trying for a few months to successfully compile Wayland. It
used to work but has failed ever since the api freeze. I did upgrade the
machines being used to the newest Ubuntu but this hardly made things better.
It has been building fine here for at least a couple months. You should try
latest with the patches for mesa from here
https://bugs.freedesktop.org/show_bug.cgi?id=52267
Post by Bill Spitzak
At the moment the display I have is skewed unless the surfaces are a
multiple of 16 pixels wide. It is obvious that some stride requirement is
not being communicated correctly from the driver to the client, but I did
not want to annoy the list until I tried to figure out how this information
should be sent.
You should probably file a bug report about this.
Post by Bill Spitzak
It also is dreadfully slow because, despite changes to the driver and 2
updates to the entire os, and changing the window manager to 4 different
ones (both compositing and non-compositing), and following suggestions that
I alter compositing settings) I still have the problem that x11_compositor
only sees one event and then insists on redrawing the screen. Does anybody
else have this problem?
I don't know what you're talking about here but you can try the
show_repaint patch on the list to see if it's actually redrawing what you
think it is.



Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20120813/3aa13a4a/attachment-0001.html>
Bill Spitzak
2012-08-14 16:54:41 UTC
Permalink
Post by Scott Moreau
For maximize, it shouldn't draw the shadow
border.
Every window manager I have seen does not draw the "edges" as well.

If you have not noticed this, it may explain why you are not seeing the
problem as I am.
Post by Scott Moreau
Snapping will work fine. You do not snap to the inside edge of the
decoration, you snap to the input region.
Plenty of X window managers do. Most common is when windows are snapped
to top panels, the top edge goes under the panel to match the titlebar
position to that which it will have when maximized. I agree that
snapping to the inside on the other 4 edges is less common, but I was
hoping Wayland would allow more consistency.

Basically what I want is for users to be able to 'snap' to the same
locations that a maximized window is in. I would also like maximize
vertically to produce the same vertical size as maximize.
Post by Scott Moreau
It has been building fine here for at least a couple months. You should
try latest with the patches for mesa from here
https://bugs.freedesktop.org/show_bug.cgi?id=52267
I sent some email with my current problems. Recompiling everything from
the most recent git is not helping.

Continue reading on narkive:
Loading...