Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 11, 2015, 2:41 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Christoph Feck. Changes --- Submitted with commit 841f23a712dfc6be7fee16f8a6451ec4a084c45c by David Edmundson to branch master. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 10, 2015, 3:42 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs (updated) - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77266 --- Ship it! Ship It! - Christoph Feck On March 10, 2015, 3:42 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 10, 2015, 3:42 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77222 --- Hi, this fixes the pixelation issues in the Kate document list (thought the project plugin tree view is still pixelated, guess need to take a look at the Kate code for that myself ;=) - Christoph Cullmann On March 9, 2015, 5:54 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 9, 2015, 5:54 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77223 --- makes no sense to me, you are getting a rect to paint on and painting outside of it. what even warrants that there will be space on the qpainter? should the size increase be done in upper levels? - Albert Astals Cid On mar. 9, 2015, 5:54 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated mar. 9, 2015, 5:54 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
On March 9, 2015, 6:26 p.m., Albert Astals Cid wrote: makes no sense to me, you are getting a rect to paint on and painting outside of it. what even warrants that there will be space on the qpainter? should the size increase be done in upper levels? It's not painting outside it, the first argument is the target area, the pixmap is scaled to fit. Which is actually what's happening before this patch, scaling it upwards making it look blocky with a high QT_DEVICE_PIXEL_RATIO. In this case I'm making sure what I'm painting does match the painter. We know the painter's target is rect user pixels big. Which means in reality it is rect * devicePixelRatio real device pixels big. To draw a clean icon, we need the pixmaps with a matching size in device pixels, not a matching size in user pixels. Normally this does happen in an upper level. QIcon::pixmap() does this resizing, but QIcon::paint which calls this cannot. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77223 --- On March 9, 2015, 5:54 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 9, 2015, 5:54 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77228 --- src/kiconengine.cpp https://git.reviewboard.kde.org/r/122875/#comment53034 Please use spaces around binary '*' operator. - Christoph Feck On March 9, 2015, 5:54 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated March 9, 2015, 5:54 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122875: Fix KIconEngine::paint to handle different devicePixelRatios
On mar. 9, 2015, 6:26 p.m., Albert Astals Cid wrote: makes no sense to me, you are getting a rect to paint on and painting outside of it. what even warrants that there will be space on the qpainter? should the size increase be done in upper levels? David Edmundson wrote: It's not painting outside it, the first argument is the target area, the pixmap is scaled to fit. Which is actually what's happening before this patch, scaling it upwards making it look blocky with a high QT_DEVICE_PIXEL_RATIO. In this case I'm making sure what I'm painting does match the painter. We know the painter's target is rect user pixels big. Which means in reality it is rect * devicePixelRatio real device pixels big. To draw a clean icon, we need the pixmaps with a matching size in device pixels, not a matching size in user pixels. Normally this does happen in an upper level. QIcon::pixmap() does this resizing, but QIcon::paint which calls this cannot. Ok, i have no idea about this, so i guess this is a ±0 from my side - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/#review77223 --- On mar. 9, 2015, 5:54 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122875/ --- (Updated mar. 9, 2015, 5:54 p.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kiconthemes Description --- This now matches the behaviour of QPixmapIconEngine::paint Diffs - src/kiconengine.cpp 6dff533 Diff: https://git.reviewboard.kde.org/r/122875/diff/ Testing --- Opened configure toolbars in konversation with QT_DEVICE_PIXEL_RATIO=2 QStyledItemDelegate calls QIcon::paint which ends up going through this code with our QPA. Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
