Re: [PATCH weston 2/2] compositor-drm: the cursor plane only supports ARGB
On Fri, Feb 05, 2016 at 02:30:22PM +0200, Pekka Paalanen wrote: > On Thu, 3 Dec 2015 14:07:12 -0600 > Derek Foreman wrote: > > > Keep XRGB apps out of the cursor plane, only ARGB is supported. > > > > This prevents programs like weston-simple-shm from landing in the cursor > > plane and being misrendered. > > > > Signed-off-by: Derek Foreman > > --- > > src/compositor-drm.c | 12 +--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > > index a6db933..84105e1 100644 > > --- a/src/compositor-drm.c > > +++ b/src/compositor-drm.c > > @@ -1094,6 +1094,7 @@ drm_output_prepare_cursor_view(struct drm_output > > *output, > > struct drm_backend *b = > > (struct drm_backend *)output->base.compositor->backend; > > struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport; > > + struct wl_shm_buffer *shmbuf; > > > > if (ev->transform.enabled && > > (ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE)) > > @@ -1112,9 +1113,14 @@ drm_output_prepare_cursor_view(struct drm_output > > *output, > > return NULL; > > if (ev->geometry.scissor_enabled) > > return NULL; > > - if (ev->surface->buffer_ref.buffer == NULL || > > - !wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) || > > - ev->surface->width > b->cursor_width || > > + if (ev->surface->buffer_ref.buffer == NULL) > > + return NULL; > > + shmbuf = wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource); > > + if (!shmbuf) > > + return NULL; > > + if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB) > > + return NULL; > > + if (ev->surface->width > b->cursor_width || > > ev->surface->height > b->cursor_height) > > return NULL; > > > > Reviewed-by: Pekka Paalanen > > Also tested on intel SNB. > > This will also prevent RGB565 surfaces going to the cursor plane, and > cursor_bo_update() would just screw that up totally because it has a > hardcoded "ev->surface->width * 4" in memcpy(). > > Not to mention that using surface dimensions for buffer access is... > not good. drm_output_prepare_cursor_view() is accounting for > buffer/output_scale and scissor, but it's missing viewport. It's also > not checking buffer_transform. So if someone happened to use > wl_viewport or buffer_transform on a small shm surface, we'd get funny > stuff. But that's another matter, just noticed it while reviewing. Thanks, pushed for rc1: To ssh://git.freedesktop.org/git/wayland/weston b042756..6c19b69 master -> master ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/2] compositor-drm: the cursor plane only supports ARGB
On Thu, 3 Dec 2015 14:07:12 -0600 Derek Foreman wrote: > Keep XRGB apps out of the cursor plane, only ARGB is supported. > > This prevents programs like weston-simple-shm from landing in the cursor > plane and being misrendered. > > Signed-off-by: Derek Foreman > --- > src/compositor-drm.c | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > index a6db933..84105e1 100644 > --- a/src/compositor-drm.c > +++ b/src/compositor-drm.c > @@ -1094,6 +1094,7 @@ drm_output_prepare_cursor_view(struct drm_output > *output, > struct drm_backend *b = > (struct drm_backend *)output->base.compositor->backend; > struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport; > + struct wl_shm_buffer *shmbuf; > > if (ev->transform.enabled && > (ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE)) > @@ -1112,9 +1113,14 @@ drm_output_prepare_cursor_view(struct drm_output > *output, > return NULL; > if (ev->geometry.scissor_enabled) > return NULL; > - if (ev->surface->buffer_ref.buffer == NULL || > - !wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) || > - ev->surface->width > b->cursor_width || > + if (ev->surface->buffer_ref.buffer == NULL) > + return NULL; > + shmbuf = wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource); > + if (!shmbuf) > + return NULL; > + if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB) > + return NULL; > + if (ev->surface->width > b->cursor_width || > ev->surface->height > b->cursor_height) > return NULL; > Reviewed-by: Pekka Paalanen Also tested on intel SNB. This will also prevent RGB565 surfaces going to the cursor plane, and cursor_bo_update() would just screw that up totally because it has a hardcoded "ev->surface->width * 4" in memcpy(). Not to mention that using surface dimensions for buffer access is... not good. drm_output_prepare_cursor_view() is accounting for buffer/output_scale and scissor, but it's missing viewport. It's also not checking buffer_transform. So if someone happened to use wl_viewport or buffer_transform on a small shm surface, we'd get funny stuff. But that's another matter, just noticed it while reviewing. Thanks, pq pgpdghe1oGdxK.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 2/2] compositor-drm: the cursor plane only supports ARGB
Keep XRGB apps out of the cursor plane, only ARGB is supported. This prevents programs like weston-simple-shm from landing in the cursor plane and being misrendered. Signed-off-by: Derek Foreman --- src/compositor-drm.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/compositor-drm.c b/src/compositor-drm.c index a6db933..84105e1 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -1094,6 +1094,7 @@ drm_output_prepare_cursor_view(struct drm_output *output, struct drm_backend *b = (struct drm_backend *)output->base.compositor->backend; struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport; + struct wl_shm_buffer *shmbuf; if (ev->transform.enabled && (ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE)) @@ -1112,9 +1113,14 @@ drm_output_prepare_cursor_view(struct drm_output *output, return NULL; if (ev->geometry.scissor_enabled) return NULL; - if (ev->surface->buffer_ref.buffer == NULL || - !wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) || - ev->surface->width > b->cursor_width || + if (ev->surface->buffer_ref.buffer == NULL) + return NULL; + shmbuf = wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource); + if (!shmbuf) + return NULL; + if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB) + return NULL; + if (ev->surface->width > b->cursor_width || ev->surface->height > b->cursor_height) return NULL; -- 2.6.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
