<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112873/">http://git.reviewboard.kde.org/r/112873/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So i had a quick functional test, i.e. i started kreversi with the patch and played a bit of reversi with it. Looks good, can't see much difference, not sure if that's good or bad :D I guess good :-)
What i can see is that in the old version there seems to be some pixmap cache around, so if i maximize the window and then double click on the title to get to the previous size and then maximize again, the second maximize is very fast, while now it takes the same time as the first time. Nothing huge though.
I have not looked at the code because it's very hard to review, there's millions of space changes, which make looking at the diff very painful because for every single line I have to look very carefully to see if there's any real change of just a space change. Do you think you could try to reduce the diff removing this changes?
Also I'd like to hear Viranch's opinion :.)</pre>
<br />
<p>- Albert Astals Cid</p>
<br />
<p>On September 21st, 2013, 7:18 p.m. UTC, Denis Kuplyakov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Games and Viranch Mehta.</div>
<div>By Denis Kuplyakov.</div>
<p style="color: grey;"><i>Updated Sept. 21, 2013, 7:18 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kreversi
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">QML port for KReversi.
List of changes:
1) KReversiChip and KReversiScene classes were deleted as they not needed
2) default_theme.desktop files is now installed to make KgThemeProvider discover it
3) Added folowing QML files:
Table (contains Board and Popup, draws background)
Popup (implements KgPopupItem)
Board (draws board with labels and grid of Cells)
Cell (represents cell of board, contains Chip and displays various marks)
Chip (draws chip and it's animation)
CanvasItem (wrapper to acces themeProvider allover the code),
globals (used to store constants)
4) Fixed bug: "If you undo move, last move marker isn't showing"
5) KReversiView was compeletely rewritten to use QML implementation.
6) Minor changes at other files: added credits to me, deleted QGraphicsScene specific things
7) Introduced ColorScheme class to access KColorScheme color from QML.
8) Documentation is uptodate now.
Current bugs: I haven't found any ;)
Future plans:
See http://kreversiqml.blogspot.ru/2013/08/new-kreversi-design.html . All of this is implemented at deniskup/gsoc2013/newdesign branch. Now I am working on
1) post a patch to kdelibs that will make possible using of ColorScheme class (I mean newdesign implementation here)
2) export ColorScheme class to kdelibs as KColorSchemeToken
* search for "KDE theme colors API for QML" thread at kdelibs mailinglists *
3) export Popup.qml to libkdegames
4) apply newdesign</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Have played it many times, it seems to work right.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>AUTHORS <span style="color: grey">(93f0925)</span></li>
<li>CMakeLists.txt <span style="color: grey">(8f650f8)</span></li>
<li>DESIGN <span style="color: grey">(d2a1320)</span></li>
<li>TODO <span style="color: grey">(97aedcd)</span></li>
<li>colorscheme.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>colorscheme.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kreversichip.h <span style="color: grey">(37695b9)</span></li>
<li>kreversichip.cpp <span style="color: grey">(f6e909d)</span></li>
<li>kreversigame.h <span style="color: grey">(d6b8a43)</span></li>
<li>kreversigame.cpp <span style="color: grey">(4fb231b)</span></li>
<li>kreversiscene.h <span style="color: grey">(6880a67)</span></li>
<li>kreversiscene.cpp <span style="color: grey">(2731f78)</span></li>
<li>kreversiview.h <span style="color: grey">(ff3db89)</span></li>
<li>kreversiview.cpp <span style="color: grey">(1c80e4b)</span></li>
<li>main.cpp <span style="color: grey">(dd05bd4)</span></li>
<li>mainwindow.h <span style="color: grey">(c9c3160)</span></li>
<li>mainwindow.cpp <span style="color: grey">(1855b16)</span></li>
<li>pics/CMakeLists.txt <span style="color: grey">(744ec7b)</span></li>
<li>qml/Board.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/CanvasItem.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/Cell.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/Chip.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/Popup.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/Table.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>qml/globals.js <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112873/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>