davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.
I've written you a unit test.
I can either take over this or upload as a separate review that you can merge
in.
INLINE COMMENTS
> zzag wrote in surface_interface.cpp:819
> So, it would be fine to return `true` in the following case:
>
> QRectF(QPoint(0, 0), QSize(10, 10)).contains(QPointF(10, 10));
>
> ?
I would have said so.
But then it means sizeContains and inputContains behave differently as
QRegion(QRect(0,0,10,10)).contains(10,10); returns false
which gives us some inconsistency.
> surface_interface.cpp:824
> {
> - if (!isMapped()) {
> + return sizeContains(size, QRegion(), position) &&
> input.contains(position.toPoint());
> +}
This needs to be
&& (surface.inputIsInfinite() || input.contains(..))
which means either an extra arg in our function pointer or go back to the
frankly simpler first revision. Writing N complex lines to save duplicating N
simple lines doesn't make much sense to me, especially when they end up
deviating.
REPOSITORY
R127 KWayland
REVISION DETAIL
https://phabricator.kde.org/D7038
To: romangg, #frameworks, graesslin, davidedmundson
Cc: davidedmundson, zzag, kde-frameworks-devel, graesslin, plasma-devel,
ragreen, Pitel, schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin,
lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
