Bug 62109 - Redrawing is very slow over network connection
Summary: Redrawing is very slow over network connection
Status: RESOLVED FIXED
Alias: None
Product: kmahjongg
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Mauricio Piacentini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-08-04 14:08 UTC by Daniel Schepler
Modified: 2006-10-27 05:12 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Schepler 2003-08-04 14:08:47 UTC
Version:            (using KDE KDE 3.1.3)
Compiler:          gcc version 3.3.1 20030728 (Debian prerelease) 
OS:          Linux

(This is forwarded from Debian bug #203421.)

I'm currently playing Kmahjongg over a 10 MBit half duplex line. Every
time I click at a piece it takes some seconds until the screen is
updated. I think it is because the complete client area of Kmahjongg is
calculated and then drawn on the screen.

For me, it would be better if only the area that really changed would be
updated, as this would save a lot of bandwidth :)

Suggestion: You could always have a memory bitmap of the current screen
and then calculate the new screen as another memory bitmap. Then you
could check which lines are different, and only make these lines
candidates for repainting. Then, for every line you could calculate the
interval that actually changed and then paint that interval.

If that algorithm is too slow for normal viewing (local, with graphics
accelerator), you could add a configuration option to switch between
these algorithms.

[I don't think this is a good suggestion, myself.  If the whole board is really being redrawn, I'd say the proper fix is to make sure the paint event handler only draws tiles in the update region, and to limit update() calls to the changed rectangles.]
Comment 1 Nicolas L. 2005-08-23 16:56:43 UTC
is it still valid for you ?
Comment 2 Mauricio Piacentini 2006-08-25 21:35:46 UTC
Yes, I have noticed this already, and you are right about the source of the problem. The whole field is redrawn when tiles are added or removed, this has to do with calculation of shadows, among other issues. We definately need to optmize this.
I am working on enhancements for KMahjongg in KDE4, you can read more about this at
http://tsdgeos.blogspot.com/2006/08/kmahjongg-revamp.html

I will definately try to address this issue, by enabling partial invalidation of the rects, and more selective redrawing. I noticed that in my new test build (using SVG and big tiles) that this issue is even more important with scalable tilesets. When the play field are gets bigger (1200X700 for example) redraws are slow even on the local machine, and cpu usage spikes to 50% or more when the game is simply blinking the hint tile. 

Too bad the resolution is not simple, it basically involves rewriting the drawing code... It will take a while, but I plan to have it ready for KDE4.
Comment 3 Mauricio Piacentini 2006-10-27 05:12:20 UTC
SVN commit 599425 by piacentini:

After a lot of preparation, finally removed full redraw at every tile 
addition and removal. There is still some room for optimization, but at 
800x600 play area the game is now using less than 1% of a 2GHz cpu, 
with minimal redrawings. 
Unfortunately, this change is for KDE4 only and  can not be backported 
to 3.5.x series.
BUG: 62109
CCBUG: 62109


 M  +52 -19    boardwidget.cpp  
 M  +2 -1      boardwidget.h  


--- trunk/KDE/kdegames/kmahjongg/boardwidget.cpp #599424:599425
@@ -161,8 +161,7 @@
   resize(width() , height());
 }
 
-
-void BoardWidget::updateSpriteMap() {
+void BoardWidget::populateSpriteMap() {
     QPixmap  *back;
 
     int xx = rect().left();
@@ -175,11 +174,9 @@
 
     spriteMap.clear();
 
-    //if (!backsprite) {
-      back = theBackground.getBackground();
-      backsprite = new KGameCanvasPixmap(*back, this);
-      backsprite->show();
-    //}
+    back = theBackground.getBackground();
+    backsprite = new KGameCanvasPixmap(*back, this);
+    backsprite->show();
 
     // initial offset on the screen of tile 0,0
     int xOffset = theTiles.width()/2;
@@ -210,25 +207,53 @@
 				Game->BoardData(z,y,x)-TILE_OFFSET);
                 }
 
-		//if (!spriteMap.contains(QString("X%1Y%2Z%3").arg(x).arg(y).arg(z)))
-		//{
 		  KGameCanvasPixmap * thissprite = new KGameCanvasPixmap(*t, this);
 
-		  //if (Game->tilePresent(z, y-1, x-2)) {
-		    //qDebug() << "overlap at " << y << x;
-		  //}
 		  thissprite->moveTo(sx, sy);
 		  thissprite->show();
-		  //spriteMap.insert(QString("X%1Y%2Z%3").arg(x).arg(y).arg(z), thissprite);
 		  spriteMap.insert(TileCoord(x,y,z), thissprite);
-		//}
             }
         }
         xOffset +=theTiles.levelOffset();
         yOffset -=theTiles.levelOffset();
     }
 
