--===============4963823019950862988== Content-Type: multipart/alternative; boundary="===============9191496418246799735==" --===============9191496418246799735== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127260/#review94156 ----------------------------------------------------------- autotests/themetest.cpp (line 71) We need to test the cache aspect of everything, as that's really the entire point. it looks like we can access it with m_svg->theme()->findInCache() to check things if icons were actually being inserted. and then we can test that the cache *doesn't* contain an entry after the spy finishes. I made a patch: https://paste.kde.org/pgyzgxmzv From what I can tell, it doesn't work. Though that's possibly my part new lines? src/plasma/svg.cpp (line 228) I added a debug line in here, and got nothing in plasmashell actually enters here. (the test does though) Are there some other changes needed? src/plasma/svg.cpp (line 232) Don't we need someone to call discardCache somewhere if the icon changes? (in other words, shouldn't this be in theme.cpp somewhere instead) - David Edmundson On March 31, 2016, 1:07 p.m., Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127260/ > ----------------------------------------------------------- > > (Updated March 31, 2016, 1:07 p.m.) > > > Review request for Plasma. > > > Repository: plasma-framework > > > Description > ------- > > this attempts to cache as well svg icons from the icon theme (invalidating as well when the icon theme is updated) > > still to be done, to figure out to invalidate cache when the icon theme is changed in the two cases: > * theme changed with plasmashell running > * theme changed with plasma shell not running > > > Diffs > ----- > > autotests/CMakeLists.txt d475ac3 > autotests/data/icons/test-theme-two/apps/22/tst-plasma-framework-test-icon.svg PRE-CREATION > autotests/data/icons/test-theme-two/index.theme PRE-CREATION > autotests/themetest.h PRE-CREATION > autotests/themetest.cpp PRE-CREATION > src/plasma/private/theme_p.h 69a8934 > src/plasma/private/theme_p.cpp 98bccab > src/plasma/svg.cpp 6c9c75c > > Diff: https://git.reviewboard.kde.org/r/127260/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > > --===============9191496418246799735== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127260/

autotests/themetest.cpp (Diff revision 3)
		71

We need to test the cache aspect of everything, as that's really the entire point.

it looks like we can access it with

m_svg->theme()->findInCache() to check things if icons were actually being inserted.

and then we can test that the cache doesn't contain an entry after the spy finishes.

I made a patch: https://paste.kde.org/pgyzgxmzv

From what I can tell, it doesn't work. Though that's possibly my part new lines?

src/plasma/svg.cpp (Diff revision 3)
219	if (themed) {	228	if (inIconTheme) {

I added a debug line in here, and got nothing in plasmashell actually enters here. (the test does though)

Are there some other changes needed?

src/plasma/svg.cpp (Diff revision 3)
		232	QObject::connect(KIconLoader::global(), SIGNAL(iconChanged(int)), q, SLOT(themeChanged()));

Don't we need someone to call discardCache somewhere if the icon changes? (in other words, shouldn't this be in theme.cpp somewhere instead)

- David Edmundson

On March 31st, 2016, 1:07 p.m. UTC, Marco Martin wrote:

Review request for Plasma.

By Marco Martin.

Updated March 31, 2016, 1:07 p.m.

Repository: plasma-framework

Description

this attempts to cache as well svg icons from the icon theme (invalidating as well when the icon theme is updated)

still to be done, to figure out to invalidate cache when the icon theme is changed in the two cases: theme changed with plasmashell running theme changed with plasma shell not running

Diffs

autotests/CMakeLists.txt (d475ac3)
autotests/data/icons/test-theme-two/apps/22/tst-plasma-framework-test-icon.svg (PRE-CREATION)
autotests/data/icons/test-theme-two/index.theme (PRE-CREATION)
autotests/themetest.h (PRE-CREATION)
autotests/themetest.cpp (PRE-CREATION)
src/plasma/private/theme_p.h (69a8934)
src/plasma/private/theme_p.cpp (98bccab)
src/plasma/svg.cpp (6c9c75c)

--===============9191496418246799735==-- --===============4963823019950862988== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============4963823019950862988==--