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

[Bastian Holst]

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

Ship it!


I like this patch, for more information, see below ;).


trunk/KDE/kdeedu/marble/src/lib/TextureTile.h
<http://reviewboard.kde.org/r/4444/#comment6122>

    I wonder why you see a problem here.
    If it is (shallow) copy against pointer not-copy, it'll be basically the question of:
    
    * do we want that changes of a “copy” do affect other “copies”
    
    or 
    
    * do we want that changes of a “copy” do not affect other “copies”
    
    If there are no use cases for the first option (and I do think that there are none), I'd also prefer the second (shallow-copy) version.
    



trunk/KDE/kdeedu/marble/src/lib/TextureTile.cpp
<http://reviewboard.kde.org/r/4444/#comment6123>

    As it looks in this line, we are creating a shallow-copy anyhow.



trunk/KDE/kdeedu/marble/src/lib/TextureTile.cpp
<http://reviewboard.kde.org/r/4444/#comment6124>

    And here goes a lot of possibly leaky “delete”-code.


- Bastian


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/20100706/864c84d5/attachment.htm