+    updateSpriteMap();
+}
+
+
+void BoardWidget::updateSpriteMap() {
+    // initial offset on the screen of tile 0,0
+    int xOffset = theTiles.width()/2;
+    int yOffset = theTiles.height()/2;
+    //short tile = 0;
+
+    // we iterate over the depth stacking order. Each successive level is
+    // drawn one indent up and to the right. The indent is the width
+    // of the 3d relief on the tile left (tile shadow width)
     for (int z=0; z<Game->m_depth; z++) {
+        // we draw down the board so the tile below over rights our border
+        for (int y = 0; y < Game->m_height; y++) {
+            // drawing right to left to prevent border overwrite
+            for (int x=Game->m_width-1; x>=0; x--) {
+                int sx = x*(theTiles.qWidth()  )+xOffset;
+                int sy = y*(theTiles.qHeight()  )+yOffset;
+
+		// skip if no tile to display
+		if (!Game->tilePresent(z,y,x))
+			continue;
+
+		  KGameCanvasPixmap * thissprite =spriteMap.value(TileCoord(x,y,z));
+
+		  if (thissprite) thissprite->moveTo(sx, sy);
+		  if (thissprite) thissprite->show();
+            }
+        }
+        xOffset +=theTiles.levelOffset();
+        yOffset -=theTiles.levelOffset();
+    }
+
+    for (int z=0; z<Game->m_depth; z++) {
         // start drawing in diagonal for correct layering. For this we double the board, 
 	//actually starting outside of it, in the bottom right corner, so our first diagonal ends
 	// at the actual top right corner of the board
@@ -250,7 +275,6 @@
     //qDebug() << items()->size();
     //qDebug() << spriteMap.size();
     update();
-
 }
 
 // for a given cell x y calc how that cell is shadowed
@@ -773,9 +797,7 @@
 void BoardWidget::drawBoard(bool )
 {
    updateBackBuffer=true;
-  // updateSpriteMap();
-  // update(); 
-   updateSpriteMap();
+   populateSpriteMap();
    drawTileNumber();
 }
 
@@ -789,6 +811,14 @@
 	// we ensure that any tile we put on has highlighting off
     Game->putTile( E, Y, X, Pos.f );
     Game->setHighlightData(E,Y,X,0);
+
+    QPixmap *t;
+    t= theTiles.unselectedPixmaps(Game->BoardData(E,Y,X)-TILE_OFFSET);
+    KGameCanvasPixmap * thissprite = new KGameCanvasPixmap(*t, this);
+    //thissprite->moveTo(sx, sy);
+    thissprite->show();
+     spriteMap.insert(TileCoord(X,Y,E), thissprite);
+
     if (doRepaint) {
 	updateBackBuffer=true;
 	update(); 
@@ -808,12 +838,15 @@
     Game->TileNum--;                    // Eine Figur weniger
     Game->setMoveListData(Game->TileNum,Pos); // Position ins Protokoll eintragen
 
+    KGameCanvasPixmap * thissprite =spriteMap.value(TileCoord(X,Y,E));
+    if (thissprite) delete thissprite;
+    spriteMap.remove(TileCoord(X,Y,E));
     // remove tile from game board
     Game->putTile( E, Y, X, 0 );
     if (doRepaint) {
         updateBackBuffer=true;
 	update(); 
-	updateSpriteMap();
+//	updateSpriteMap();
     }
 }
 
--- trunk/KDE/kdegames/kmahjongg/boardwidget.h #599424:599425
@@ -86,7 +86,8 @@
         bool loadBoard      ( );
         void updateScaleMode ();
         void drawBoard(bool deferUpdate = true);
-	void updateSpriteMap(); 
+	void updateSpriteMap();
+	void populateSpriteMap();
         bool loadBackground ( const QString&, bool bShowError = true );
     signals:
         void statusTextChanged ( const QString&, long );
Comment 4 Mauricio Piacentini 2006-10-27 05:12:22 UTC
SVN commit 599425 by piacentini:

After a lot of preparation, finally removed full redraw at every tile 
addition and removal. There is still some room for optimization, but at 
800x600 play area the game is now using less than 1% of a 2GHz cpu, 
with minimal redrawings. 
Unfortunately, this change is for KDE4 only and  can not be backported 
to 3.5.x series.
BUG: 62109
CCBUG: 62109


 M  +52 -19    boardwidget.cpp  
 M  +2 -1      boardwidget.h  


--- trunk/KDE/kdegames/kmahjongg/boardwidget.cpp #599424:599425
@@ -161,8 +161,7 @@
   resize(width() , height());
 }
 
-
-void BoardWidget::updateSpriteMap() {
+void BoardWidget::populateSpriteMap() {
     QPixmap  *back;
 
     int xx = rect().left();
@@ -175,11 +174,9 @@
 
     spriteMap.clear();
 
-    //if (!backsprite) {
-      back = theBackground.getBackground();
-      backsprite = new KGameCanvasPixmap(*back, this);
-      backsprite->show();
-    //}
+    back = theBackground.getBackground();
+    backsprite = new KGameCanvasPixmap(*back, this);
+    backsprite->show();
 
     // initial offset on the screen of tile 0,0
     int xOffset = theTiles.width()/2;
@@ -210,25 +207,53 @@
 				Game->BoardData(z,y,x)-TILE_OFFSET);
                 }
 
