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

[Manusheel Gupta]

James,

Thanks for the timely feedback.


Ayush and team members,

Please make sure that we communicate clearly on list servs on our
implementation plans and strategies before we submit patches. Will improve
productivity and clarity of work.

Regards,

Manu



On Thu, Oct 21, 2010 at 7:38 AM, 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.  ;-)
>
> > +        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/
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20101021/7798e58a/attachment.html>