[Sugar-devel] [PATCH Paint activity v2] Implemented Mirroring Effect in Paint Activity (SL#2463)

[James Cameron]

On Wed, Oct 20, 2010 at 08:00:16PM +0530, Ayush Goyal wrote:
> +        if self.selmove:
> +            size = self.pixmap_sel.get_size()
> +            pix = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, False, 8,
> +                size[0], size[1])
> +            pix.get_from_drawable(self.pixmap_sel,
> +                gtk.gdk.colormap_get_system(), 0, 0, 0, 0, size[0], size[1])
> +        else:
> +            pix = gtk.gdk.Pixbuf(gtk.gdk.COLORSPACE_RGB, False, 8,
> +                width, height)
> +            pix.get_from_drawable(self.pixmap, gtk.gdk.colormap_get_system(),
> +                0, 0, 0, 0, width, height)
> +
> +            pix = pix.flip(horizontal)

Are you sure this version of the patch works for you for both selected
area and whole image?  The pix.flip is only on the whole image branch of
the if.  Either this version of the patch does not work, or you did not
test it, or I misunderstand.  Lay odds.  ;-)

> +        if self.selmove:
> +            self.pixmap_sel.draw_pixbuf(self.gc, pix, 0, 0, 0, 0,
> +                size[0], size[1], dither=gtk.gdk.RGB_DITHER_NORMAL,
> +                x_dither=0, y_dither=0)
> +
> +            self.pixmap_temp.draw_drawable(self.gc, self.pixmap, 0, 0, 0, 0,
> +                width, height)
> +            self.pixmap_temp.draw_drawable(self.gc, self.pixmap_sel,
> +                0, 0, self.orig_x, self.orig_y, size[0], size[1])
> +            self.pixmap_temp.draw_rectangle(self.gc_selection, False,
> +                self.orig_x, self.orig_y, size[0], size[1])
> +            self.pixmap_temp.draw_rectangle(self.gc_selection1, False,
> +                self.orig_x - 1, self.orig_y - 1, size[0] + 2, size[1] + 2)
> +
> +        else:
> +            self.pixmap.draw_pixbuf(self.gc, pix, 0, 0, 0, 0, width, height,
> +                dither=gtk.gdk.RGB_DITHER_NORMAL, x_dither=0, y_dither=0)
> +
> +        self.queue_draw()
> +        if not self.selmove:
> +            self.enableUndo(widget)

I still think there need only be one check for self.selmove, and
duplicated calls for pix.flip and queue_draw in each branch.  Whether
this is done as a set of different functions or not is up to you ...

... but I'm frustrated in this communication with you because I suggest
n changes and see only a patch come back with n-m changes, with no
discussion of why m changes were not adopted.

-- 
James Cameron
http://quozl.linux.org.au/