[Kde-games-devel] Review Request: KCardCache: Delay loading SVG file until it is actually needed.

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/676/#review1089
-----------------------------------------------------------


Patch seems safe to me though i'm not very knowledgeable of that code i think you can commit if you feel your testing is enough.

- Albert


On 2009-05-06 10:04:25, Parker Coates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/676/
> -----------------------------------------------------------
> 
> (Updated 2009-05-06 10:04:25)
> 
> 
> Review request for KDE Games and Andreas Pakulat.
> 
> 
> Summary
> -------
> 
> Some new card decks have entered into SVN, which although very beautiful there also quite large. For example, the Ancient Egyptians card deck adds 3 seconds to KPat's start up time compared to a lighter deck like Nicu White. This increase in start up time isn't due to rendering time, as KPat already makes use of KCardCache to cache the rendered pixmaps between runs. The time consumed is used purely to load the SVG file into the KSvgRenderer.
> 
> This patch to KCardCache causes the SVG file to only be loaded on into the KSvgRenderer on first use. At first I thought this would be sufficient to improve start up time when the cards are already cached, but it is not. KPat must also call defaultFrontSize() on start up to learn the aspect ratio of the cards (something LSkat currently doesn't do). Previously, this function used to get the element size from the renderer, which then has to be loaded. This patch also causes that function to store it's default size in the cache, eliminating the need to access the renderer the next time the function is run, even across sessions.
> 
> I should point out that the expense of loading the SVG hasn't been removed, just delayed until a card rendering is needed that is already cached. If this patch works out, I'll look into using KCardCache's built-in threading to load the SVG file in the background.
> 
> I have tried to be careful about properly locking the mutexes, but I have no previous experience with QThreads, so please let me know if I'm doing anything dangerous.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/libkdegames/cardcache.cpp 962731 
>   /trunk/KDE/kdegames/libkdegames/cardcache_p.h 962731 
> 
> Diff: http://reviewboard.kde.org/r/676/diff
> 
> 
> Testing
> -------
> 
> A fair bit of testing with KPat. A little bit of testing with LSkat.
> 
> 
> Thanks,
> 
> Parker
> 
>