Re: [g-a-devel]gnome-mag IDL

[Bill Haneman <bill haneman sun com>]

On Mon, 2002-09-16 at 15:28, Michael Meeks wrote:

> 	First of all; I'd bin the 'out long x1, y1, x2, y2' business,
> completely;
> 
> 	I would personally have a:
> 
> struct Dimensions {
> 	long x, y;
> 	long width, height;
> };

Will think about this...

> 	And pass that to all the methods; it's marginally more efficient, and
> better expresses what is going on to my mind; also it can be used to
> provide properties of the same name:
> 
> >     /**
> >      * short createZoomRegion:
> >      * Creates a new zoom region for the magnifier.
> >      * The new region is initially unmanaged'.
> >      * @zx: the scale factor in the x direction for the new zoom region
> >      * @zy: the scale factor in the y direction for the new zoom region
> >      * @x1: the minimum X coordinate of the zoomed area bounding box
> >      * @x2: the maximum X coordinate of the zoomed area bounding box
> >      * @y1: the minimum Y coordinate of the zoomed area bounding box
> >      * @y2: the maximum Y coordinate of the zoomed area bounding box
> >      **/
> >     short createZoomRegion (in float zx, in float zy,
> > 			    in long x1, in long y1, 
> > 			    in long x2, in long y2); 
> 
> 	This looks like a method to set the ROI / bounding area of the zoom
> window, in which case there is a duplicate method method on the
> zoomRegion for one of these; I'd tend to avoid duplicating only part of
> that code. - Perhaps I misunderstood setROI / resizeWhatnot ...

Not quite, the bounds specified in the 'create' call specify the
viewports on the destination (target) display, not the ROIs from the
source display.  I will update the docs to make that clearer.
 
> 	I think the indexing thing is probably a mistake; I'd just do:
> 
> 	ZoomRegion addZoomRegion ();
> 
> 	But I'd also have a :
> 
> 	typedef sequence<ZoomRegion> ZoomRegionList;
> 
> 	ZoomRegionList getZoomRegions ();
> 
> 	method as well ...

Makes sense, thanks.
 
> >     /** 
> >      * void exit: 
> >      * Unmap the current magnifier from the display.
> >      **/
> 
> 	I think it's best to use the method name 'Dispose' to dispose this, (
> and the ZoomRegion ) interfaces.

dispose() for ZoomRegion sounds good, I guess it's harmless to use this
convention for Magnifier also.
 
> 	Indeed; I'd very much lika a 'Bonobo_Disposable' interface inside
> libbonobo, if you'd like to hack it up that way [ since this is 2.2 work
> yes ? ].

This work is "for immediate consumption", BAUM is blocking on it.  Of
course it's for HEAD and not the gnome-2-0 branch; though with this much
API churn I question whether the 2-0 branch of gnome-mag is still
useful.

> 
> >     void setMagFactor (in float magX, in float magY);
> 
> 	Again; I'd have a struct here, to make it cleaner, and so we can have a
> MagFactor property in the bag, that takes the same struct. [ indeed why
> expose these set/gets at all ? ].
> 
> >     oneway void setROI (in long x1, in long y1, in long x2, in long y2);

I think MagFactor and ROI are so fundamental to the idea of ZoomRegions
that it makes sense to expose them directly.
 
> 
> >     /**
> >      * oneway void setROI:
> ...
> >     void markDirty (in long x1, in long y1, in long x2, in long y2); 
> 
> 	cut and paste error on the copy ;-)

and an ancient one at that ;-)
 
> >     void destroyZoomRegion ();
> 
> 	dispose is a good name again.

I meant just "destroy()", but "dispose()" is fine.

thanks,

-Bill