> On Oct. 24, 2015, 3:07 p.m., David Edmundson wrote: > > src/plasma/theme.cpp, lines 469-473 > > <https://git.reviewboard.kde.org/r/125773/diff/1/?file=412447#file412447line469> > > > > that looks wrong. size is already width and height and users (should) > > be already using the right one for the situation. > > > > some code is already using mSize(font).height() > > > > they'll be getting totally weird results after this change > > > > I don't think it's needed > > David Rosca wrote: > QFontMetrics(font).boundingRect("M").size() = QSize(9, 17) > QFontMetrics(font).boundingRect(QLatin1Char('M')).size() = QSize(9, 9) > > So actually only height needs to be multiplied. Also in this case (after > multiplying with 1.6) the difference will be slightly bigger than in > units.gridSize, because units.gridSize is rounded to even number.
Eh, the bigger difference (9 * 1.6 = *14,4* vs *17*) in this case is a good thing. This is with Noto Sans font that has this issue (boundingRect(QString) returns too big height) = lower height fixes the issue. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125773/#review87339 ----------------------------------------------------------- On Oct. 24, 2015, 3:22 p.m., David Rosca wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125773/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2015, 3:22 p.m.) > > > Review request for Plasma. > > > Bugs: 343349 > http://bugs.kde.org/show_bug.cgi?id=343349 > > > Repository: plasma-framework > > > Description > ------- > > For some fonts, QFontMetrics::boundingRect(QString) too high rect which makes > the gridSize too big. > It now returns correctly the actual height of M character. For backwards > compatibility, the value is multiplied with 1.6. > > This affects eg. Noto Sans font that is now default for Plasma 5.5. > > > Diffs > ----- > > src/declarativeimports/core/units.h fa2256e > src/declarativeimports/core/units.cpp 4e2adae > src/plasma/theme.cpp c49ad4c > > Diff: https://git.reviewboard.kde.org/r/125773/diff/ > > > Testing > ------- > > When switching to Noto Sans font, I noticed that icons in system tray grow to > big size so it switched to 1 column in vertical panel. Basically everything > in Plasma grow too much (even though the font is visually the same or even > smaller than DejaVu Sans that I was using before - same font size 9 was used) > - too big spacing in task manager, too big popups (application menu, system > tray popups), etc ... > > This fixes the issue. This may also fix BUG 343349 > > > Thanks, > > David Rosca > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
