Refactor tiles manager code as suggested by Fabio D'urso

core/tilesmanager.cpp

@@ -11,16 +11,18 @@
 #include <QPixmap>
 #include <QtCore/qmath.h>
 #include <QList>
-#include <QMutex>
 
-#define RANGE_MAX 1073741823
-#define RANGE_MIN -1073741824
+#define MISSCOUNTER_MAX INT_MAX/2
+#define MISSCOUNTER_MIN INT_MIN/2
+#define TILES_MAXSIZE 2000000
 
 using namespace Okular;
 
 static bool rankedTilesLessThan( Tile *t1, Tile *t2 )
 {
-    if ( t1->dirty ^ t2->dirty )
+    // Order tiles by its dirty state and miss counter. That is: dirty tiles
+    // will be evicted first then tiles with higher miss values.
+    if ( t1->dirty == t2->dirty )
         return t1->miss < t2->miss;
 
     return !t1->dirty;
@@ -38,15 +40,15 @@ class TilesManager::Private
         /**
          * Mark @p tile and all its children as dirty
          */
-        void markDirty( Tile &tile );
+        static void markDirty( Tile &tile );
 
         /**
          * Deletes all tiles, recursively
          */
         void deleteTiles( const Tile &tile );
 
-        void onClearPixmap( const Tile &tile );
-        void rankTiles( Tile &tile );
+        void markParentDirty( const Tile &tile );
+        void rankTiles( Tile &tile, QList<Tile*> &rankedTiles );
         /**
          * Since the tile can be large enough to occupy a significant amount of
          * space, they may be split in more tiles. This operation is performed
@@ -56,6 +58,14 @@ class TilesManager::Private
          */
         void split( Tile &tile, const NormalizedRect &rect );
 
+        /**
+         * Checks whether the tile's size is bigger than an arbitrary value and
+         * performs the split operation returning true.
+         * Otherwise it just returns false, without performing any operation.
+         */
+        bool splitBigTiles( Tile &tile, const NormalizedRect &rect );
+
+        // The page is split in a 4x4 grid of tiles
         Tile tiles[16];
         int width;
         int height;
@@ -65,8 +75,6 @@ class TilesManager::Private
         NormalizedRect requestRect;
         int requestWidth;
         int requestHeight;
-
-        QList<Tile*> rankedTiles;
 };
 
 TilesManager::Private::Private()
@@ -87,6 +95,7 @@ TilesManager::TilesManager( int width, int height, Rotation rotation )
     d->height = height;
     d->rotation = rotation;
 
+    // The page is split in a 4x4 grid of tiles
     const double dim = 0.25;
     for ( int i = 0; i < 16; ++i )
     {
@@ -131,7 +140,8 @@ void TilesManager::setWidth( int width )
     markDirty();
 }
 
-int TilesManager::width() const {
+int TilesManager::width() const
+{
     return d->width;
 }
 
@@ -143,7 +153,8 @@ void TilesManager::setHeight( int height )
     d->height = height;
 }
 
