Currently working on a patch to address deprecations that trigger -Werror with Qt 5.14.1
Jean-Marc Lasgouttes
lasgouttes at lyx.org
Fri Mar 20 09:17:51 UTC 2020
Le 20/03/2020 à 03:41, Scott Kostyshak a écrit :
>> The entry in the change log hints (from my understanding) that it is a
>> renaming, but the blog article I linked to suggests things could be more
>> complicated.
>
> Looking at the code, it does seem to be equivalent:
>
> qreal QFontMetricsF::width(const QString &text) const
> {
> return horizontalAdvance(text);
> }
Good.
> So it seems it would not be a regression for us to rename every instance
> to horizontalAdvance(), although I would have to check the equivalence
> for all variants of the method. However, I guess the idea here is it's a
> good opportunity to catch a mistake in our code?
Yes, we should catch the cases where we need the total width(). I have
issues to find time though, due to our confinement status in France. 5
people in a flat all day is a bit complicated :)
> Let's take the first case then, which is CCItemDelegate::drawCategoryHeader:
>
> // draw the centered text
> QFontMetrics fm(font);
> int w = fm.width(category);
> int x = opt.rect.x() + (opt.rect.width() - w) / 2;
> int y = opt.rect.y() + 3 * fm.ascent() / 2;
> int left = x;
> int right = x + w;
> painter->drawText(x, y, category);
>
> From what I understand, this code wants to center the category header.
> Although I've read up on the differences of boundingRect().width() and
> horizontalAdvance(), I don't know which one is correct here. In some
> sense, we are not really drawing text after the string---we just want to
> know where to start drawing it so that it is centered. So that makes me
> think boundingRect().width() would be more appropriate. On the other
> hand, I wonder if horizontalAdvance() would provide a centering that is
> more natural to the eye. If I had to guess, I would say that
> boundingRect().width() is the correct choice here: I'm picturing a
> string with weird bearings on the first and last character, and I think
> my eyes would prefer for the bearings to not be taken into account while
> centering. Is my approach to thinking about this reasonable?
I would say that we don't care, so width() is better since is is shorter
and easier to understand :)
More information about the lyx-devel
mailing list