[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