-int TilesManager::height() const {
+int TilesManager::height() const
+{
     return d->height;
 }
 
@@ -166,7 +177,7 @@ void TilesManager::markDirty()
 {
     for ( int i = 0; i < 16; ++i )
     {
-        d->markDirty( d->tiles[ i ] );
+        TilesManager::Private::markDirty( d->tiles[ i ] );
     }
 }
 
@@ -213,10 +224,12 @@ void TilesManager::Private::setPixmap( const QPixmap *pixmap, const NormalizedRe
 {
     QRect pixmapRect = TilesManager::toRotatedRect( rect, rotation ).geometry( width, height );
 
+    // Exclude tiles outside the viewport
     if ( !tile.rect.intersects( rect ) )
         return;
 
-    // the tile intersects an edge of the viewport
+    // if the tile is not entirely within the viewport (the tile intersects an
+    // edged of the viewport), attempt to set the pixmap in the children tiles
     if ( !((tile.rect & rect) == tile.rect) )
     {
         // paint children tiles
@@ -232,13 +245,13 @@ void TilesManager::Private::setPixmap( const QPixmap *pixmap, const NormalizedRe
         return;
     }
 
+    // the tile lies entirely within the viewport
     if ( tile.nTiles == 0 )
     {
         tile.dirty = false;
 
-        QRect tileRect = tile.rect.geometry( width, height );
-
-        if ( tileRect.width()*tileRect.height() < TILES_MAXSIZE ) // size ok
+        // check whether the tile size is big and split it if necessary
+        if ( !splitBigTiles( tile, rect ) )
         {
             if ( tile.pixmap )
             {
@@ -251,48 +264,40 @@ void TilesManager::Private::setPixmap( const QPixmap *pixmap, const NormalizedRe
         }
         else
         {
-            split( tile, rect );
-            if ( tile.nTiles > 0 )
+            if ( tile.pixmap )
             {
-                for ( int i = 0; i < tile.nTiles; ++i )
-                    setPixmap( pixmap, rect, tile.tiles[ i ] );
-
-                if ( tile.pixmap )
-                {
-                    totalPixels -= tile.pixmap->width()*tile.pixmap->height();
-                    delete tile.pixmap;
-                    tile.pixmap = 0;
-                }
+                totalPixels -= tile.pixmap->width()*tile.pixmap->height();
+                delete tile.pixmap;
+                tile.pixmap = 0;
             }
+
+            for ( int i = 0; i < tile.nTiles; ++i )
+                setPixmap( pixmap, rect, tile.tiles[ i ] );
         }
     }
     else
     {
         QRect tileRect = tile.rect.geometry( width, height );
-        if ( tileRect.width()*tileRect.height() >= TILES_MAXSIZE ) // size ok
+        // sets the pixmap of the children tiles. if the tile's size is too
+        // small, discards the children tiles and use the current one
+        if ( tileRect.width()*tileRect.height() >= TILES_MAXSIZE )
         {
             tile.dirty = false;
-            for ( int i = 0; i < tile.nTiles; ++i )
-                setPixmap( pixmap, rect, tile.tiles[ i ] );
-
             if ( tile.pixmap )
             {
                 totalPixels -= tile.pixmap->width()*tile.pixmap->height();
                 delete tile.pixmap;
                 tile.pixmap = 0;
             }
+
+            for ( int i = 0; i < tile.nTiles; ++i )
+                setPixmap( pixmap, rect, tile.tiles[ i ] );
         }
-        else // size not ok (too small)
+        else
         {
             // remove children tiles
-            tile.rect = NormalizedRect();
             for ( int i = 0; i < tile.nTiles; ++i )
             {
-                if ( tile.rect.isNull() )
-                    tile.rect = tile.tiles[ i ].rect;
-                else
-                    tile.rect |= tile.tiles[ i ].rect;
-
                 deleteTiles( tile.tiles[ i ] );
                 tile.tiles[ i ].pixmap = 0;
             }
@@ -316,9 +321,10 @@ void TilesManager::Private::setPixmap( const QPixmap *pixmap, const NormalizedRe
 
 bool TilesManager::hasPixmap( const NormalizedRect &rect )
 {
+    NormalizedRect rotatedRect = fromRotatedRect( rect, d->rotation );
     for ( int i = 0; i < 16; ++i )
     {
-        if ( !d->hasPixmap( fromRotatedRect( rect, d->rotation ), d->tiles[ i ] ) )
+        if ( !d->hasPixmap( rotatedRect, d->tiles[ i ] ) )
             return false;
     }
 
@@ -331,7 +337,7 @@ bool TilesManager::Private::hasPixmap( const NormalizedRect &rect, const Tile &t
         return true;
 
     if ( tile.nTiles == 0 )
-        return tile.pixmap && !tile.dirty;
+        return tile.isValid();
 
     // all children tiles are clean. doesn't need to go deeper
     if ( !tile.dirty )
@@ -350,9 +356,10 @@ QList<Tile> TilesManager::tilesAt( const NormalizedRect &rect, bool allowEmpty )
 {
     QList<Tile> result;
 
+    NormalizedRect rotatedRect = fromRotatedRect( rect, d->rotation );
     for ( int i = 0; i < 16; ++i )
     {
-        d->tilesAt( fromRotatedRect( rect, d->rotation ), d->tiles[ i ], result, allowEmpty );
+        d->tilesAt( rotatedRect, d->tiles[ i ], result, allowEmpty );
     }
 
     return result;
@@ -362,17 +369,17 @@ void TilesManager::Private::tilesAt( const NormalizedRect &rect, Tile &tile, QLi
 {
     if ( !tile.rect.intersects( rect ) )
     {
-        tile.miss = qMin( tile.miss+1, RANGE_MAX );
+        tile.miss = qMin( tile.miss+1, MISSCOUNTER_MAX );
         return;
     }
 
-    // split tile (if necessary)
-    split( tile, rect );
+    // split big tiles before the requests are made, otherwise we would end up
+    // requesting huge areas unnecessarily
+    splitBigTiles( tile, rect );
 
     if ( ( allowEmpty && tile.nTiles == 0 ) || ( !allowEmpty && tile.pixmap ) )
     {
-        // TODO: check tile size
-        tile.miss = qMax( tile.miss-1, RANGE_MIN );
+        tile.miss = qMax( tile.miss-1, MISSCOUNTER_MIN );
         Tile newTile = tile;
         if ( rotation != Rotation0 )
             newTile.rect = TilesManager::toRotatedRect( tile.rect, rotation );
@@ -392,19 +399,20 @@ long TilesManager::totalMemory() const
 
 void TilesManager::cleanupPixmapMemory( qulonglong numberOfBytes )
 {
-    d->rankedTiles.clear();
+    QList<Tile*> rankedTiles;
     for ( int i = 0; i < 16; ++i )
     {
-        d->rankTiles( d->tiles[ i ] );
+        d->rankTiles( d->tiles[ i ], rankedTiles );
     }
-    qSort( d->rankedTiles.begin(), d->rankedTiles.end(), rankedTilesLessThan );
+    qSort( rankedTiles.begin(), rankedTiles.end(), rankedTilesLessThan );
 
-    while ( numberOfBytes > 0 && !d->rankedTiles.isEmpty() )
+    while ( numberOfBytes > 0 && !rankedTiles.isEmpty() )
     {
-        Tile *tile = d->rankedTiles.takeLast();
+        Tile *tile = rankedTiles.takeLast();
         if ( !tile->pixmap )
             continue;
 
+        // do not evict visible pixmaps
         if ( tile->rect.intersects( d->visibleRect ) )
             continue;
 
@@ -419,11 +427,11 @@ void TilesManager::cleanupPixmapMemory( qulonglong numberOfBytes )
         delete tile->pixmap;
         tile->pixmap = 0;
 
-        d->onClearPixmap( *tile );
+        d->markParentDirty( *tile );
     }
 }
 
-void TilesManager::Private::onClearPixmap( const Tile &tile )
+void TilesManager::Private::markParentDirty( const Tile &tile )
 {
     if ( !tile.parent )
         return;
@@ -431,14 +439,14 @@ void TilesManager::Private::onClearPixmap( const Tile &tile )
     if ( !tile.parent->dirty )
     {
         tile.parent->dirty = true;
-        onClearPixmap( *tile.parent );
+        markParentDirty( *tile.parent );
     }
 }
 
-void TilesManager::Private::rankTiles( Tile &tile )
+void TilesManager::Private::rankTiles( Tile &tile, QList<Tile*> &rankedTiles )
 {
     if ( tile.parent )
-        tile.miss = qBound( RANGE_MIN, tile.miss + tile.parent->miss, RANGE_MAX );
+        tile.miss = qBound( MISSCOUNTER_MIN, tile.miss + tile.parent->miss, MISSCOUNTER_MAX );
 
     if ( tile.pixmap )
     {
@@ -448,7 +456,7 @@ void TilesManager::Private::rankTiles( Tile &tile )
     {
         for ( int i = 0; i < tile.nTiles; ++i )
         {
-            rankTiles( tile.tiles[ i ] );
+            rankTiles( tile.tiles[ i ], rankedTiles );
         }
 
         if ( tile.nTiles > 0 )
@@ -468,6 +476,16 @@ void TilesManager::setRequest( const NormalizedRect &rect, int pageWidth, int pa
     d->requestHeight = pageHeight;
 }
 
+bool TilesManager::Private::splitBigTiles( Tile &tile, const NormalizedRect &rect )
+{
+    QRect tileRect = tile.rect.geometry( width, height );
+    if ( tileRect.width()*tileRect.height() < TILES_MAXSIZE )
+        return false;
+
+    split( tile, rect );
+    return true;
+}
+
 void TilesManager::Private::split( Tile &tile, const NormalizedRect &rect )
 {
     if ( tile.nTiles != 0 )
@@ -476,24 +494,20 @@ void TilesManager::Private::split( Tile &tile, const NormalizedRect &rect )
     if ( rect.isNull() || !tile.rect.intersects( rect ) )
         return;
 
-    QRect tileRect = tile.rect.geometry( width, height );
-    if ( tileRect.width()*tileRect.height() >= TILES_MAXSIZE )
-    {
-        tile.nTiles = 4;
-        tile.tiles = new Tile[4];
-        double hCenter = (tile.rect.left + tile.rect.right)/2;
-        double vCenter = (tile.rect.top + tile.rect.bottom)/2;
+    tile.nTiles = 4;
+    tile.tiles = new Tile[4];
+    double hCenter = (tile.rect.left + tile.rect.right)/2;
+    double vCenter = (tile.rect.top + tile.rect.bottom)/2;
 
-        tile.tiles[0].rect = NormalizedRect( tile.rect.left, tile.rect.top, hCenter, vCenter );
-        tile.tiles[1].rect = NormalizedRect( hCenter, tile.rect.top, tile.rect.right, vCenter );
-        tile.tiles[2].rect = NormalizedRect( tile.rect.left, vCenter, hCenter, tile.rect.bottom );
-        tile.tiles[3].rect = NormalizedRect( hCenter, vCenter, tile.rect.right, tile.rect.bottom );
+    tile.tiles[0].rect = NormalizedRect( tile.rect.left, tile.rect.top, hCenter, vCenter );
+    tile.tiles[1].rect = NormalizedRect( hCenter, tile.rect.top, tile.rect.right, vCenter );
+    tile.tiles[2].rect = NormalizedRect( tile.rect.left, vCenter, hCenter, tile.rect.bottom );
+    tile.tiles[3].rect = NormalizedRect( hCenter, vCenter, tile.rect.right, tile.rect.bottom );
 
-        for ( int i = 0; i < tile.nTiles; ++i )
-        {
-            tile.tiles[ i ].parent = &tile;
-            split( tile.tiles[ i ], rect );
-        }
+    for ( int i = 0; i < tile.nTiles; ++i )
+    {
+        tile.tiles[ i ].parent = &tile;
+        splitBigTiles( tile.tiles[ i ], rect );
     }
 }
 

core/tilesmanager_p.h

@@ -6,21 +6,29 @@
  *   (at your option) any later version.                                   *
  ***************************************************************************/
 
-#ifndef _OKULAR_TILES_MANAGER_H_
-#define _OKULAR_TILES_MANAGER_H_
+#ifndef _OKULAR_TILES_MANAGER_P_H_
+#define _OKULAR_TILES_MANAGER_P_H_
 
 #include "okular_export.h"
 #include "area.h"
 
-#define TILES_MAXSIZE 2000000
-
 class QPixmap;
 
 namespace Okular {
 
 /**
- * Tile is a class that stores the pixmap of a tile and its location on the
- * page
+ * This class is a node on the tree structure of tiles on tiles manager.
+ * Each node stores the pixmap of the tile and its location on the page. Each
+ * node may have children and together they cover the same area of the node
+ * itself. Leaf tiles can have children if their size is bigger than an
+ * arbitrary value.
+ *
+ * eg: Say the page is divided into 4 tiles: A, B, C and D. If the user apply a
+ * zoom to part of the page covered by A, this tile may get too big and
+ * eventually will be split into 4 different tiles: A1, A2, A3 and A4. In the
+ * tree structure these are all children of the node A.
+ *
+ * @since 0.16 (KDE 4.10)
  */
 class OKULAR_EXPORT Tile
 {
@@ -45,10 +53,15 @@ class OKULAR_EXPORT Tile
          * If a tile doesn't have a pixmap but all its children are updated
          * (dirty = false), the parent tile is also considered updated.
          */
-        bool dirty : 1;
+        bool dirty;
 
         /**
          * Children tiles
+         * When a tile is split into multiple tiles it doesn't cease to exist.
+         * It actually adds children nodes to the data structure.
+         * Since each tile is a node on the tree structure, it's easier to
+         * consider if a large area should be evaluated without visiting all
+         * its tiles (eg: when we need to list all tiles from an small area)
          */
         Tile *tiles;
         int nTiles;
@@ -68,11 +81,14 @@ class OKULAR_EXPORT Tile
  * @short Tiles management
  *
  * This class has direct access to all tiles and handle how they should be
- * stored, deleted and retrieved.
+ * stored, deleted and retrieved. Each tiles manager only handles one page.
  *
  * The tiles manager is a tree of tiles. At first the page is divided in 16
  * tiles. Then each one of these tiles can be split in more tiles (or merged
- * back to only one).
+ * back to only one) so we keep the size of each pixmap of the tile inside a
+ * safe interval.
+ *
+ * @since 0.16 (KDE 4.10)
  */
 class OKULAR_EXPORT TilesManager
 {
@@ -81,7 +97,13 @@ class OKULAR_EXPORT TilesManager
         virtual ~TilesManager();
 
         /**
-         * Use @p pixmap to paint all tiles that are contained inside @p rect
+         * Sets the pixmap of the tiles covered by @p rect (which represents
+         * the location of @p pixmap on the page).
+         * @p pixmap may cover an area which contains multiple tiles. So each
+         * tile we get a cropped part of the @p pixmap.
+         *
+         * Also it checks the dimensions of the given parameters against the
+         * current request as to avoid setting pixmaps of late requests.
          */
         void setPixmap( const QPixmap *pixmap, const NormalizedRect &rect );
 
@@ -95,6 +117,10 @@ class OKULAR_EXPORT TilesManager
         /**
          * Returns a list of all tiles intersecting with @p rect.
          *
+         * As to avoid requests of big areas, each traversed tile is checked
+         * for its size and split if necessary.
+         * Also the miss counter is updated for the use in the evicting algorithm.
+         *
          * @param allowEmpty If false only tiles with a non null pixmap are returned
          */
         QList<Tile> tilesAt( const NormalizedRect &rect, bool allowEmpty = true );
@@ -105,7 +131,9 @@ class OKULAR_EXPORT TilesManager
         long totalMemory() const;
 
         /**
-         * Removes the least ranked tiles from memory
+         * Removes at least @p numberOfBytes bytes worth of tiles (least ranked
+         * tiles are removed first).
+         * Visible tiles are not discarded.
          */
         void cleanupPixmapMemory( qulonglong numberOfBytes = 1 );
 
@@ -124,12 +152,21 @@ class OKULAR_EXPORT TilesManager
          * Inform the new width of the page and mark all tiles to repaint
          */
         void setWidth( int width );
+        /**
+         * Gets the width of the page in tiles manager
+         */
         int width() const;
+        /**
+         * Inform the new height of the page
+         */
         void setHeight( int height );
+        /**
+         * Gets the height of the page in tiles manager
+         */
         int height() const;
 
         /**
-         * Perform a rotation and mark all tiles to repaint.
+         * Inform the new rotation of the page and mark all tiles to repaint.
          */
         void setRotation( Rotation rotation );
         Rotation rotation() const;
@@ -140,7 +177,8 @@ class OKULAR_EXPORT TilesManager
         void markDirty();
 
         /**
-         * Sets the visible area of the page
+         * Sets the visible area of the page so tiles in this area will not be
+         * removed in the evicting process.
          */
         void setVisibleRect( const NormalizedRect &rect );
 
@@ -167,4 +205,4 @@ class OKULAR_EXPORT TilesManager
 
 }
 
-#endif // _OKULAR_TILES_MANAGER_H_
+#endif // _OKULAR_TILES_MANAGER_P_H_