On Thu, 5 Jul 2018 18:16:35 +0100 Daniel Stone <[email protected]> wrote:
> Use the new helper to populate the cursor state as well, with some > special-case handling to account for how we always upload a full-size > BO. > > As this now fully takes care of buffer transformations, HiDPI client > cursors work, and we also clip the cursor plane completely to CRTC > bounds. > > Signed-off-by: Daniel Stone <[email protected]> > Reported-by: Derek Foreman <[email protected]> > Tested-by: Emre Ucan <[email protected]> > Fixes: https://gitlab.freedesktop.org/wayland/weston/issues/118 > --- > libweston/compositor-drm.c | 68 ++++++++++++++++++-------------------- > 1 file changed, 32 insertions(+), 36 deletions(-) The subject should say plane_state_coords_for_view. > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 2927a0ebf..9aab6e523 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -2841,14 +2841,14 @@ err: > /** > * Update the image for the current cursor surface > * > - * @param b DRM backend structure > - * @param bo GBM buffer object to write into > - * @param ev View to use for cursor image > + * @param plane_state DRM cursor plane state > + * @param ev Source view for cursor > */ > static void > -cursor_bo_update(struct drm_backend *b, struct gbm_bo *bo, > - struct weston_view *ev) > +cursor_bo_update(struct drm_plane_state *plane_state, struct weston_view *ev) > { > + struct drm_backend *b = plane_state->plane->backend; > + struct gbm_bo *bo = plane_state->fb->bo; > struct weston_buffer *buffer = ev->surface->buffer_ref.buffer; > uint32_t buf[b->cursor_width * b->cursor_height]; > int32_t stride; > @@ -2857,18 +2857,16 @@ cursor_bo_update(struct drm_backend *b, struct gbm_bo > *bo, > > assert(buffer && buffer->shm_buffer); > assert(buffer->shm_buffer == wl_shm_buffer_get(buffer->resource)); > - assert(ev->surface->width <= b->cursor_width); > - assert(ev->surface->height <= b->cursor_height); Rather than removing these asserts, it would be better to fix them to use buffer size. It would catch overflowing buf. An assert to ensure bytes-per-pixel is the hardcoded four below would be a nice addition. > > memset(buf, 0, sizeof buf); > stride = wl_shm_buffer_get_stride(buffer->shm_buffer); > s = wl_shm_buffer_get_data(buffer->shm_buffer); > > wl_shm_buffer_begin_access(buffer->shm_buffer); > - for (i = 0; i < ev->surface->height; i++) > + for (i = 0; i < buffer->height; i++) > memcpy(buf + i * b->cursor_width, > s + i * stride, > - ev->surface->width * 4); > + buffer->width * 4); > wl_shm_buffer_end_access(buffer->shm_buffer); > > if (gbm_bo_write(bo, buf, sizeof buf) < 0) > @@ -2883,10 +2881,8 @@ drm_output_prepare_cursor_view(struct drm_output_state > *output_state, > struct drm_backend *b = to_drm_backend(output->base.compositor); > struct drm_plane *plane = output->cursor_plane; > struct drm_plane_state *plane_state; > - struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport; > struct wl_shm_buffer *shmbuf; > bool needs_update = false; > - float x, y; > > if (!plane) > return NULL; > @@ -2916,26 +2912,25 @@ drm_output_prepare_cursor_view(struct > drm_output_state *output_state, > if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB8888) > return NULL; > > - if (output->base.transform != WL_OUTPUT_TRANSFORM_NORMAL) > - return NULL; > - if (ev->transform.enabled && > - (ev->transform.matrix.type > WESTON_MATRIX_TRANSFORM_TRANSLATE)) > - return NULL; > - if (viewport->buffer.scale != output->base.current_scale) > - return NULL; > - if (ev->geometry.scissor_enabled) > - return NULL; > - > - if (ev->surface->width > b->cursor_width || > - ev->surface->height > b->cursor_height) > - return NULL; > - I'm happy to see checks like above... > plane_state = > drm_output_state_get_plane(output_state, output->cursor_plane); > > if (plane_state && plane_state->fb) > return NULL; > > + /* We can't scale with the legacy API, and we don't try to account for > + * simple cropping/translation in cursor_bo_update. */ > + plane_state->output = output; > + if (!drm_plane_state_coords_for_view(plane_state, ev)) > + goto err; > + > + if (plane_state->src_x != 0 || plane_state->src_y != 0 || > + plane_state->src_w > (unsigned) b->cursor_width << 16 || > + plane_state->src_h > (unsigned) b->cursor_height << 16 || > + plane_state->src_w != plane_state->dest_w << 16 || > + plane_state->src_h != plane_state->dest_h << 16) > + goto err; > + ...are being replaced with checks like this. It's a step towards inspecting the full end-to-end transformation. Looks good, so in any case: Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq
pgpwLlygyUgYe.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
