Is it correct though? We'll keep adding repaint regions even if compositing is disabled.

Yea, shouldn't be an issue. We can add as many we like and when compositing gets enabled again we do a repaint of the accumulated region (probably we do a full repaint then anyway).

It's better to avoid unnecessary work if compositing is disabled. Many parts of KWin call Compositor::addRepaint() without checking whether compositing is active, although some parts check.

Please add this check back.

It's a matter of personal taste, but maybe if statements should check only related stuff(i.e. don't inflate conditionals). Whether compositing is active and whether the composite timer is active seem to be unrelated.

Also, this will be a great idea to document when the composite timer condition can be fulfilled.

if (m_state != State::On) {
    return;
}

// This if statements checks something completely different, thus it's not
// merged with the previous if statement.
if (compositeTimer.isActive()) {
    return;
}

Compositing is most often active nowadays. On Wayland always. I don't even understand how this can be a point of discussion.

Yes, but the compositing still can be off, for example, when you're playing a game that blocks compositing.

So what we do more in this case is a single region addtion to repaints_region, right?

Yes, we'll be adding regions for no reason. Even though we don't hit this if statement in 99%, 1% is still there.

I'm all for mergin setCompositeTimer and scheduleRepaint methods, but I think we have to keep this check until X11 support is dropped.