[wayland] Ensure that repaints_region is in frame-local coordinates
ClosedPublic

Authored by zzag on Oct 7 2019, 10:43 AM.

Details

Summary

The repaints region is in frame-local coordinates, i.e. relative to the
top-left corner of the frame geometry.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zzag created this revision.Oct 7 2019, 10:43 AM
Restricted Application added a project: KWin. · View Herald TranscriptOct 7 2019, 10:43 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Oct 7 2019, 10:43 AM
romangg accepted this revision.Oct 29 2019, 3:23 PM
This revision is now accepted and ready to land.Oct 29 2019, 3:23 PM

When we call this line:

connect(m_surface, &SurfaceInterface::damaged, this, &Toplevel::addDamage);

TopLevel::addDamage is relative to buffer

When we call

Toplevel::addDamage(damage);

TopLevel::addDamage is relative to the frame

So that's a bit messy. We can ship this for now, but I think we should do the manipulation elsewhere.

zzag added a comment.Nov 13 2019, 5:37 PM

When we call

Toplevel::addDamage(damage);

TopLevel::addDamage is relative to the frame

Correction: it's relative to buffer.

Let me try and explain my point again.

In one invocation of TopLevel::addDamage the argument is relative to the buffer (when we connect events from the wayland surface to here)
In another invocation of TopLevel::addDamage the argument is relative to the frame (when we're in the overridden implementation calling the base implementation)

I am saying having one method defined where the argument semantics change depend on where it's called from is confusing.

(That was the state before this patch too, I was just commenting on it whilst I saw it... )

zzag added a comment.Nov 13 2019, 6:36 PM

Let me try and explain my point again.

[...]

In another invocation of TopLevel::addDamage the argument is relative to the frame (when we're in the overridden implementation calling the base implementation)

Hmm, I don't follow you. The region that we pass to Toplevel::addDamaged() is always in buffer-local a.k.a. surface-local coordinates, not in frame-local coordinates.

This revision was automatically updated to reflect the committed changes.