D26171: Implement wp_viewporter

[David Edmundson]

davidedmundson added a comment.


  Cool, I like viewporter.

INLINE COMMENTS

> surface_interface.cpp:464
> +
> +        sizeChanged |= (bool)buffer;
> +    }

C-style casts are frowned upon.

> surface_interface.cpp:485
> +                if (!QRectF(QPointF(), bufferSize).contains(source->sourceRectangle)) {
> +                    wl_resource_post_error(viewport->parentResource(),
> +                                           WP_VIEWPORT_ERROR_OUT_OF_BUFFER,

We're not testing this error in the case of:

buffer is set
viewporter.sourceRect is set

then later
buffer changes
viewporter.sourceRect remains the same

this should be an error too.

> surface_interface.cpp:882
> +    if (d->current.sourceRectangle.isValid()) {
> +        return d->current.sourceRectangle.size().toSize();
> +    }

Should this be

d->current.sourceRectangle.size()/ scale()

It would be a somewhat weird case to use both viewporter and wl_buffer.scale, but it's what the spec implies and you seem to be assuming that in the kwin render path.

destinationSize would be as-is.

> romangg wrote in surface_interface.h:130
> Needed for a call in a const function in KWin. Do we want to do it like that? Other suggestions?

Why not just

BufferInterface *buffer() const

There's no reason why a caller would need to explicitly pick a const vs non-const version.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D26171

To: romangg, #kwin
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191223/d5b02fc1/attachment.html>