After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668552 - iagno, gnomine, mahjongg, glchess: use a similar surface for the tile cache
iagno, gnomine, mahjongg, glchess: use a similar surface for the tile cache
Status: RESOLVED FIXED
Product: gnome-games-superseded
Classification: Deprecated
Component: iagno
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Games maintainers
GNOME Games maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-24 03:40 UTC by Allison Karlitskaya (desrt)
Modified: 2012-02-22 20:09 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
iagno: use a similar surface for the tile cache (1.35 KB, patch)
2012-01-24 03:40 UTC, Allison Karlitskaya (desrt) 		committed Details | Review
Mahjongg: Cairo.Surface.similar (1010 bytes, patch)
2012-01-25 05:20 UTC, Tiffany Antopolski 		committed Details | Review
Gnomine: Cairo.Surface.similar (3.70 KB, patch)
2012-01-25 05:21 UTC, Tiffany Antopolski 		committed Details | Review
GlChess: Cairo.Surface.similar (2.61 KB, patch)
2012-01-25 05:21 UTC, Tiffany Antopolski 		reviewed Details | Review
no more unsafe cast. (2.74 KB, patch)
2012-01-25 22:20 UTC, Tiffany Antopolski 		needs-work Details | Review

Description Allison Karlitskaya (desrt) 2012-01-24 03:40:05 UTC 
The current drawing code is somewhat suboptimal.  See patch for a fix.
Comment 1 Allison Karlitskaya (desrt) 2012-01-24 03:40:06 UTC 
Created attachment 205939 [details] [review]
iagno: use a similar surface for the tile cache

This stores the cached tiles in the X server (probably in video memory)
which makes rendering them quite a lot faster.  We were currently
storing them locally and sending them to the X server for each frame.
This should solve any problems with poor animation performance.
Comment 2 Allison Karlitskaya (desrt) 2012-01-24 03:41:49 UTC 
I did some tests by doing the drawing code 1000 times in a loop.  This change increases performance by a factor of 2 on the initial board, working up to more like a 100 times improvement with a full board and a lot going on (like two computer players against each other).
Comment 3 Robert Ancell 2012-01-24 04:10:21 UTC 
Review of attachment 205939 [details] [review]:

Oh, nice. I thought ImageSurface was going to do that for us.
Comment 4 Robert Ancell 2012-01-24 04:11:30 UTC 
Note this same fix should be done in gnomine, mahjongg, glchess too.
Comment 5 Tiffany Antopolski 2012-01-25 05:20:00 UTC 
Created attachment 206054 [details] [review]
Mahjongg: Cairo.Surface.similar

Following the example of the iagno fix, this hopefully fixes mahjongg.
Comment 6 Tiffany Antopolski 2012-01-25 05:21:09 UTC 
Created attachment 206055 [details] [review]
Gnomine: Cairo.Surface.similar

...and Gnomine
Comment 7 Tiffany Antopolski 2012-01-25 05:21:59 UTC 
Created attachment 206056 [details] [review]
GlChess: Cairo.Surface.similar

...and GlChess
Comment 8 Allison Karlitskaya (desrt) 2012-01-25 05:24:05 UTC 
Review of attachment 206054 [details] [review]:

::: mahjongg/src/game-view.vala
@@ -72,3 @@
             var height = image_height * 2;
 
-            var surface = new Cairo.ImageSurface (Cairo.Format.ARGB32, width, height);

it's slightly odd that you changed ARGB for COLOR (instead of COLOR_ALPHA), but if it works it's probably okay (ie: the images have no transparent content)
Comment 9 Allison Karlitskaya (desrt) 2012-01-25 05:26:14 UTC 
Review of attachment 206056 [details] [review]:

::: glchess/src/chess-view-2d.vala
@@ +212,3 @@
             c.rotate (Math.PI);
 
+        var size = ((Cairo.ImageSurface) surface).get_height ();

this cast is almost certainly unsafe...
Comment 10 Allison Karlitskaya (desrt) 2012-01-25 05:38:38 UTC 
Review of attachment 206055 [details] [review]:

looks good
Comment 11 Tiffany Antopolski 2012-01-25 22:20:52 UTC 
Created attachment 206138 [details] [review]
no more unsafe cast.
Comment 12 Robert Ancell 2012-02-04 10:43:21 UTC 
Review of attachment 206054 [details] [review]:

Worked for me!
Comment 13 Robert Ancell 2012-02-04 10:47:32 UTC 
Review of attachment 206055 [details] [review]:

Worked for me.
Comment 14 Robert Ancell 2012-02-04 11:39:23 UTC 
Review of attachment 206138 [details] [review]:

::: glchess/src/chess-view-2d.vala
@@ +214,3 @@
+        var size = 0;
+
+        if (surface == model_surface)

This is a bit unsafe - if we ever added a third surface it wouldn't be clear this method should be updated.  Instead change this to a method parameter and this patch is good to go.
Comment 15 Tiffany Antopolski 2012-02-20 16:25:54 UTC 
Just to clarify, you want me to pass the  'size' in is as a parameter?
Comment 16 Robert Ancell 2012-02-20 22:16:28 UTC 
yes