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

[Ayush Goyal]

  James,

I am sorry for the confusion over the OLPC#2495 & SL#2463 bug issue i 
should also have contacted u but instead i made most of the changes u 
suggested and uploaded them. Also i uploaded some errors lat time it 
must have created some confusion.

Regarding the changes you are asking me to make in implementation of the 
effects separately for applying effect over a selection and for entire 
image i have referenced the functions for effect already present and 
used that as guideline to make changes in code. I have just started 
developing and i still hv much to learn about efficient coding for these 
issues so please pardon my mistakes. I will try my best to code more 
efficiently in future and thanks for reviewing the patch.

Regards,
Ayush




On 10/21/2010 07:38 AM, James Cameron 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.
>