<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/105292/">http://git.reviewboard.kde.org/r/105292/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 25th, 2012, 6:57 a.m., <b>Thorsten Zachmann</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;">When saving I get the following error when validating the saved document.
content.xml:8:419: error: bad value for attribute "vertical-segments" from namespace "urn:oasis:names:tc:opendocument:xmlns:dr3d:1.0"
content.xml:46:205: error: required attributes missing
content.xml:47:205: error: required attributes missing
The value for vertical-segments has a % at the end which should not be there.
The draw:rotate miss the svg:viewBox attribute which is mandetory.
I think the code should not be committed before roundtripping to LO/OO does not work as this is the only thing it tries to archive up to now.</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;">Good. This may be the problem. Btw, which tool are you using to get these diagnostics?
And I agree about roundtripping to LO/OO.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 25th, 2012, 6:57 a.m., <b>Thorsten Zachmann</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="http://git.reviewboard.kde.org/r/105292/diff/2/?file=70349#file70349line81" style="color: black; font-weight: bold; text-decoration: underline;">plugins/staging/threedshape/Object3D.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class KoXmlWriter;</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">81</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">// For now we have chosen option 2.</span></pre></td>
</tr>
<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">82</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">//</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;">I think that is a bad descition as when the layers where implemented we made the explicit design decision that we use hierarchy for the shapes and when a layer does not match the parent we fix that during loading.
If it is only for having style information available I think it is much easier to reuse only the style loading code and don't inherit the shapes as shape are also painted by the shape manager but I'm not sure that is wanted here.</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;">If there is such an explicit design decision about layers, then option 2 is not just a bad decision, it's totally wrong. But as it says in the explanation it has nothing to do with styling. The styling works fine as it is.
As a side note, I think that these design decisions should be documented somewhere, e.g. at the community.kde.org wiki.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 25th, 2012, 6:57 a.m., <b>Thorsten Zachmann</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="http://git.reviewboard.kde.org/r/105292/diff/2/?file=70349#file70349line99" style="color: black; font-weight: bold; text-decoration: underline;">plugins/staging/threedshape/Object3D.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class KoXmlWriter;</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">99</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">virtual</span> <span class="kt">void</span> <span class="n">saveOdf2</span><span class="p">(</span><span class="n">KoShapeSavingContext</span> <span class="o">&</span><span class="n">context</span><span class="p">)</span> <span class="k">const</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;">saveOdf2 is a very bad name in my oppinion. How about using saveObjectOdf?</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;">Yeah, that is a better name.
But if we go back to just having the entire scene in one layer, then the whole method becomes unnecessary so the name doesn't matter. :)</pre>
<br />
<p>- Inge</p>
<br />
<p>On June 25th, 2012, 4:52 a.m., Inge Wallin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Inge Wallin.</div>
<p style="color: grey;"><i>Updated June 25, 2012, 4:52 a.m.</i></p>
<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;">This patch adds rudimentary support for the dr3d:scene element and its children by introducing a 3D shape. The 3D support in ODF is pretty simple: it just defines 4 different object types: cube, sphere, extrude and rotate as well as a scene element that acts a bit like a group (draw:g).
I implemented all the object types as KoShapes because they can have styling and the scene object is also a KoShapeContainer. None of the shapes except for the scene itself can be modified now, and for the scene it's only the standard shape parameters (size, position, transform, etc).
My plan is to work in 3 stages:
1. Loading and saving - this prevents data loss
2. Rendering - this implements viewing
3. Scene and object editing
This patch implements stage 1 only.
My goal was to load dr3d:scenes and save them back with full compatibility with OOo and LO. Unfortunately I didn't manage to do that yet.
I have tested with Karbon during the development and it loads and savs the scenes nicely. But calling KoShape::loadOdfAttributes(..., OdfAllAttributes) in karbon still loses the layer information. This makes OOo/LO not show the shapes when they are loaded again after a roundtrip through Karbon. I have not been able to analyze why Karbon saves back all shapes with layer="" when the infile clearly has layer="layout".
And if I want to make it work in Words and presumably also Stage (and Sheets?) I have to integrate it with the KoInlineObjectRegistry in kotext, something that I have also not yet managed to do. Help would be appreciated here.
But until Karbon is fixed and Words integration is done, I'm happy to receive and integrate feedback here.</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;">Tested with a simple ODG file containing 3D objects.</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>plugins/staging/CMakeLists.txt <span style="color: grey">(f55b316)</span></li>
<li>plugins/staging/threedshape/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/Messages.sh <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/Object3D.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/Object3D.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/Objects.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/Objects.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/PLAN <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/SceneObject.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/SceneObject.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/ThreedShapeFactory.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/ThreedShapeFactory.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/ThreedShapePlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/ThreedShapePlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/threedshape.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/utils.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/staging/threedshape/utils.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105292/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>