Review Request 120733: Pass more data between layout and RootAreaProvider

[Camilla Boemann]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120733/#review68957
-----------------------------------------------------------


I like the idea of the constraints, and how you fill it

but i am not so sure about the doLayout() testing and acting on the constraints. I feel this belong in the provider if at all possible

And encountering a different set of constraints shouldn't have to mean we release all root areas after. So releaseAllAfter should only be called when we are at the end of the layout process, otherwise the provider should just give us the page we need, and we should just keep on checking if we end where we ended before and if we get back in sync we shouldn't have to relayout everything (which releasAllAfter would mean we had to do and just think if we were editng on page 1 of a 800 page document)

i am not saying it's possible but i would surely like it better like that


libs/textlayout/KoTextDocumentLayout.cpp
<https://git.reviewboard.kde.org/r/120733/#comment48216>

    this part i don't like being in the library - i think it should rest upon the provider



words/part/KWRootAreaProvider.cpp
<https://git.reviewboard.kde.org/r/120733/#comment48217>

    i guess we should insert a page instead of appending (thus avoiding the need to release all after) if it mismatches the next available page in the current list of pages
    
    possibly this could be even smarter


- Camilla Boemann


On Oct. 23, 2014, 7:15 a.m., Pierre Ducroquet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120733/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 7:15 a.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 805f11d1c4aaf90a1a2a654053889e06800193de 
>   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/KWRootAreaProvider.h 8fb9b5e379a0561fceb7ba32935084df46ce6fd9 
>   words/part/KWRootAreaProvider.cpp 62b6fb6fba4dea8d9f68b0b3c1afb7324c50d502 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141023/c4933830/attachment.htm>