[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request 120733: Pass more data between layout and RootAreaProvider
From: "Elvis Stansvik" <elvstone () gmail ! com>
Date: 2014-11-27 21:49:17
Message-ID: 20141127214917.11306.29063 () probe ! kde ! org
[Download RAW message or body]
--===============2509021920089941830==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On nov 27, 2014, 9:14 e.m., Camilla Boemann wrote:
> > libs/textlayout/KoTextDocumentLayout.cpp, line 858
> > <https://git.reviewboard.kde.org/r/120733/diff/7-8/?file=330774#file330774line858>
> >
> > but doing nothing and not incrmenting ??
either this objection is too complex for me to understand, or you are misreading :) \
this is the `while` part of the big `do-while` loop, which does tons of stuff, right? \
(the `while` should probably be moved up to the same line as the closing `}`)
- Elvis
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120733/#review71039
-----------------------------------------------------------
On nov 27, 2014, 9:05 e.m., Pierre Ducroquet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120733/
> -----------------------------------------------------------
>
> (Updated nov 27, 2014, 9:05 e.m.)
>
>
> Review request for Calligra and Camilla Boemann.
>
>
> Repository: calligra
>
>
> Description
> -------
>
> This patch refactors a bit the KoTextLayoutRootAreaProvider.
> It extends the provideNext call by adding a constraints structure.
> This structure contains so far the masterPageName and the page
> number of the requested new rootArea.
> This way, the provider don't have to sneak in layout specific data.
> It also adds a way for KoTextLayoutRootAreaProvider to help the
> layout and fix inserting a new page (by adding lines to a paragraph
> for instance) before a page break changing the page style.
>
> Some more explanations regarding this bug :
> - Page 1, style AAA
> * page break, requesting style BBB
> - Page 2, style BBB
>
> If you add more data to the page 1, then dynamically the layout should
> do :
> - Page 1, style AAA
> - Page 2, style AAA
> * page break, requesting style BBB
> - Page 3, style BBB
>
> But the layout did recycle the page 2, leading to :
> - Page 1, style AAA
> - Page 2, style BBB
> * page break, requesting style BBB
> - Page 3, style BBB
>
> Reloading the document leads to a correct layout.
>
>
> Diffs
> -----
>
> libs/textlayout/KoTextDocumentLayout.cpp 757bc338fcb8b5978fad19fe3da48eef06ac3a3d
> libs/textlayout/KoTextLayoutRootAreaProvider.h \
> 23b91bc519761eef2861fc6fc4546294271389cc \
> libs/textlayout/tests/MockRootAreaProvider.h \
> 23056384fb55918ae360ad062d5dfd13ee7c4837 \
> libs/textlayout/tests/MockRootAreaProvider.cpp \
> 0402528998c26cfb586f297abe7c879459adda50 \
> plugins/textshape/SimpleRootAreaProvider.h afada40af1ea3341a820dc0d275a2db5bbb5a3f8 \
> plugins/textshape/SimpleRootAreaProvider.cpp \
> 42df2ecd30aa744780642618556a7013a9ff27da words/part/KWDocument.cpp \
> c1a80522c5573f0db920b72dd73e88650b5ace07 words/part/KWRootAreaProvider.h \
> 8fb9b5e379a0561fceb7ba32935084df46ce6fd9 words/part/KWRootAreaProvider.cpp \
> ce039219c1a044b13ae81420832eb894346b88db
> Diff: https://git.reviewboard.kde.org/r/120733/diff/
>
>
> Testing
> -------
>
> A simple document with page styles (changing the background color to help speed up \
> the development) now doesn't exhibit the wrong layout behaviour.
>
> Thanks,
>
> Pierre Ducroquet
>
>
--===============2509021920089941830==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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/120733/">https://git.reviewboard.kde.org/r/120733/</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 27th, 2014, 9:14 e.m. UTC, <b>Camilla \
Boemann</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/120733/diff/7-8/?file=330774#file330774line858" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/textlayout/KoTextDocumentLayout.cpp</a> <span style="font-weight: \
normal;">
(Diff revisions 7 - 8)
</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; ">bool \
KoTextDocumentLayout::doLayout()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">858</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">855</font></th> <td bgcolor="#ffffff" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">while</span> <span class="p">(</span><span \
class="n">transferedFootNoteCursor</span> <span class="o">||</span> <span \
class="n">d</span><span class="o">-></span><span \
class="n">layoutPosition</span><span class="o">-></span><span class="n">it</span> \
<span class="o">!=</span> <span class="n">document</span><span \
class="p">()</span><span class="o">-></span><span class="n">rootFrame</span><span \
class="p">()</span><span class="o">-></span><span class="n">end</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;">but \
doing nothing and not incrmenting ??</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;">either this objection is too complex for me to understand, or you are \
misreading :) this is the <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">while</code> \
part of the big <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">do-while</code> loop, which \
does tons of stuff, right? (the <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">while</code> \
should probably be moved up to the same line as the closing <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">}</code>)</p>< /pre>
<br />
<p>- Elvis</p>
<br />
<p>On november 27th, 2014, 9:05 e.m. UTC, Pierre Ducroquet 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 Calligra and Camilla Boemann.</div>
<div>By Pierre Ducroquet.</div>
<p style="color: grey;"><i>Updated nov 27, 2014, 9:05 e.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</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 refactors a bit the \
KoTextLayoutRootAreaProvider. It extends the provideNext call by adding a constraints \
structure. This structure contains so far the masterPageName and the page
number of the requested new rootArea.
This way, the provider don't have to sneak in layout specific data.
It also adds a way for KoTextLayoutRootAreaProvider to help the
layout and fix inserting a new page (by adding lines to a paragraph
for instance) before a page break changing the page style.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Some more explanations regarding this \
bug :
- Page 1, style AAA
* page break, requesting style BBB
- Page 2, style BBB</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">If you add more data to the page 1, then dynamically \
the layout should do :
- Page 1, style AAA
- Page 2, style AAA
* page break, requesting style BBB
- Page 3, style BBB</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">But the layout did recycle the page 2, \
leading to :
- Page 1, style AAA
- Page 2, style BBB
* page break, requesting style BBB
- Page 3, style BBB</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Reloading the document leads to a correct \
layout.</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;">A simple document with page styles (changing the \
background color to help speed up the development) now doesn't exhibit the wrong \
layout behaviour.</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>libs/textlayout/KoTextDocumentLayout.cpp <span style="color: \
grey">(757bc338fcb8b5978fad19fe3da48eef06ac3a3d)</span></li>
<li>libs/textlayout/KoTextLayoutRootAreaProvider.h <span style="color: \
grey">(23b91bc519761eef2861fc6fc4546294271389cc)</span></li>
<li>libs/textlayout/tests/MockRootAreaProvider.h <span style="color: \
grey">(23056384fb55918ae360ad062d5dfd13ee7c4837)</span></li>
<li>libs/textlayout/tests/MockRootAreaProvider.cpp <span style="color: \
grey">(0402528998c26cfb586f297abe7c879459adda50)</span></li>
<li>plugins/textshape/SimpleRootAreaProvider.h <span style="color: \
grey">(afada40af1ea3341a820dc0d275a2db5bbb5a3f8)</span></li>
<li>plugins/textshape/SimpleRootAreaProvider.cpp <span style="color: \
grey">(42df2ecd30aa744780642618556a7013a9ff27da)</span></li>
<li>words/part/KWDocument.cpp <span style="color: \
grey">(c1a80522c5573f0db920b72dd73e88650b5ace07)</span></li>
<li>words/part/KWRootAreaProvider.h <span style="color: \
grey">(8fb9b5e379a0561fceb7ba32935084df46ce6fd9)</span></li>
<li>words/part/KWRootAreaProvider.cpp <span style="color: \
grey">(ce039219c1a044b13ae81420832eb894346b88db)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120733/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============2509021920089941830==--
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic