<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="https://git.reviewboard.kde.org/r/117816/">https://git.reviewboard.kde.org/r/117816/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 28th, 2014, 11:15 p.m. UTC, <b>Commit Hook</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit 6a896c373e0940c09a5efae6ee067bb9f59be7e0 by Percy Camilo Triveño Aucahuasi to branch master.</pre>
</blockquote>
<p>On April 28th, 2014, 11:22 p.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Oh come on. You're a mathematician, 4 or 6, what difference does it make? ;)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">hehehe ... :D :D ok If we use 8 as initial cube edge, then some examples (surfaces) in plots3ddemo will not be plotted totally. I guess, this was because our old implementation covered more space at the initial iteration (partition) see:
octree.cpp:25 raiz->cubo.setCenter(initial_edge/2,initial_edge/2,initial_edge/2);
With 12 as new size the issue is solved and the surfaces are plotted ok.
If you want a more deeply explanation of this topic, then the answer is in the Octree. Since we were using an octree based space partition method, instead Aqsis approach is using a fixed interval to traverse through x,y and z. The reference to create a new space in the octree method was the center of the father/old cube.
octree.cpp:47 father->nodes[i]->cube.setHalfEdge(father->cube.halfEdge()/2);
Instead, in Aqsis, the reference to generate a new cube is the previous one, since you just need to jump to the next cube using the interval length. To jump intervals we have this variables, in new version of marchingcubes.cpp:98-99, see:
hx = (xmax-xmin)/i_size_x;
hy = (ymax-ymin)/i_size_y;
hz = (zmax-zmin)/i_size_z;
In any case, and for sure, I trust more in Aqsis implementation than our old one.
If you need more explanation, I can study this to confirm and point out wich of those quotients in octree (that are edge/2) are causing the difference ;)
:p</pre>
<br />
<p>- Percy Camilo</p>
<br />
<p>On April 28th, 2014, 11:15 p.m. UTC, Percy Camilo Triveño Aucahuasi wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Edu.</div>
<div>By Percy Camilo Triveño Aucahuasi.</div>
<p style="color: grey;"><i>Updated April 28, 2014, 11:15 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
analitza
</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;">Replace our marching cubes algorithm implementation for the one of Aqsis project. The Aqsis implementation is fastest and offers topological guarantees, in other words we'll have better surfaces as result (without missing faces, etc.)
Finally, we'll have less code to maintain :)</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;">Builds ok.
All analitzaplot tests pass.
Also, there is no licenses issues since the code is GPL >= 2 (same of Analitza)</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>analitzaplot/private/utils/octree.cpp <span style="color: grey">(6106120)</span></li>
<li>analitzaplot/private/utils/octree.h <span style="color: grey">(1cf4b24)</span></li>
<li>analitzaplot/private/utils/marchingcubes.cpp <span style="color: grey">(49331f1)</span></li>
<li>analitzaplot/private/utils/marchingcubes.h <span style="color: grey">(ef0b0c4)</span></li>
<li>analitzaplot/private/utils/lookuptable.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>analitzaplot/private/backends/implicitsurface.cpp <span style="color: grey">(68c7918)</span></li>
<li>analitzaplot/CMakeLists.txt <span style="color: grey">(9890449)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117816/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>