On Thu, May 05, 2005 at 03:09:36PM +0100, Angus Leeming wrote:
> Martin Vermeer wrote:
>
> > Attached, please review
> >
> > - Martin
>
> tabular.getCellInset(cell)->metrics(m, dim);
> + if (!p_width.zero())
> + dim.wid = m.base.textwidth;
>
> Well, I can see what it does, but it seems a bit ugly. Surely, this should
> be happening inside of InsetText? Also, if you look at the code:
>
> void InsetText::metrics(MetricsInfo & mi, Dimension & dim) const
> {
> setViewCache(mi.base.bv);
> mi.base.textwidth -= 2 * border_;
> font_ = mi.base.font;
> text_.font_ = mi.base.font;
> text_.metrics(mi, dim);
> dim.asc += border_;
> dim.des += border_;
> dim.wid += 2 * border_;
> mi.base.textwidth += 2 * border_;
> dim_ = dim;
> }
>
> Note that your hack is invalidating the cached dim_. What are the
> repurcussions of that?
Keeping the dim_ cache valid between phase 1 and 2 is (in theory...)
crucial for the whole drawing scheme...
> Looking deeper, it seems to me that this code is just hacked together.
>
> Did you know that mi.base.textwidth is not changed at all by this
> function? (I don't believe it's used at all by LyXText::metrics.)
>
> Shouldn't we be using this 'max width' setting (that's what
> mi.base.textwidth is, right?) in text_.metrics so that your hack isn't
> needed at all?
At least that was the intention when introducing base.textwidth.
> Now I see why I try not to venture into these innards :)
Why not? You seem to get the intention by looking at the code
without sacrificing black roosters. Does not seem that bad...
Andre'
--
