[Marble-devel] Review Request: More robust tile loading

[Bernhard Beschow]

> On 2010-06-25 13:05:44, jmho wrote:
> > trunk/KDE/kdeedu/marble/src/lib/TextureTile.h, line 77
> > <http://reviewboard.kde.org/r/4444/diff/2/?file=29754#file29754line77>
> >
> >     Could not this stay a pointer also? It would prevent a QImage copy.
> 
> Bernhard Beschow wrote:
>     Since QImage is implicitly shared, copying is really fast. Here is a quote from the Qt documentation:
>     
>     "Implicitly shared classes are both safe and efficient when passed as arguments, because only a pointer to the data is passed around, and the data is copied only if and when a function writes to it, i.e., copy-on-write."
>     
>     This also means that if m_image is a value, QImage will do the memory management for us.
> 
> jmho wrote:
>     Sure, but
>     1) with pointers the code is easier to understand in this regard, that is one can easier see if a copy is a deep copy as there are much less copy operations
>     2) with a copy there is still the copy ctor executed, refernce counting done, etc
>

1) Why are there much less copy operations using a pointer? I'd say the number of deep copy operations are the same.
2) The copy constructor copies a pointer and increments the reference count. Given that this only happens after much more expensive operations like loading a tile from disk, do you really think that the additional increment of the reference count will impact performance?


- Bernhard


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


On 2010-06-25 12:49:24, Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4444/
> -----------------------------------------------------------
> 
> (Updated 2010-06-25 12:49:24)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Motivation: It may happen that Marble receives invalid image data over the network or loads invalid image data from disk, making Marble really slow. This is caused by heavy debug output from QImage while Marble tries to access invalid image data. This will be qFatal in the upcoming Qt 4.7.
> 
> Solution: Check if the image is valid ater loading data into a QImage instead of blindly loading data into it. Also Q_ASSERT every image assignment in TextureTile.
> 
> Note that by this patch, Marble will abort when TileLoader::scaledLowerLevelTile() is not able to produce a scaled image. This could happen when the level-zero tile is missing.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/lib/TextureTile.h 1142673 
>   trunk/KDE/kdeedu/marble/src/lib/TextureTile.cpp 1142673 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoader.h 1142673 
>   trunk/KDE/kdeedu/marble/src/lib/TileLoader.cpp 1142673 
> 
> Diff: http://reviewboard.kde.org/r/4444/diff
> 
> 
> Testing
> -------
> 
> Works for me. Please test if it works for you as well!
> 
> 
> Thanks,
> 
> Bernhard
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100625/3c275abf/attachment-0001.htm