diff mbox series

[v6,12/23] virtio-gpu: replace PIXMAN for region/rect test

Message ID 20231025190818.3278423-13-marcandre.lureau@redhat.com
State New
Headers show
Series Make Pixman an optional dependency | expand

Commit Message

Marc-André Lureau Oct. 25, 2023, 7:08 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Use a simpler implementation for rectangle geometry & intersect, drop
the need for (more complex) PIXMAN functions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/ui/rect.h       | 59 +++++++++++++++++++++++++++++++++++++++++
 hw/display/virtio-gpu.c | 30 ++++++++-------------
 2 files changed, 70 insertions(+), 19 deletions(-)
 create mode 100644 include/ui/rect.h

Comments

BALATON Zoltan Oct. 25, 2023, 8:54 p.m. UTC | #1 
On Wed, 25 Oct 2023, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Use a simpler implementation for rectangle geometry & intersect, drop
> the need for (more complex) PIXMAN functions.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/ui/rect.h       | 59 +++++++++++++++++++++++++++++++++++++++++
> hw/display/virtio-gpu.c | 30 ++++++++-------------
> 2 files changed, 70 insertions(+), 19 deletions(-)
> create mode 100644 include/ui/rect.h
>
> diff --git a/include/ui/rect.h b/include/ui/rect.h
> new file mode 100644
> index 0000000000..94898f92d0
> --- /dev/null
> +++ b/include/ui/rect.h
> @@ -0,0 +1,59 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QEMU_RECT_H
> +#define QEMU_RECT_H
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +typedef struct QemuRect {
> +    int16_t x;
> +    int16_t y;
> +    uint16_t width;
> +    uint16_t height;
> +} QemuRect;
> +
> +static inline void qemu_rect_init(QemuRect *rect,
> +                                  int16_t x, int16_t y,
> +                                  uint16_t width, uint16_t height)
> +{
> +    rect->x = x;
> +    rect->y = x;
> +    rect->width = width;
> +    rect->height = height;
> +}
> +
> +static inline void qemu_rect_translate(QemuRect *rect,
> +                                       int16_t dx, int16_t dy)
> +{
> +    rect->x += dx;
> +    rect->y += dy;
> +}
> +
> +static inline bool qemu_rect_intersect(const QemuRect *a, const QemuRect *b,
> +                                       QemuRect *res)
> +{
> +    int16_t x1, x2, y1, y2;
> +
> +    x1 = MAX(a->x, b->x);
> +    y1 = MAX(a->y, b->y);
> +    x2 = MIN(a->x + a->width, b->x + b->width);
> +    y2 = MIN(a->y + a->height, b->y + b->height);
> +
> +    if (x1 >= x2 || y1 >= y2) {
> +        if (res) {
> +            qemu_rect_init(res, 0, 0, 0, 0);
> +        }
> +
> +        return false;
> +    }
> +
> +    if (res) {
> +        qemu_rect_init(res, x1, y1, x2 - x1, y2 - y1);
> +    }
> +
> +    return true;
> +}
> +
> +#endif

Maybe you could move these to pixman-minimal.h as well implementing the 
missing pixman_region funcs so no need to change the callers? But I don't 
mind either way.

Regards,
BALATON Zoltan

> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 4265316cbb..a8e767245f 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -16,6 +16,7 @@
> #include "qemu/iov.h"
> #include "sysemu/cpus.h"
> #include "ui/console.h"
> +#include "ui/rect.h"
> #include "trace.h"
> #include "sysemu/dma.h"
> #include "sysemu/sysemu.h"
> @@ -503,7 +504,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
>     struct virtio_gpu_simple_resource *res;
>     struct virtio_gpu_resource_flush rf;
>     struct virtio_gpu_scanout *scanout;
> -    pixman_region16_t flush_region;
> +    QemuRect flush_rect;
>     bool within_bounds = false;
>     bool update_submitted = false;
>     int i;
> @@ -565,34 +566,25 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
>         return;
>     }
>
> -    pixman_region_init_rect(&flush_region,
> -                            rf.r.x, rf.r.y, rf.r.width, rf.r.height);
> +    qemu_rect_init(&flush_rect, rf.r.x, rf.r.y, rf.r.width, rf.r.height);
>     for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> -        pixman_region16_t region, finalregion;
> -        pixman_box16_t *extents;
> +        QemuRect rect;
>
>         if (!(res->scanout_bitmask & (1 << i))) {
>             continue;
>         }
>         scanout = &g->parent_obj.scanout[i];
>
> -        pixman_region_init(&finalregion);
> -        pixman_region_init_rect(&region, scanout->x, scanout->y,
> -                                scanout->width, scanout->height);
> +        qemu_rect_init(&rect, scanout->x, scanout->y,
> +                       scanout->width, scanout->height);
>
> -        pixman_region_intersect(&finalregion, &flush_region, &region);
> -        pixman_region_translate(&finalregion, -scanout->x, -scanout->y);
> -        extents = pixman_region_extents(&finalregion);
>         /* work out the area we need to update for each console */
> -        dpy_gfx_update(g->parent_obj.scanout[i].con,
> -                       extents->x1, extents->y1,
> -                       extents->x2 - extents->x1,
> -                       extents->y2 - extents->y1);
> -
> -        pixman_region_fini(&region);
> -        pixman_region_fini(&finalregion);
> +        if (qemu_rect_intersect(&flush_rect, &rect, &rect)) {
> +            qemu_rect_translate(&rect, -scanout->x, -scanout->y);
> +            dpy_gfx_update(g->parent_obj.scanout[i].con,
> +                           rect.x, rect.y, rect.width, rect.height);
> +        }
>     }
> -    pixman_region_fini(&flush_region);
> }
>
> static void virtio_unref_resource(pixman_image_t *image, void *data)
>
Marc-André Lureau Oct. 26, 2023, 7:24 a.m. UTC | #2 
Hi

On Thu, Oct 26, 2023 at 12:55 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Wed, 25 Oct 2023, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Use a simpler implementation for rectangle geometry & intersect, drop
> > the need for (more complex) PIXMAN functions.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > include/ui/rect.h       | 59 +++++++++++++++++++++++++++++++++++++++++
> > hw/display/virtio-gpu.c | 30 ++++++++-------------
> > 2 files changed, 70 insertions(+), 19 deletions(-)
> > create mode 100644 include/ui/rect.h
> >
> > diff --git a/include/ui/rect.h b/include/ui/rect.h
> > new file mode 100644
> > index 0000000000..94898f92d0
> > --- /dev/null
> > +++ b/include/ui/rect.h
> > @@ -0,0 +1,59 @@
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#ifndef QEMU_RECT_H
> > +#define QEMU_RECT_H
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +typedef struct QemuRect {
> > +    int16_t x;
> > +    int16_t y;
> > +    uint16_t width;
> > +    uint16_t height;
> > +} QemuRect;
> > +
> > +static inline void qemu_rect_init(QemuRect *rect,
> > +                                  int16_t x, int16_t y,
> > +                                  uint16_t width, uint16_t height)
> > +{
> > +    rect->x = x;
> > +    rect->y = x;
> > +    rect->width = width;
> > +    rect->height = height;
> > +}
> > +
> > +static inline void qemu_rect_translate(QemuRect *rect,
> > +                                       int16_t dx, int16_t dy)
> > +{
> > +    rect->x += dx;
> > +    rect->y += dy;
> > +}
> > +
> > +static inline bool qemu_rect_intersect(const QemuRect *a, const QemuRect *b,
> > +                                       QemuRect *res)
> > +{
> > +    int16_t x1, x2, y1, y2;
> > +
> > +    x1 = MAX(a->x, b->x);
> > +    y1 = MAX(a->y, b->y);
> > +    x2 = MIN(a->x + a->width, b->x + b->width);
> > +    y2 = MIN(a->y + a->height, b->y + b->height);
> > +
> > +    if (x1 >= x2 || y1 >= y2) {
> > +        if (res) {
> > +            qemu_rect_init(res, 0, 0, 0, 0);
> > +        }
> > +
> > +        return false;
> > +    }
> > +
> > +    if (res) {
> > +        qemu_rect_init(res, x1, y1, x2 - x1, y2 - y1);
> > +    }
> > +
> > +    return true;
> > +}
> > +
> > +#endif
>
> Maybe you could move these to pixman-minimal.h as well implementing the
> missing pixman_region funcs so no need to change the callers? But I don't
> mind either way.
>

