Currently working on a patch to address deprecations that trigger -Werror with Qt 5.14.1

[Scott Kostyshak]

On Sun, Mar 01, 2020 at 08:04:47PM +0100, Jean-Marc Lasgouttes wrote:
> Le 29/02/2020 à 18:27, Scott Kostyshak a écrit :
> > In Qt 5.11, horizontalAdvance() was added "to replace the confusingly
> > named width() function", which was deprecated in the same release. [1]
> 
> Do you know whether they are equivalent, or subtly different?

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.
> 
> >    "I am pretty sure that in most cases QFontMetrics::boundingRect() is
> >    what you want, unless you are writing custom text shaping/layouting
> >    code".
> 
> This will be the case where we also measure ascent and descent indeed.
> 
> >  From what I understand, we are indeed writing custom text
> > shaping/layouting code.
> 
> Yes, but mostly for the code in GuiFontMetrics.
> 
> > Attached is a patch. I don't propose to commit this patch; I just attach
> > it to give an idea of how many replacements are needed and in case
> > someone can check that we do indeed want horizontalAdvance() as the
> > replacement in all cases and not boundingRect().
> 
> The best is to apply small bits by small bits. As I said, only
> GuiFontMetrics should be problematic.
> 
> > JMarc, I'm guessing you're the one that I should bother about taking a
> > look at this specific change. Can you take a look when you have time? If
> > you prefer to not think about this or don't have time, it can wait; it's
> > just a compilation warning message.
> 
> Mostly for font metrics, but I can have a look. For the ones that are in
> dialogs and such; you guess is as good as mine.

Sounds good. I'll wait for you to take a look at font metrics then. If
you prefer, I could take a naive brute force approach of using extra
code that calculates all three of horizontalAdvance(),
boundingRect().width(), and the deprecated width() and then seeing which
are equal. I guess I would need to find a case where horizontalAdvance()
and boundingRect().width are different in order to be sure which should
be the replacement, but I suppose I could play around.

> > After figuring out the above, the next question would be how to
> > implement the changes in a way that preserves compatibility with Qt
> > versions < 5.11. Should we use a directive to condition on the Qt
> > version around each instance or since there are several instances should
> > we use the approach of a macro as is done at [3]?
> 
> I think that the only places where we need horizontal advance is when we do
> layout things, so mainly what is in GuiFontMetrics. In the other cases, we
> want the size of a box that contains the string, so the
> boundingBox().width() approach is fine

Good to know, that does simplify things since it seems that
QFontMetrics::boundingRect has been in Qt for a while so no need to
do a conditional with directives.

> (although it seems to me tht it
> computes ascent and descent unnecessarily).

Ah good point. I wonder if the "old" width() method actually avoided
that or not (maybe both were calculated and stored as members and width
was just a get() method?). In any case, I guess it is not worth it to
make a custom method to increase the efficiency.

Thanks for your comments.

Scott
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20200302/46eb08f9/attachment.asc>