D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

cullmann accepted this revision. cullmann added a comment. I think this should go in now. I think the usage of the API is correct even for HiDPI. In KF6, we should just fold that interface into the main class. REPOSITORY R39 KTextEditor BRANCH addmarkinterfacev2 REVISION DETAIL

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added inline comments. INLINE COMMENTS > dhaumann wrote in kateviewhelpers.cpp:1963 > This is true. But in this case a non-answer: Maybe QIcon::paint does it > correct as well. In other words, the code can very likely be improved, but > also ok as is. @anthonyfieroni Why exactly do

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kateviewhelpers.cpp:1963 > You should keep devicePixelRatioF calls This is true. But in this case a non-answer: Maybe QIcon::paint does it correct as well. In other words, the code can very likely be improved, but also

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

anthonyfieroni added inline comments. INLINE COMMENTS > kossebau wrote in kateviewhelpers.cpp:1963 > Possibly `QIcon::paint()` might be also working here as wanted? Needs someone > with HiDPI to check if all things behave as wanted. The old code with all the > `devicePixelRatioF()` made me

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor BRANCH addmarkinterfacev2 REVISION DETAIL https://phabricator.kde.org/D27533 To: kossebau, #kate, #kdevelop, dhaumann Cc: anthonyfieroni, dhaumann, kwrite-devel,

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added a comment. Ok, then I am fine with this. Maybe add a KF6 task to the KF6 board? INLINE COMMENTS > kossebau wrote in katedocument.h:86 > See the warning in the API docs, nobody should rely on this private API, so > the BIC here is okay. > The class is exported for the unit

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added inline comments. INLINE COMMENTS > anthonyfieroni wrote in katedocument.h:86 > Why private class is exported, changing parent of an exported class is BIC. See the warning in the API docs, nobody should rely on this private API, so the BIC here is okay. The class is exported for

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

anthonyfieroni added inline comments. INLINE COMMENTS > katedocument.h:86 > class KTEXTEDITOR_EXPORT KTextEditor::DocumentPrivate : public > KTextEditor::Document, > -public > KTextEditor::MarkInterface, > +

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added a comment. In D27533#615478 , @dhaumann wrote: > Still, I wonder whether we should postpone adding the MarkInterfaceV2 until the KF6 branch. Then we have it properly fixed in KF6. KF5 will stay around for a few more years

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added a comment. Definitely a +1 for using a QIcon here. Still, I wonder whether we should postpone adding the MarkInterfaceV2 until the KF6 branch. Then we have it properly fixed in KF6. Or do you think we also need this in KF5 days to properly support e.g. hidpi ?

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau marked an inline comment as done. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D27533 To: kossebau, #kate, #kdevelop Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars,

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau updated this revision to Diff 76127. kossebau added a comment. Switch to QIcon::paint, leaving all vodoo with devicePixelRatioF to QIcon logic Also results in better handling of pixmaps when they would be oversampled. Tested with QT_SCALE_FACTOR REPOSITORY R39 KTextEditor

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added inline comments. INLINE COMMENTS > kateviewhelpers.cpp:1963 > const int s = qMin(m_iconAreaWidth * > devicePixelRatioF(), h * devicePixelRatioF()) - 2; > -px_mark = px_mark.scaled(s, s, > Qt::KeepAspectRatio,

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added a comment. See e.g. how bookmark mark symbols become crisp in bigger sizes (though they take advantage currently in being mostly regular lines and thus partially cope with pixelscaling): F8116357: Screenshot_20200221_003940.png

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau added a comment. https://invent.kde.org/kde/kate/merge_requests/68 would be the respective patch for Kate. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D27533 To: kossebau, #kate, #kdevelop Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n,

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

kossebau created this revision. kossebau added reviewers: Kate, KDevelop. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. kossebau requested review of this revision. REVISION SUMMARY Symbols used for marks can be used in different sizes,