Re: patch/bug fix - 642

Stephen Hack (shack@uiuc.edu)
Tue, 9 Nov 1999 23:23:20 -0600


Ok. So, the UT_convertToDimensionString function is not what the ruler should
be using, but UT_convertInchesToDimension. Well, the ruler used the
convertToDimensionString via the GR_Graphics::invertDimension function.

My investigation continues... As I do a grep -r for invertDimension, the only
calls are from the ap_TopRuler class.

Thus, the function should be changed from .... src/af/gr/xp/gr_Graphics.cpp

const char * GR_Graphics::invertDimension(UT_Dimension dim, double dValue) const
{
// return pointer to static buffer -- use it quickly.

double dResolution = getResolution();// NOTE: assumes square pixels/dpi/etc.
double dInches = dValue / dResolution;

return UT_convertToDimensionString(dim, dInches);
}

.... to convertInchesToDimension

Is there any reason why GR_Graphics::invertDimension should not be changed to
InchesToDimension?

-shack

On Tue, Nov 09, 1999 at 12:53:09PM -0800, Paul Rohr wrote:
> At 09:31 PM 11/7/99 -0600, Stephen Hack wrote:
> >Bug fix patch - bug 642
> >
> >The bug was in placing tabs in non-inches units. If you look at the code,
> >it's obvious, for af/util/xp/ut_units.cpp :: UT_convertToDimensionString did
> >not actually do the conversion.
> >
> >If turned out to be a variable problem. We we doing the conversion, the
> >returning the value passed in as a parameter.
>
> Argh. You just got bit by the same thing I did. That function needs to be
> rethought, and all uses of it rechecked.
>
> >>From scanning through Bonsai's CVS history on this function, it looks like
> this function has served several different purposes over time.
>
> Originally (1.13), it converted from inches to whatever units you wanted.
> At the time, I did a lot of testing to make sure that the ruler code used
> this logic properly by switching unit systems in code and recompiling.
>
> Later on (1.17), it morphed into a fancy sprintf which didn't convert units.
> By the time I started working with Shaw to port the Paragraph dialog (which
> does even more unit-conversion work than the ruler), I got bit too. At the
> time I tried changing back to the old semantics, but ran across other code
> which depended on the new semantics.
>
> Instead (1.21), I added a new function UT_convertInchesToDimension() which
> does the other half of the original job, as seen here:
>
> abi/src/wp/ap/xp/ap_Dialog_Paragraph.cpp
>
> What a mess. :-(
>
> At this point, rather than applying your patch, I think we should:
>
> - strip out the last traces of the old semantics from the function,
> - rename it to be clearer about its new job, and
> - recheck all uses to see which functions should be called.
>
> It's a thankless janitorial task, but it'll help prevent similar confusion
> in the future.
>
> Paul



This archive was generated by hypermail 1.03b2.