Re: Wacom Calibration - Phase 1

[Bastien Nocera <hadess hadess net>]

On Mon, 2012-01-09 at 10:18 -0800, Jason Gerecke wrote:
> On Mon, Jan 9, 2012 at 7:52 AM, Bastien Nocera <hadess hadess net> wrote:
> > On Mon, 2012-01-09 at 15:27 +0000, Bastien Nocera wrote:
> >> Hey Jason,
> >>
> >> On Wed, 2012-01-04 at 18:11 -0800, Jason Gerecke wrote:
> >> > (Moved to the gnomecc-list)
> >> >
> >> > The first of three phases on adding calibration into the Wacom panel
> >> > is now "complete". That is to say, the code is working, but not
> >> > necessarily pretty! I'd be most appreciative if anyone could review
> >> > the changes and give me some feedback. The changes to g-c-c itself are
> >> > pretty minor, but I don't doubt I've done a couple of stupid things :D
> >> >
> >> > The "calibration" branch at
> >> > https://github.com/jigpu/gnome-control-center has the changes to
> >> > g-c-c. If you want to see the gory changeset for xinput_calibrator
> >> > (instead of just the commit in g-c-c where I copy the files over),
> >> > it's available in the "stripped" branch of
> >> > https://github.com/jigpu/xinput_calibrator Note that these are
> >> > development branches -- I'll be rebasing and changing history as I
> >> > please ;) Work on phase 2 (multiple monitor support) begins tomorrow!
> >>
> >> Could you please upload 2 separate patches to the GNOME Bugzilla, one
> >> mega-patch that adds the calibrator to the tree, and one with the panel
> >> integration code?
> >>
> >> I'll review both and merge as soon as possible, so that we can carry on
> >> working upstream on this.
> >
> > I'll add that:
> > - the calibrator code needs to be moved into a sub directory
> > - the horror that is run_gui() needs to be changed, we can't have it
> > blocking the UI
> The mockup struck me as being modal, so I wrote run_gui under that
> assumption. If its not supposed to be modal (not sure what use that
> would serve, but then again, that may be part of why I suck at writing
> GUIs) I can see how that'd be a problem.

If you're in dual-head, there's no reason why the calibration part of
the UI to be on the screen as the toolkit. Blocking the shell is not an
option.

> > - the copy/pasted code is in severe need of a clean up
> I'll agree, but I need a little more direction than that :D Are we
> talking cleaner (i.e. more GNOME-like) style? Cleaner (i.e.
> simplified) functions? Cleaner (i.e. unapplicable functions removed)
> files? I've lost most of my perspective of what needs attention after
> mucking through the trenches with the C++ to C conversion.

Removing unused code would be nice. I'm sure there's a number of places
where we can use GTK+ or GDK function to do the work.

> > - the code in cc-wacom-page.c needs to use the same coding style
> > (indentation) as the rest of the existing code
> > - s/axys/axis/
> >
> > Cheers
> >
> 
> Do you want me to make these changes before uploading the patches, or
> after they're upstream? I'm all for getting the code upstream as
> quickly as possible (so some of this work can be parallelized and not
> block on my learning curve), but I'm not sure how cooked you want the
> patches.

Move the calibrator code to separate directory, and attach the patches,
I'll clean them up as I review them.

Cheers