Re: [g-a-devel]impl_getBoundedRanges

[Bill Haneman <bill haneman sun com>]

On Wed, 2003-01-15 at 11:46, Padraig O'Briain wrote:
> Bill,
> 
> I have looked at this more closely and have attached an updated patch.
> 
> I think this addresses the points you raised.

Yes, that's it, thanks.  Please commit.

-Bill

> Padraig
> 
> > Subject: Re: [g-a-devel]impl_getBoundedRanges
> > To: "Padraig O'Briain" <Padraig Obriain sun com>
> > Cc: gnome-accessibility-devel gnome org
> > Mime-Version: 1.0
> > Content-Transfer-Encoding: 7bit
> > 
> > On Tue, 2003-01-14 at 14:40, Bill Haneman wrote:
> > > On Tue, 2003-01-14 at 14:30, Padraig O'Briain wrote:
> > > > Bill,
> > > > 
> > > > I have reviewed this patch and found that the change in the while 
> statements 
> > > > from < to <= is not necessary,
> > > > 
> > > > Updated patch is attached.
> > > 
> > > Thanks Padraig, this version of the patch looks good.  Please commit.
> > ...
> > 
> > I took a second look, and I don't agree with this change:
> > 
> > > > -  startOffset = MIN (minLineStart, maxLineStart);
> > > > -  endOffset  = MIN (minLineEnd, maxLineEnd);
> > > > +  startOffset = MIN (minLineStart, minLineEnd);
> > > > +  endOffset  = MAX (maxLineStart, maxLineEnd);
> > 
> > 
> > Our API should guarantee that minLineStart <= minLineEnd, so your test
> > is redundant.
> > 
> > However, my intent was to make sure that the line of text at (x, y) has
> > a starting offset less than (or equal to) that of the line at (x+w,
> > y+h).
> > 
> > Granted you need rather weird text (bottom-to-top alignment) for
> > maxLineStart to be less than minLineStart in this example, but that was
> > the intent.  So the patch should read, IMO:
> > 
> >  -  endOffset  = MIN (minLineEnd, maxLineEnd);
> >  +  endOffset  = MAX (minLineStart, maxLineEnd);
> > 
> > FWIW,
> > the only example I can think of is a "clock face" text buffer,
> > containing the numbers 1 through 12 'in order', but laid out to look
> > like this:
> > 
> >         12
> >      11     1
> >     10       2
> >     9         3
> >      8       4
> >        7 6 5
> > 
> > 
> > regards,
> > 
> > Bill
> > 
> > > >  
> > > >    curr_offset = startOffset;
> > > >  
> > -- 
> > Bill Haneman <bill haneman sun com>
> > 
> 
> ______________________________________________________________________
> 
> cvs server: Diffing .
> Index: text.c
> ===================================================================
> RCS file: /cvs/gnome/at-spi/libspi/text.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 text.c
> --- text.c	10 Jan 2003 14:45:53 -0000	1.19
> +++ text.c	15 Jan 2003 11:48:17 -0000
> @@ -559,6 +559,7 @@ impl_getBoundedRanges(PortableServer_Ser
>    int curr_offset;
>    gint minLineStart, minLineEnd, maxLineStart, maxLineEnd;
>    long bounds_min_offset;
> +  long bounds_max_offset;
>  
>    clip.x = x;
>    clip.y = y;
> @@ -568,14 +569,16 @@ impl_getBoundedRanges(PortableServer_Ser
>    /* for horizontal text layouts, at least, the following check helps. */
>    bounds_min_offset =  atk_text_get_offset_at_point (text, x, y, 
>  						     (AtkCoordType) coordType);
> +  bounds_max_offset =  atk_text_get_offset_at_point (text, x + width, y + height, 
> +						     (AtkCoordType) coordType);
>    atk_text_get_text_at_offset (text, bounds_min_offset, 
>  			       ATK_TEXT_BOUNDARY_LINE_START,
>  			       &minLineStart, &minLineEnd);
> -  atk_text_get_text_at_offset (text, bounds_min_offset, 
> +  atk_text_get_text_at_offset (text, bounds_max_offset, 
>  			       ATK_TEXT_BOUNDARY_LINE_START,
>  			       &maxLineStart, &maxLineEnd);
>    startOffset = MIN (minLineStart, maxLineStart);
> -  endOffset  = MIN (minLineEnd, maxLineEnd);
> +  endOffset  = MAX (minLineEnd, maxLineEnd);
>  
>    curr_offset = startOffset;
>  
-- 
Bill Haneman <bill haneman sun com>