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