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

[Gonzalo Odiard]

On Wed, Oct 20, 2010 at 11:08 PM, James Cameron <quozl at laptop.org> wrote:

> 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.  ;-)
>
>
Sorry, I detected it, and Ayush send a new patch.


> > +        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.
>
>
I appreciate your reviews and are a great help.
The code in Paint have a lot of bad examples, and the new developers are
taking it like reference.
I am trying to resolve the major issues, but can't do a major refactoring
now.


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



-- 
Gonzalo Odiard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20101020/5ed31b8d/attachment.html>