Created attachment 299918 [details] [review]
wayland: Fix damage of infinite regions

Infinte regions (0, 0) -> (INT32_MAX, INT32_MAX) were still scaled,
causing integer overflow.

Review of attachment 299918 [details] [review]:

The overflow might still happen in meta_region_scale() if a (dumb?) client sends us big enough values.

Perhaps we should sanitize the rects in wl_surface_damage() as they come in, intersecting them with the surface rect?

(In reply to Rui Matos from comment #2)
> Review of attachment 299918 [details] [review] [review]:
> 
> The overflow might still happen in meta_region_scale() if a (dumb?) client
> sends us big enough values.
> 
> Perhaps we should sanitize the rects in wl_surface_damage() as they come in,
> intersecting them with the surface rect?

Yes I suppose we could do that. I'll update the patch.

When do we have infinite damage?

When the client uses the method eglSwapBuffers without damage.(In reply to Jasper St. Pierre from comment #4)
> When do we have infinite damage?

EGL clients.

(In reply to Jasper St. Pierre from comment #4)
> When do we have infinite damage?

Yes that dumb mesa hack.

Created attachment 300333 [details] [review]
wayland: Fix damage of infinite regions

To avoid integer overflow when scaling "infinite" regions (0, 0)
(INT32_MAX, INT32_MAX), intersect with the surface rect before scaling,
instead of intersecting with the buffer rect afterwards.

Review of attachment 300333 [details] [review]:

::: src/wayland/meta-wayland-surface.c
@@ +136,3 @@
+  buffer_width = cogl_texture_get_width (surface->buffer->texture);
+  buffer_height = cogl_texture_get_height (surface->buffer->texture);
+  surface_rect = (cairo_rectangle_int_t) {

Surely surface_rect's (lack of) initialization above means that .x and .y have undefined values here?

Also, FWIW, we don't use this kind of struct assignment anywhere (AFAIK) in mutter though I don't personally mind it.

(In reply to Rui Matos from comment #8)
> Review of attachment 300333 [details] [review] [review]:
> 
> ::: src/wayland/meta-wayland-surface.c
> @@ +136,3 @@
> +  buffer_width = cogl_texture_get_width (surface->buffer->texture);
> +  buffer_height = cogl_texture_get_height (surface->buffer->texture);
> +  surface_rect = (cairo_rectangle_int_t) {
> 
> Surely surface_rect's (lack of) initialization above means that .x and .y
> have undefined values here?

Actually in C99 .x and .y will be initialized to 0 here.

> 
> Also, FWIW, we don't use this kind of struct assignment anywhere (AFAIK) in
> mutter though I don't personally mind it.

Personally I prefer this declarative kind of definitions.

I've used it before in mutter. I really like the construction, and wish we could use it more. It's known as a "compound literal".

https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Compound-Literals.html

Review of attachment 300333 [details] [review]:

(In reply to Jonas Ådahl from comment #9)
> Actually in C99 .x and .y will be initialized to 0 here.

Indeed, cool. And Jasper likes it too so let's not delay this

Attachment 300333 [details] pushed as dbca333 - wayland: Fix damage of infinite regions