Changing the skin is (too) slow

[Markus]

While not deal breaking, the performance when changing skins or scheme takes from very long to forever (= kill mixxx after 15min from the task manager).

[Markus (markusb)]

I'm using Mixxx 1.6.1 on Windows XP with an Thinkpad T60 (1.6GHz, 2GB memory).

[RJ Skerry-Ryan (rryan)]

Hi Markus, thanks for the bug report. I agree that changing skins sometimes takes strangely long. I once tried to track this down, and found that it has something to do with our theme/moods that are available for the outline family of skins. Changing skins for non-outline skins is pretty much instant. I wasn't able to fix this though.

[Albert Santoni (gamegod)]

Hi RJ,

During our last bug day, I profiled Mixxx with gprof to try to figure out what was going on here. It looks like I overwrote the good log file with a crappy one (where Mixxx was compiled without profile=1), so the data I have is useless. It's worth giving this another shot through gprof though, if anyone's bored. I remember Adam and I looking at the good data set I had, and not understanding why it was spending so much time in some piece of code.... It was something to do with the scheme/colouring stuff (as you said), but we couldn't figure out why it was doing what it was doing.

That was vague, but this would be an interesting project for someone new to Mixxx...

Thanks,
Albert

[Albert Santoni (gamegod)]

Right, and the attachment above is my crappy data set. It doesn't seem to have the %time column filled out, which is why I think I ran it with profile=0 by accident.

[Albert Santoni (gamegod)]

Sean, one might be good to look at. Maybe look at the code for a bit and see if you can figure out why skin changing takes so long. This only changed in beta4 or the 1.6.0 final, it was working fine before that. The biggest change that happened around that time was the merge of RJ's new waveform widget. It's possible that there's some weird interaction going on between the colour-schemes code and the new waveform, but we never had time to seriously look into it. If we can't fix this before 1.6.2, it's not the end of the world, but it would be good to fix. Thanks!

[Sean M. Pappalardo (pegasus-renegadetech)]

What, me? Uhh, okay, in a little while. In the meantime, here's a back trace of the issue incase anyone can give me some insight or wants to fix it.

[Sean M. Pappalardo (pegasus-renegadetech)]

Looks like a race condition indeed. The attached backtrace was taken when the waveform, audio playback and GUI froze during a skin change (it would eventually free itself after 3 minutes or so if I let it.)

Looks like it's between SoundDevicePortAudio::callbackProcess and QWidget::update from WOverview::setValue and WWidget::setValue in the couple back traces I got.

RRyaaaaaan!

[Sean M. Pappalardo (pegasus-renegadetech)]

I also noticed, having commented out i->setPixel at line 61 in imgsource.h, that this loop block takes a long time in and of itself being called per pixel over a large screen area in total.

My question: why aren't we using transparent-background PNG files (at least for the Outline series?) That way, we need only block-fill the background with the desired color (or change the background image for anything more complex,) instead of painting every pixel individually. Since we're requiring OpenGL anyway for the waveform, alpha channel and block-fill work should run quickly too. Doesn't OpenGL also support search-n-replace HSV modification on-the-fly, which would also take care of the foreground control images?

That aside, taking the Collusion skin, it would be far more efficient to have separate background images for each skin than to color-modify the single one the way we're currently doing it. (Not to mention the side-effect of mangling the Mixxx logo colors.) BTW, that background needs a version number update.

[Adam Davison (adamdavison)]

Hi. I actually did spend a while debugging this. We do indeed modify
the colour of every pixel as part of the colour scheme stuff. And that
is definitely where it spends a long time. However. The weird thing is
that this used to be fine. I mean even changing a fullscreen image's
colours, say 1M pixels, the actual colour conversion is fast, say 100
cycles, it should take well under a second, which it used to. Perhaps
there's some function in the QImage or QRgb stuff that's become very
slow for some reason... It was always a bit of a mystery.

2009/3/19 Pegasus <email address hidden>:
> I also noticed, having commented out i->setPixel at line 61 in
> imgsource.h, that this loop block takes a long time in and of itself
> being called per pixel over a large screen area in total.
>
> My question: why aren't we using transparent-background PNG files (at
> least for the Outline series?) That way, we need only block-fill the
> background with the desired color (or change the background image for
> anything more complex,) instead of painting every pixel individually.
> Since we're requiring OpenGL anyway for the waveform, alpha channel and
> block-fill work should run quickly too. Doesn't OpenGL also support
> search-n-replace HSV modification on-the-fly, which would also take care
> of the foreground control images?
>
> That aside, taking the Collusion skin, it would be far more efficient to
> have separate background images for each skin than to color-modify the
> single one the way we're currently doing it. (Not to mention the side-
> effect of mangling the Mixxx logo colors.) BTW, that background needs a
> version number update.
>
> --
> Changing the skin is (too) slow
> https://bugs.launchpad.net/bugs/289279
> You received this bug notification because you are a member of Mixxx
> Development Team, which is subscribed to Mixxx.
>

[RJ Skerry-Ryan (rryan)]

It turns out this is because we use setPixel() in the loop that goes over changing the colors. The QImage docs say that setPixel has performance problems because of its implementation. This must have changed since we wrote the color scheme code, because it was fast before. I committed a fix to trunk in r2739. I also pushed this to the 1.6.2 release branch in r2307.

[RJ Skerry-Ryan (rryan)]

Also, the Qt docs only started showing the warning in Qt 4.4, which goes well with our 'Qt changed' theory of why this wasn't a problem before.

[ironstorm (ironstorm-gmail)]

New windows build from trunk with this fix here: http://mixxx.org/packages/windows/mixxx-SVN2740M-20090321_2129-win.exe

[ironstorm (ironstorm-gmail)]

New build: http://mixxx.org/packages/windows/mixxx-SVN2740-20090322_1916-win.exe
This one has the fixes and an option to run inside GDB.

[Markus (markusb)]

Gentlemen,

I had a look (installed the SVN2740 build) and the problem is gone. Skin changes are fast now.

Thanks Markus

[ironstorm (ironstorm-gmail)]

Markus, go with the 1.7.0 Release (get it from http://mixxx.org/)... that SVN build is ancient and should probably be removed.

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/5052