Code review comment for lp:~mc-return/compiz/compiz.merge-fix1169353-screenshot-needs-color-options

Revision history for this message
MC Return (mc-return) wrote :

> Hrm, I don't think the way you've fixed this is really correct:
>

> The whole idea of those two damageRegion () calls is to mark the *old*
> selection rectangle region as damaged and then mark the new selection region
> as damaged, so that the damage area covers the selection area.
>
> This change does that *and* damages the whole screen every time there is a
> motion event, regardless of whether or not screenshot even has an active grab
> or the mouse cursor position actually changed.
>
> If the damage code isn't working, then you should look into why the current
> region-based damage isn't working rather than damaging the whole screen every
> time there is a motion event.

You are right -> the fix is not entirely correct. The cScreen->damageScreen ();
call should be removed completely as we do never need to damage more than the
actual selection...

I'll fix that immediately.

« Back to merge proposal