> On Nov. 2, 2015, 12:05 p.m., Sebastian Kügler wrote: > > I don't like this change, as it introduces a magic constant for a value > > that we completely control our own. (Well, to the degree that we say "a > > gridUnit is roughly the height of a line of text". The 1.6 constant looks > > weird here, and I'm against adding font specific hacks in, especially since > > at that point in the code, we don't even know what the font is. > > > > This is not a structural solution, so -1.
I understand, but what is a structural solution? The font metric is clearly wrong for Noto, Oxygen (and certainly some other fonts) and this change is backwards compatible with current code. And this is not a font specific hack, this just scales the actual height of M to the expected gridUnit value because gridUnit never was a height of M character (it is like 1.6 * height of M = thus the magic number). - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125773/#review87864 ----------------------------------------------------------- On Oct. 29, 2015, 6:16 p.m., David Rosca wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125773/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2015, 6:16 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) returns 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 > > > File Attachments > ---------------- > > systray + popup before > > https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/d409b391-e35e-451f-bb5b-aa42e7eb2bf3__before.png > after > > https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/445009bc-a4a8-4998-8e82-996a0a4e33fb__after.png > > > Thanks, > > David Rosca > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