-		//if (!spriteMap.contains(QString("X%1Y%2Z%3").arg(x).arg(y).arg(z)))
-		//{
 		  KGameCanvasPixmap * thissprite = new KGameCanvasPixmap(*t, this);
 
-		  //if (Game->tilePresent(z, y-1, x-2)) {
-		    //qDebug() << "overlap at " << y << x;
-		  //}
 		  thissprite->moveTo(sx, sy);
 		  thissprite->show();
-		  //spriteMap.insert(QString("X%1Y%2Z%3").arg(x).arg(y).arg(z), thissprite);
 		  spriteMap.insert(TileCoord(x,y,z), thissprite);
-		//}
             }
         }
         xOffset +=theTiles.levelOffset();
         yOffset -=theTiles.levelOffset();
     }
 
+    updateSpriteMap();
+}
+
+
+void BoardWidget::updateSpriteMap() {
+    // initial offset on the screen of tile 0,0
+    int xOffset = theTiles.width()/2;
+    int yOffset = theTiles.height()/2;
+    //short tile = 0;
+
+    // we iterate over the depth stacking order. Each successive level is
+    // drawn one indent up and to the right. The indent is the width
+    // of the 3d relief on the tile left (tile shadow width)
     for (int z=0; z<Game->m_depth; z++) {
+        // we draw down the board so the tile below over rights our border
+        for (int y = 0; y < Game->m_height; y++) {
+            // drawing right to left to prevent border overwrite
+            for (int x=Game->m_width-1; x>=0; x--) {
+                int sx = x*(theTiles.qWidth()  )+xOffset;
+                int sy = y*(theTiles.qHeight()  )+yOffset;
+
+		// skip if no tile to display
+		if (!Game->tilePresent(z,y,x))
+			continue;
+
+		  KGameCanvasPixmap * thissprite =spriteMap.value(TileCoord(x,y,z));
+
+		  if (thissprite) thissprite->moveTo(sx, sy);
+		  if (thissprite) thissprite->show();
+            }
+        }
+        xOffset +=theTiles.levelOffset();
+        yOffset -=theTiles.levelOffset();
+    }
+
+    for (int z=0; z<Game->m_depth; z++) {
         // start drawing in diagonal for correct layering. For this we double the board, 
 	//actually starting outside of it, in the bottom right corner, so our first diagonal ends
 	// at the actual top right corner of the board
@@ -250,7 +275,6 @@
     //qDebug() << items()->size();
     //qDebug() << spriteMap.size();
     update();
-
 }
 
 // for a given cell x y calc how that cell is shadowed
@@ -773,9 +797,7 @@
 void BoardWidget::drawBoard(bool )
 {
    updateBackBuffer=true;
-  // updateSpriteMap();
-  // update(); 
-   updateSpriteMap();
+   populateSpriteMap();
    drawTileNumber();
 }
 
@@ -789,6 +811,14 @@
 	// we ensure that any tile we put on has highlighting off
     Game->putTile( E, Y, X, Pos.f );
     Game->setHighlightData(E,Y,X,0);
+
+    QPixmap *t;
+    t= theTiles.unselectedPixmaps(Game->BoardData(E,Y,X)-TILE_OFFSET);
+    KGameCanvasPixmap * thissprite = new KGameCanvasPixmap(*t, this);
+    //thissprite->moveTo(sx, sy);
+    thissprite->show();
+     spriteMap.insert(TileCoord(X,Y,E), thissprite);
+
     if (doRepaint) {
 	updateBackBuffer=true;
 	update(); 
@@ -808,12 +838,15 @@
     Game->TileNum--;                    // Eine Figur weniger
     Game->setMoveListData(Game->TileNum,Pos); // Position ins Protokoll eintragen
 
+    KGameCanvasPixmap * thissprite =spriteMap.value(TileCoord(X,Y,E));
+    if (thissprite) delete thissprite;
+    spriteMap.remove(TileCoord(X,Y,E));
     // remove tile from game board
     Game->putTile( E, Y, X, 0 );
     if (doRepaint) {
         updateBackBuffer=true;
 	update(); 
-	updateSpriteMap();
+//	updateSpriteMap();
     }
 }
 
--- trunk/KDE/kdegames/kmahjongg/boardwidget.h #599424:599425
@@ -86,7 +86,8 @@
         bool loadBoard      ( );
         void updateScaleMode ();
         void drawBoard(bool deferUpdate = true);
-	void updateSpriteMap(); 
+	void updateSpriteMap();
+	void populateSpriteMap();
         bool loadBackground ( const QString&, bool bShowError = true );
     signals:
         void statusTextChanged ( const QString&, long );