Re: Review Request 129281: [Konsole] Render text at primary font's baseline
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129281/#review100745 --- looks good to me, but I think hindenburg should ack it (might want to add him to reviewers) - Martin Tobias Holmedahl Sandsmark On Nov. 6, 2016, 8:40 p.m., Christoph Feck wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129281/ > --- > > (Updated Nov. 6, 2016, 8:40 p.m.) > > > Review request for KDE Frameworks and Konsole. > > > Bugs: 371687 > http://bugs.kde.org/show_bug.cgi?id=371687 > > > Repository: konsole > > > Description > --- > > When Konsole draws a line of text, it first computes the rectangle of the > line that the text covers (taking into account cells width and height), then > passes this rectangle to the drawText(QRect, flags, text) call. > > Qt detects if the selected font does not offer all characters in the text, > and substitutes individual characters with a different font. Due to designer > choices, the same font point size does not lead to same pixel height (or > ascent size) in all fonts, so the substituted characters might be larger than > the characters from the primary font. > > Using a rectangle causes Qt to position glyphs relative to the bounding box > of the text, instead of anchored to the baseline. > > This patch uses a pixel position instead of a rectangle to draw the text, > taking into account only the baseline of the primary font. > > I have added all "frameworks" developers to increase possible test coverage. > > > Diffs > - > > src/TerminalDisplay.cpp 39a8b84 > > Diff: https://git.reviewboard.kde.org/r/129281/diff/ > > > Testing > --- > > On my system, lines with substituted Unicode characters are no longer shifted > away from the baseline, and therefore do not appear cropped. > > Further testing is needed, as there are many (equivalent, similar, or > different) bug reports about font rendering on different systems, see > https://bugs.kde.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=CONFIRMED&component=font&product=konsole > > > Thanks, > > Christoph Feck > >
Re: Review Request 129133: Silent warning of unused variable in lamda function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129133/ --- (Updated Nov. 9, 2016, 2 p.m.) Review request for KDE Frameworks and Valentin Rusu. Repository: kwallet Description --- ^^ Diffs (updated) - src/runtime/kwalletd/migrationwizard.cpp cdd2a92 Diff: https://git.reviewboard.kde.org/r/129133/diff/ Testing --- Done Thanks, Nilesh Kokane
Re: Review Request 129281: [Konsole] Render text at primary font's baseline
> On Nov. 9, 2016, 1:22 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > looks good to me, but I think hindenburg should ack it (might want to add > > him to reviewers) Pretty sure "konsole" reviewers group includes the maintainer, but I would actually prefer feedback from someone using non-Western characters in Konsole. - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129281/#review100745 --- On Nov. 6, 2016, 9:40 p.m., Christoph Feck wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129281/ > --- > > (Updated Nov. 6, 2016, 9:40 p.m.) > > > Review request for KDE Frameworks and Konsole. > > > Bugs: 371687 > http://bugs.kde.org/show_bug.cgi?id=371687 > > > Repository: konsole > > > Description > --- > > When Konsole draws a line of text, it first computes the rectangle of the > line that the text covers (taking into account cells width and height), then > passes this rectangle to the drawText(QRect, flags, text) call. > > Qt detects if the selected font does not offer all characters in the text, > and substitutes individual characters with a different font. Due to designer > choices, the same font point size does not lead to same pixel height (or > ascent size) in all fonts, so the substituted characters might be larger than > the characters from the primary font. > > Using a rectangle causes Qt to position glyphs relative to the bounding box > of the text, instead of anchored to the baseline. > > This patch uses a pixel position instead of a rectangle to draw the text, > taking into account only the baseline of the primary font. > > I have added all "frameworks" developers to increase possible test coverage. > > > Diffs > - > > src/TerminalDisplay.cpp 39a8b84 > > Diff: https://git.reviewboard.kde.org/r/129281/diff/ > > > Testing > --- > > On my system, lines with substituted Unicode characters are no longer shifted > away from the baseline, and therefore do not appear cropped. > > Further testing is needed, as there are many (equivalent, similar, or > different) bug reports about font rendering on different systems, see > https://bugs.kde.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=CONFIRMED&component=font&product=konsole > > > Thanks, > > Christoph Feck > >
Re: Review Request 129133: Silent warning of unused variable in lamda function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129133/#review100750 --- Ship it! Remember to update the commit message ;-) - David Faure On Nov. 9, 2016, 2 p.m., Nilesh Kokane wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129133/ > --- > > (Updated Nov. 9, 2016, 2 p.m.) > > > Review request for KDE Frameworks and Valentin Rusu. > > > Repository: kwallet > > > Description > --- > > ^^ > > > Diffs > - > > src/runtime/kwalletd/migrationwizard.cpp cdd2a92 > > Diff: https://git.reviewboard.kde.org/r/129133/diff/ > > > Testing > --- > > Done > > > Thanks, > > Nilesh Kokane > >
Re: Review Request 129133: Silent warning of unused variable in lamda function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129133/ --- (Updated Nov. 9, 2016, 10:35 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Valentin Rusu. Changes --- Submitted with commit da8cb1c530ad07028d886cc07a4310d89c79dfbd by Albert Astals Cid on behalf of Nilesh Kokane to branch master. Repository: kwallet Description --- ^^ Diffs - src/runtime/kwalletd/migrationwizard.cpp cdd2a92 Diff: https://git.reviewboard.kde.org/r/129133/diff/ Testing --- Done Thanks, Nilesh Kokane
Re: Review Request 129361: Add library path so utempter binary is found in Ubuntu 16.10
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129361/ --- (Updated Nov. 9, 2016, 10:36 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Jonathan Riddell and Rex Dieter. Changes --- Submitted with commit 97876c1d152f7d0ea0e5adcfe88abeab4e7117c9 by Albert Astals Cid to branch master. Repository: kpty Description --- Read the summary Diffs - cmake/FindUTEMPTER.cmake d3236c8 Diff: https://git.reviewboard.kde.org/r/129361/diff/ Testing --- Works Thanks, Albert Astals Cid
Re: Review Request 129327: Expose desktopFileName in KWindowInfo
> On Nov. 8, 2016, 6:08 p.m., Aleix Pol Gonzalez wrote: > > It's weird that it's called `desktopFileName` but it doesn't offer a file > > name? I think it should be renamed or fixed to provide the path. In the end > > it's an API that then requires the user to do a rather big look-up. The naming follows the naming in: NETWinInfo, KAboutData and QGuiApplication. Naming it differently would be wrong IMHO. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129327/#review100732 --- On Nov. 7, 2016, 7:29 a.m., Martin Gräßlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129327/ > --- > > (Updated Nov. 7, 2016, 7:29 a.m.) > > > Review request for KDE Frameworks and Eike Hein. > > > Repository: kwindowsystem > > > Description > --- > > This change introduced a new method in KWindowInfo: > QByteArray KWindowInfo::desktopFileName() const > > It returns the desktop file name of the application if known on the > given platform. So far only provided on X11 through > NETWinInfo::desktopFileName. > > > Diffs > - > > autotests/kwindowinfox11test.cpp 5dfbcfa67d74244122c86433a40a7ed6923fb1ab > src/kwindowinfo.h 5d9799b20d640caa1b1cf9ab4d9dc69b8cceefe3 > src/kwindowinfo.cpp 658a0b645797676d4e48585ede3d832333688081 > src/kwindowinfo_p.h 45390c06e7b5ad064ea9368ca102b2462a029c06 > src/platforms/xcb/kwindowinfo.cpp eca607e18a979439593e05e1da232548d0e7d139 > src/platforms/xcb/kwindowinfo_p_x11.h > 68805765fd630c2bc7cf0d77be688333b4a363f7 > > Diff: https://git.reviewboard.kde.org/r/129327/diff/ > > > Testing > --- > > > Thanks, > > Martin Gräßlin > >
