hi Martin (resend, I forgot to CC mailing list) On Sun, Jan 08, 2012 at 12:04:25PM -0600, Martin Franco wrote: > On Sun, Jan 08, 2012 at 11:14:44AM +0100, Martin Renold wrote: > > <snip> > > Maybe mirror paint should > > be a property of the surface (tiledsurface.hpp), instead of the brush class? > Attached is a patch implementing symmetry in draw_dab instead of > stroke_to
Thanks! Yes I think that's better. I'm somewhat sceptical about the use of the global variables. But it's okay with me, I don't really have a good alternative to propose right now. And symmetrical painting is fun :-) I take back my comment that get_color() should average the result with the symmetrical dab. I have actually tried this case now (a smudging brush with non-symmetrical background strokes) and to my surprise I think the actual behaviour is expected and useful: It picks up the color from where the cursor is, and mirrors it to the other side of the stroke. > > <snip> > > The "dab_angle" parameter should also be mirrored. And this is a good time > > to think about a fix for https://gna.org/bugs/?17400 > angle is now mirrored Good. > and second patch fixes canvas rotation rotates brush. This patch depends > on the first patch to make symmetry mode work with rotation. I appreciate the effort. After reading your patch, I was even expecting it to work :-) But there is a problem: brushlib calculates the spacing of the dabs. For an elliptical dab, the dabs are spaced much more tightly along the short axis. However your patch only rotates the final dab, while brushlib still does the spacing as if the dab wasn't rotated. The problem is easy to see with the classic/calligraphy brush. Comments about your first patch: > --- a/gui/tileddrawwidget.py > +++ b/gui/tileddrawwidget.py > @@ -96,6 +96,7 @@ class TiledDrawWidget(gtk.DrawingArea): > + self.symmetrical = False > > @@ -621,6 +622,9 @@ class TiledDrawWidget(gtk.DrawingArea): > > + def symmetry(self): > + self.symmetrical = > self.doc.layer._surface.toggle_symmetry(self.get_allocation()[2]/2) > + Looks to me like tdw.symmetrical is set, but never used. Remove? And from reading only the code above, it is very unexpected that this call affects all surface classes, and not just this instance. If you keep this global behaviour, please label it clearly somehow, eg. rename the method to global_toggle_symmetry() or similar. > + int toggle_symmetry(float axis = 0) { > + surface_do_symmetry = !surface_do_symmetry; > + surface_center_x = axis; > + return surface_do_symmetry; > + } Looks like toggle_symmetry() is never called without argument, so please remove the default value. Appart from that, it looks good to me. -- Martin Renold _______________________________________________ Mypaint-discuss mailing list [email protected] https://mail.gna.org/listinfo/mypaint-discuss
