[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: koffice/plugins/textshape
From:       "C. Boemann" <cbo () boemann ! dk>
Date:       2009-12-16 12:08:19
Message-ID: 200912161308.19923.cbo () boemann ! dk
[Download RAW message or body]

On Wednesday 16 December 2009 12:26:18 Thomas Zander > 
> -        qreal y = line.position().y() + line.height() / 2;
> > +        qreal y = line.position().y() + line.ascent();
> 
> This makes no sense. y is at the bottom of the line, not the 
top.
Well you are obviously wrong about that one. Look no longer 
than to the fact that the half height was _added_ before as well.
> 
> > +        QTextCharFormat::VerticalAlignment valign =
> >  fmt.verticalAlignment();
> > +
> > +        QFont font(fmt.font());
> > +        if(valign == QTextCharFormat::AlignSubScript || valign 
==
> >  QTextCharFormat::AlignSuperScript) +
> >  font.setPointSize((font.pointSize() * 2) / 3);
> 
> You likely want to use pointSizeF
No I don't. Qt uses the integer version internally too. Using 
anything else her will cause a difference that would be visible 
and ugly.
> 
> > +        QFontMetrics metrics(font);
> 
> And QFontMetricsF
Good point, will commit shortly.

> And you loose the paintdevice like this.
Yeah, but i've tried adding it and i don't see a difference. I would 
actually have expected it to fix the divide by 2 weirdness, but it 
doesn't.

> > +        y -= metrics.strikeOutPos()/2;
> 
> This makes no sense; why divide by 2?
Yes it makes no sense but it is completely correct

> 
> > +        qreal height = (metrics.ascent() + 
metrics.descent())/2;
> 
> Can't you use metrics.height()?
Again Qt does it like this, and I'd like to do it as similar as 
possible, to avoid any potential problems.
Height may be the same as the sum, so i think you are right but 
still why take the chance.
The dividing by 2 is again the same as above - makes no sense 
but that is how it is.

> >  -                        fmt.font());
> > +                          font)/2;
> 
the font is used because the width of the line should change 
along with the smaller font and fmt.font is the "original font"
The dividing by 2 is again the same as above - makes no sense 
but that is how it is.

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]