The pixman region functions are more complex than what we actually
need in QEMU (simple rectangle geometry).

> Regards,
> BALATON Zoltan
>
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 4265316cbb..a8e767245f 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -16,6 +16,7 @@
> > #include "qemu/iov.h"
> > #include "sysemu/cpus.h"
> > #include "ui/console.h"
> > +#include "ui/rect.h"
> > #include "trace.h"
> > #include "sysemu/dma.h"
> > #include "sysemu/sysemu.h"
> > @@ -503,7 +504,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
> >     struct virtio_gpu_simple_resource *res;
> >     struct virtio_gpu_resource_flush rf;
> >     struct virtio_gpu_scanout *scanout;
> > -    pixman_region16_t flush_region;
> > +    QemuRect flush_rect;
> >     bool within_bounds = false;
> >     bool update_submitted = false;
> >     int i;
> > @@ -565,34 +566,25 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
> >         return;
> >     }
> >
> > -    pixman_region_init_rect(&flush_region,
> > -                            rf.r.x, rf.r.y, rf.r.width, rf.r.height);
> > +    qemu_rect_init(&flush_rect, rf.r.x, rf.r.y, rf.r.width, rf.r.height);
> >     for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> > -        pixman_region16_t region, finalregion;
> > -        pixman_box16_t *extents;
> > +        QemuRect rect;
> >
> >         if (!(res->scanout_bitmask & (1 << i))) {
> >             continue;
> >         }
> >         scanout = &g->parent_obj.scanout[i];
> >
> > -        pixman_region_init(&finalregion);
> > -        pixman_region_init_rect(&region, scanout->x, scanout->y,
> > -                                scanout->width, scanout->height);
> > +        qemu_rect_init(&rect, scanout->x, scanout->y,
> > +                       scanout->width, scanout->height);
> >
> > -        pixman_region_intersect(&finalregion, &flush_region, &region);
> > -        pixman_region_translate(&finalregion, -scanout->x, -scanout->y);
> > -        extents = pixman_region_extents(&finalregion);
> >         /* work out the area we need to update for each console */
> > -        dpy_gfx_update(g->parent_obj.scanout[i].con,
> > -                       extents->x1, extents->y1,
> > -                       extents->x2 - extents->x1,
> > -                       extents->y2 - extents->y1);
> > -
> > -        pixman_region_fini(&region);
> > -        pixman_region_fini(&finalregion);
> > +        if (qemu_rect_intersect(&flush_rect, &rect, &rect)) {
> > +            qemu_rect_translate(&rect, -scanout->x, -scanout->y);
> > +            dpy_gfx_update(g->parent_obj.scanout[i].con,
> > +                           rect.x, rect.y, rect.width, rect.height);
> > +        }
> >     }
> > -    pixman_region_fini(&flush_region);
> > }
> >
> > static void virtio_unref_resource(pixman_image_t *image, void *data)
> >
diff mbox series

Patch

