<br><br><div class="gmail_quote">On Thu, Jan 27, 2011 at 10:51 AM, C. Boemann <span dir="ltr"><<a href="mailto:cbo@boemann.dk">cbo@boemann.dk</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Well this is a step on the road and not the end result. So bear with me.<br>
<br>
But I've indeed changed my mind about who does the layout and drawing, and<br>
have come up with an abstraction that will allow the kotext library to do just<br>
that.</blockquote><div><br>So you mean, drawing/layouting would be done at kotext level?<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Well totally controlled by the textshape. This abstraction will allow<br>
any application to gain layout and painting.</blockquote><div><br>This is already the case. kotext provides an abstracted interface for layouting. the user of the lib needs to implement his layouting logic. What layouting logic/paradigm would you favor for kotext? this would also mean that any other layouting paradigm/logic, would still need to carry the weight (memory footprint,...) of a layouting logic he does not even care about?<br>
Sounds like a step in the opposite direction of the goal of deploying/using on a multitude of devices, for a multitude of use cases.<br>A bit like MS having internet explorer right tied up with their kernel. It becomes awefully difficult to have a very lean kernel based on windows.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">First app that will benefit from<br>
this is our very own Tables.<br>
<br>
But shapes and pages will soon be abstracted away as i said. Oh and no one<br>
requires those pagesizes and numbers to be filled out by apps that don't use<br>
it. For those apps the anchoring mode that relate to these things will simply<br>
not be activated.<br></blockquote><div> </div><div>Yes, no one has to fill these, but you are bloating a general purpose library with specific cases peculiarities. You'll end up with a melting pot library of things which do not relate to each other. You mention Tables, so why not also add the Spreadsheet/Cell coordinates to the mix? And since kotext could also be used in a painting application, the layer to which the anchored shape belongs. then, one could use kotext for a music notation stuff, so we could add score/voice/bar information.<br>
<br>I am exaggerating this a lot, but once the box is opened, it is very difficult to draw a line.<br><br><br>Pierre<br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5"><br>
On Thursday 27 January 2011 10:25:08 Pierre Stirnweiss wrote:<br>
> Sorry to arrive after the battle, but I still would like to give an opinion<br>
> on this one.<br>
> I don't think this change is going in the right direction. It mixes<br>
> responsibilities between kotext lib/textshape/application even more.<br>
><br>
> I think we should have a clear separation of responsibilities:<br>
> kotext lib: handles character/string level<br>
> textshapes: handles lines inside a shape, given constraints of that shape<br>
> application: handles shapes<br>
><br>
> I think it is bad idea to put things like page size, page number,... inside<br>
> kotext. This lib is supposed to be used by applications which are not<br>
> necessarily page driven.<br>
> Equally, I don't think it is the responsibility of every text shape to have<br>
> a track record of the other shapes. Children shapes eventually, but<br>
> definitely not shapes that are unrelated. This should be at application<br>
> level.<br>
><br>
> Now you have even more diffusion of stuff. If this is the direction you<br>
> want to give to the text stuff, why are we still having a textshape and<br>
> kotext lib?<br>
><br>
><br>
> Pierre<br>
><br>
> On Thu, Jan 27, 2011 at 6:13 AM, Thorsten Zachmann<br>
<<a href="mailto:t.zachmann@zagge.de">t.zachmann@zagge.de</a>>wrote:<br>
> > This is an automatically generated e-mail. To reply, visit:<br>
> > <a href="http://git.reviewboard.kde.org/r/100442/" target="_blank">http://git.reviewboard.kde.org/r/100442/</a><br>
> ><br>
> > I just added some small comments mostly about the style. Would be nice if<br>
> > you could fix those before committing.<br>
> ><br>
</div></div>> > plugins/textshape/Layout.h<<a href="http://git.reviewboard.kde.org/r/100442/dif" target="_blank">http://git.reviewboard.kde.org/r/100442/dif</a><br>
> > f/1/?file=7551#file7551line56> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > class ToCGenerator;<br>
> ><br>
> > 56<br>
> ><br>
> > ~Layout();<br>
> ><br>
> > the destructor should be marked virtual<br>
> ><br>
</div>> > plugins/textshape/TextAnchorStrategy.h<<a href="http://git.reviewboard.kde.org/" target="_blank">http://git.reviewboard.kde.org/</a><br>
> > r/100442/diff/1/?file=7553#file7553line28> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > class KoTextShapeData;<br>
> ><br>
> > 28<br>
> ><br>
> > class TextAnchorStrategy : public KoAnchorStrategy {<br>
> ><br>
> > the opening { should be moved to the next line<br>
> ><br>
</div>> > plugins/textshape/TextAnchorStrategy.cpp<<a href="http://git.reviewboard.kde.or" target="_blank">http://git.reviewboard.kde.or</a><br>
> > g/r/100442/diff/1/?file=7554#file7554line335> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > bool TextAnchorStrategy::countVerticalRel(QRectF &anchorBoundingRect,<br>
> > QRectF containerBoundingRect,<br>
> ><br>
> > 335<br>
> ><br>
> > switch (m_anchor->verticalRel()) {<br>
> ><br>
> > the indention of the switch and case is wrong here<br>
> ><br>
</div>> > words/part/frames/KWTextDocumentLayout.cpp<<a href="http://git.reviewboard.kde" target="_blank">http://git.reviewboard.kde</a>.<br>
> > org/r/100442/diff/1/?file=7559#file7559line302> (Diff<br>
<div><div></div><div class="h5">> ><br>
> > revision 1)<br>
> ><br>
> > private:<br>
> > 286<br>
> ><br>
> > // if part of page is already layouted than check if<br>
> > there are some anchored shapes and register them<br>
> ><br>
> > 259<br>
> ><br>
> > // QRectF bounds = m_state->shape->boundingRect();<br>
> ><br>
> > is it worth to keep the commented out code. If so please add a comment<br>
> > on why it is commented out, if not please delete it. That will make it<br>
> > easier later to figure out why the code is commented out<br>
> ><br>
> > - Thorsten<br>
> ><br>
> > On January 25th, 2011, 2:04 p.m., Matus Hanzes wrote:<br>
> > Review request for Calligra and Casper Boemann.<br>
> ><br>
> > By Matus Hanzes.<br>
> ><br>
> > *Updated Jan. 25, 2011, 2:04 p.m.*<br>
> > Description<br>
> ><br>
> > This patch moves KWAnchorStrategy into text shape.<br>
> ><br>
> > The reason is that it is not possible to do advanced shape anchor logic<br>
> > outside Layout.cpp.<br>
> ><br>
> > The main idea is to register the shapes into Layout.cpp and layout<br>
> > handles all the necessary things.<br>
> ><br>
> > The registration is done in KWTextDocumentLayout::positionInlineObject<br>
> > where all the words dependent data are set.<br>
> > (pageRectangle,pageContentRectangle,pageNumber)<br>
> ><br>
> > If the document or anchored shape changes<br>
> > KoTextDocumentLayout::resetInlineObject is called which resets all the<br>
> > shapes that are not valid and layout finds the right place for them.<br>
> ><br>
> > Any comments are welcome<br>
> ><br>
> > Diffs<br>
> ><br>
> > - libs/flake/KoShape.h (f7179d7)<br>
> > - libs/flake/KoShape.cpp (c5aee86)<br>
> > - libs/kotext/KoTextAnchor.h (2bbbf9a)<br>
> > - libs/kotext/KoTextAnchor.cpp (ece23d6)<br>
> > - libs/kotext/KoTextDocumentLayout.h (4284d37)<br>
> > - libs/kotext/KoTextDocumentLayout.cpp (6b66e0f)<br>
> > - libs/kotext/KoTextShapeContainerModel.h (ce3a6ae)<br>
> > - libs/kotext/KoTextShapeContainerModel.cpp (00ca9b5)<br>
> > - plugins/textshape/CMakeLists.txt (a23ecc3)<br>
> > - plugins/textshape/Layout.h (5e42b7a)<br>
> > - plugins/textshape/Layout.cpp (e1228e4)<br>
> > - plugins/textshape/TextAnchorStrategy.h (PRE-CREATION)<br>
> > - plugins/textshape/TextAnchorStrategy.cpp (PRE-CREATION)<br>
> > - words/part/CMakeLists.txt (2d5c667)<br>
> > - words/part/frames/KWAnchorStrategy.h (b39f377)<br>
> > - words/part/frames/KWAnchorStrategy.cpp (c168962)<br>
> > - words/part/frames/KWTextDocumentLayout.h (59add4f)<br>
> > - words/part/frames/KWTextDocumentLayout.cpp (15a8803)<br>
> ><br>
</div></div>> > View Diff <<a href="http://git.reviewboard.kde.org/r/100442/diff/" target="_blank">http://git.reviewboard.kde.org/r/100442/diff/</a>><br>
<div><div></div><div class="h5">> ><br>
> > _______________________________________________<br>
> > calligra-devel mailing list<br>
> > <a href="mailto:calligra-devel@kde.org">calligra-devel@kde.org</a><br>
> > <a href="https://mail.kde.org/mailman/listinfo/calligra-devel" target="_blank">https://mail.kde.org/mailman/listinfo/calligra-devel</a><br>
_______________________________________________<br>
calligra-devel mailing list<br>
<a href="mailto:calligra-devel@kde.org">calligra-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/calligra-devel" target="_blank">https://mail.kde.org/mailman/listinfo/calligra-devel</a><br>
</div></div></blockquote></div><br>