<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/118310/">https://git.reviewboard.kde.org/r/118310/</a>
     </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On November 18th, 2014, 11:51 a.m. UTC, <b>David Narváez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  


<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/118310/diff/2/?file=328741#file328741line55" style="color: black; font-weight: bold; text-decoration: underline;">geogebra/geogebrasection.h</a>
    <span style="font-weight: normal;">

     (Diff revision 2)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">55</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">std</span><span class="o">::</span><span class="n">vector</span><span class="o"><</span><span class="n">ObjectDrawer</span><span class="o">*></span> <span class="n">m_drawers</span><span class="p">;</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If there will be exactly one drawer in m_drawers as there will be output objects in m_outputObjects, make m_outputObjects a vector of pair of ObjectCalcer<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> and ObjectDrawer</em></p></pre>
 </blockquote>



 <p>On November 21st, 2014, 1:43 p.m. UTC, <b>Aniket Anvit</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Actually I had thought of it but we can't use a vector pair since while reading tools we would need to construct objectHierarchy from the inputObjects and the outputObjects. If the outputObjects is a vector< pair<ObjectCalcer<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">, ObjectDrawer</em> >> then it won't be possible.</p></pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Right, nice catch. Add a comment explaining m_drawers needs to be kept separate for that reason.</p></pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On November 18th, 2014, 11:51 a.m. UTC, <b>David Narváez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  


<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line154" style="color: black; font-weight: bold; text-decoration: underline;">geogebra/geogebratransformer.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 2)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void GeogebraTransformer::attribute( const QXmlName& name, const QStringRef& value )</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">154</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">      <span class="n">m_pointType</span> <span class="o">=</span> <span class="mi">2</span><span class="p">;</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Use int ObjectDrawer::pointStyleFromString( const QString& style ), which is evil but not worse than magic constants.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm thinking this pointType thing should be an Enum and we could use the Q_ENUM facility instead of these conversions, but this is out of the scope of this patch.</p></pre>
 </blockquote>



 <p>On November 21st, 2014, 1:43 p.m. UTC, <b>Aniket Anvit</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I saw the documentation on how to use Q_ENUM along with Q_PROPERTY and I liked it. Tell me if you would like to go with it.</p></pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Use int ObjectDrawer::pointStyleFromString( const QString& style ) for now</p></pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On November 18th, 2014, 11:51 a.m. UTC, <b>David Narváez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  


<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line156" style="color: black; font-weight: bold; text-decoration: underline;">geogebra/geogebratransformer.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 2)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void GeogebraTransformer::attribute( const QXmlName& name, const QStringRef& value )</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">156</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">      <span class="n">m_pointType</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span>   <span class="c1">// should be mapped to RoundEmpty ( i.e. 1) but for some reason it is not drawing in KIG</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Set it to RoundEmpty and then either figure out why is it not drawing, or add a TODO comment here</p></pre>
 </blockquote>



 <p>On November 21st, 2014, 1:43 p.m. UTC, <b>Aniket Anvit</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">All the pointStyles are drawing fine except the hollow ones ( RoundEmpty and RectangularEmpty ). Can't seem to figure out why!</p></pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I can't guess what not drawing fine means in this context, but if Kig doesn't crash, use int ObjectDrawer::pointStyleFromString( const QString& style ) regardless, don't just set it to 0. If it doesn't work we'll have to figure out why later.</p></pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On November 18th, 2014, 11:51 a.m. UTC, <b>David Narváez</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  


<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://git.reviewboard.kde.org/r/118310/diff/2/?file=328744#file328744line158" style="color: black; font-weight: bold; text-decoration: underline;">geogebra/geogebratransformer.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 2)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void GeogebraTransformer::attribute( const QXmlName& name, const QStringRef& value )</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">158</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">      <span class="n">m_pointType</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span>   <span class="c1">// should be mapped to RectangularEmpty ( i.e. 3) but for some reason it is not drawing in KIG</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Set it to RectangularEmpty and then either figure out why is it not drawing, or add a TODO comment here</p></pre>
 </blockquote>





</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I can't guess what not drawing fine means in this context, but if Kig doesn't crash, use int ObjectDrawer::pointStyleFromString( const QString& style ) regardless, don't just set it to 0. If it doesn't work we'll have to figure out why later.</p></pre>
<br />




<p>- David</p>


<br />
<p>On November 21st, 2014, 1:43 p.m. UTC, Aniket Anvit wrote:</p>









<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
 <tr>
  <td>

<div>Review request for KDE Edu and David Narváez.</div>
<div>By Aniket Anvit.</div>


<p style="color: grey;"><i>Updated Nov. 21, 2014, 1:43 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kig
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch enables KIG to read the graphic properties of Geogebra objects from Geogebra worksheets.
Here is some explanation about my implementation:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">the graphic properties ( lineStyle, pointStyle, lineWidth, color ) are read for each geogebra object. I add an attribute for each property to each of the intermediate representation of each of the objects created using XSLT. Some variables are added to the KigFilterGeogebra class to hold the values of the properties for each of the objects. In the attribute() callback method of the KigFilterGeogebra class , the values of the properties are read ( and suitably mapped to KIG's ) and when the endElement() callback is called a new ObjectDrawer with the last read properties is allocated for this object.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested! some trouble in the mapping of PointStyles.</p></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>filters/geogebra-filter.cpp <span style="color: grey">(cff342d)</span></li>

 <li>geogebra/geogebra.xsl <span style="color: grey">(c1e4749)</span></li>

 <li>geogebra/geogebrasection.h <span style="color: grey">(aed7dcb)</span></li>

 <li>geogebra/geogebrasection.cpp <span style="color: grey">(e9778ed)</span></li>

 <li>geogebra/geogebratransformer.h <span style="color: grey">(5f36827)</span></li>

 <li>geogebra/geogebratransformer.cpp <span style="color: grey">(aee8669)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/118310/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>


 <li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/11/15/9799362b-472e-4814-9d78-da2bf07d612c__gui.ggb">Test file</a></li>

</ul>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>