diff --git a/include/ui/rect.h b/include/ui/rect.h
new file mode 100644
index 0000000000..94898f92d0
--- /dev/null
+++ b/include/ui/rect.h
@@ -0,0 +1,59 @@ 
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QEMU_RECT_H
+#define QEMU_RECT_H
+
+#include <stdint.h>
+#include <stdbool.h>
+
+typedef struct QemuRect {
+    int16_t x;
+    int16_t y;
+    uint16_t width;
+    uint16_t height;
+} QemuRect;
+
+static inline void qemu_rect_init(QemuRect *rect,
+                                  int16_t x, int16_t y,
+                                  uint16_t width, uint16_t height)
+{
+    rect->x = x;
+    rect->y = x;
+    rect->width = width;
+    rect->height = height;
+}
+
+static inline void qemu_rect_translate(QemuRect *rect,
+                                       int16_t dx, int16_t dy)
+{
+    rect->x += dx;
+    rect->y += dy;
+}
+
+static inline bool qemu_rect_intersect(const QemuRect *a, const QemuRect *b,
+                                       QemuRect *res)
+{
+    int16_t x1, x2, y1, y2;
+
+    x1 = MAX(a->x, b->x);
+    y1 = MAX(a->y, b->y);
+    x2 = MIN(a->x + a->width, b->x + b->width);
+    y2 = MIN(a->y + a->height, b->y + b->height);
+
+    if (x1 >= x2 || y1 >= y2) {
+        if (res) {
+            qemu_rect_init(res, 0, 0, 0, 0);
+        }
+
+        return false;
+    }
+
+    if (res) {
+        qemu_rect_init(res, x1, y1, x2 - x1, y2 - y1);
+    }
+
+    return true;
+}
+
+#endif
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4265316cbb..a8e767245f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -16,6 +16,7 @@ 
 #include "qemu/iov.h"
 #include "sysemu/cpus.h"
 #include "ui/console.h"
+#include "ui/rect.h"
 #include "trace.h"
 #include "sysemu/dma.h"
 #include "sysemu/sysemu.h"
@@ -503,7 +504,7 @@  static void virtio_gpu_resource_flush(VirtIOGPU *g,
     struct virtio_gpu_simple_resource *res;
     struct virtio_gpu_resource_flush rf;
     struct virtio_gpu_scanout *scanout;
-    pixman_region16_t flush_region;
+    QemuRect flush_rect;
     bool within_bounds = false;
     bool update_submitted = false;
     int i;
@@ -565,34 +566,25 @@  static void virtio_gpu_resource_flush(VirtIOGPU *g,
         return;
     }
 
-    pixman_region_init_rect(&flush_region,
-                            rf.r.x, rf.r.y, rf.r.width, rf.r.height);
+    qemu_rect_init(&flush_rect, rf.r.x, rf.r.y, rf.r.width, rf.r.height);
     for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
-        pixman_region16_t region, finalregion;
-        pixman_box16_t *extents;
+        QemuRect rect;
 
         if (!(res->scanout_bitmask & (1 << i))) {
             continue;
         }
         scanout = &g->parent_obj.scanout[i];
 
-        pixman_region_init(&finalregion);
-        pixman_region_init_rect(&region, scanout->x, scanout->y,
-                                scanout->width, scanout->height);
+        qemu_rect_init(&rect, scanout->x, scanout->y,
+                       scanout->width, scanout->height);
 
-        pixman_region_intersect(&finalregion, &flush_region, &region);
-        pixman_region_translate(&finalregion, -scanout->x, -scanout->y);
-        extents = pixman_region_extents(&finalregion);
         /* work out the area we need to update for each console */
-        dpy_gfx_update(g->parent_obj.scanout[i].con,
-                       extents->x1, extents->y1,
-                       extents->x2 - extents->x1,
-                       extents->y2 - extents->y1);
-
-        pixman_region_fini(&region);
-        pixman_region_fini(&finalregion);
+        if (qemu_rect_intersect(&flush_rect, &rect, &rect)) {
+            qemu_rect_translate(&rect, -scanout->x, -scanout->y);
+            dpy_gfx_update(g->parent_obj.scanout[i].con,
+                           rect.x, rect.y, rect.width, rect.height);
+        }
     }
-    pixman_region_fini(&flush_region);
 }
 
 static void virtio_unref_resource(pixman_image_t *image, void *data)