Review Request: Moving anchor strategy into text shape

[C. Boemann]

Well this is a step on the road and not the end result. So bear with me.

But I've indeed changed my mind about who does the layout and drawing, and 
have come up with an abstraction that will allow the kotext library to do just 
that. Well totally controlled by the textshape. This abstraction will allow 
any application to gain layout and painting. First app that will benefit from 
this is our very own Tables.

But shapes and pages will soon be abstracted away as i said. Oh and no one 
requires those pagesizes and numbers to be filled out by apps that don't use 
it. For those apps the anchoring mode that relate to these things will simply 
not be activated.

On Thursday 27 January 2011 10:25:08 Pierre Stirnweiss wrote:
> Sorry to arrive after the battle, but I still would like to give an opinion
> on this one.
> I don't think this change is going in the right direction. It mixes
> responsibilities between kotext lib/textshape/application even more.
> 
> I think we should have a clear separation of responsibilities:
> kotext lib: handles character/string level
> textshapes: handles lines inside a shape, given constraints of that shape
> application: handles shapes
> 
> I think it is bad idea to put things like page size, page number,... inside
> kotext. This lib is supposed to be used by applications which are not
> necessarily page driven.
> Equally, I don't think it is the responsibility of every text shape to have
> a track record of the other shapes. Children shapes eventually, but
> definitely not shapes that are unrelated. This should be at application
> level.
> 
> Now you have even more diffusion of stuff. If this is the direction you
> want to give to the text stuff, why are we still having a textshape and
> kotext lib?
> 
> 
> Pierre
> 
> On Thu, Jan 27, 2011 at 6:13 AM, Thorsten Zachmann 
<t.zachmann at zagge.de>wrote:
> >    This is an automatically generated e-mail. To reply, visit:
> > http://git.reviewboard.kde.org/r/100442/
> > 
> > I just added some small comments mostly about the style. Would be nice if
> > you could fix those before committing.
> > 
> >    plugins/textshape/Layout.h<http://git.reviewboard.kde.org/r/100442/dif
> >    f/1/?file=7551#file7551line56> (Diff
> > 
> > revision 1)
> > 
> > class ToCGenerator;
> > 
> >    56
> >    
> >     ~Layout();
> >   
> >   the destructor should be marked virtual
> >   
> >    plugins/textshape/TextAnchorStrategy.h<http://git.reviewboard.kde.org/
> >    r/100442/diff/1/?file=7553#file7553line28> (Diff
> > 
> > revision 1)
> > 
> > class KoTextShapeData;
> > 
> >    28
> > 
> > class TextAnchorStrategy  : public KoAnchorStrategy {
> > 
> >   the opening { should be moved to the next line
> >   
> >    plugins/textshape/TextAnchorStrategy.cpp<http://git.reviewboard.kde.or
> >    g/r/100442/diff/1/?file=7554#file7554line335> (Diff
> > 
> > revision 1)
> > 
> > bool TextAnchorStrategy::countVerticalRel(QRectF &anchorBoundingRect,
> > QRectF containerBoundingRect,
> > 
> >    335
> >    
> >     switch (m_anchor->verticalRel()) {
> >   
> >   the indention of the switch and case is wrong here
> >   
> >    words/part/frames/KWTextDocumentLayout.cpp<http://git.reviewboard.kde.
> >    org/r/100442/diff/1/?file=7559#file7559line302> (Diff
> > 
> > revision 1)
> > 
> > private:
> >   286
> >   
> >                 // if part of page is already layouted than check if
> >                 there are some anchored shapes and register them
> > 
> > 259
> > 
> > //                QRectF bounds = m_state->shape->boundingRect();
> > 
> >   is it worth to keep the commented out code. If so please add a comment
> >   on why it is commented out, if not please delete it. That will make it
> >   easier later to figure out why the code is commented out
> > 
> > - Thorsten
> > 
> > On January 25th, 2011, 2:04 p.m., Matus Hanzes wrote:
> >   Review request for Calligra and Casper Boemann.
> > 
> > By Matus Hanzes.
> > 
> > *Updated Jan. 25, 2011, 2:04 p.m.*
> > Description
> > 
> > This patch moves KWAnchorStrategy into text shape.
> > 
> > The reason is that it is not possible to do advanced shape anchor logic
> > outside Layout.cpp.
> > 
> > The main idea is to register the shapes into Layout.cpp and layout
> > handles all the necessary things.
> > 
> > The registration is done in KWTextDocumentLayout::positionInlineObject
> > where all the words dependent data are set.
> > (pageRectangle,pageContentRectangle,pageNumber)
> > 
> > If the document or anchored shape changes
> > KoTextDocumentLayout::resetInlineObject is called which resets all the
> > shapes that are not valid and layout finds the right place for them.
> > 
> > Any comments are welcome
> > 
> >   Diffs
> >   
> >    - libs/flake/KoShape.h (f7179d7)
> >    - libs/flake/KoShape.cpp (c5aee86)
> >    - libs/kotext/KoTextAnchor.h (2bbbf9a)
> >    - libs/kotext/KoTextAnchor.cpp (ece23d6)
> >    - libs/kotext/KoTextDocumentLayout.h (4284d37)
> >    - libs/kotext/KoTextDocumentLayout.cpp (6b66e0f)
> >    - libs/kotext/KoTextShapeContainerModel.h (ce3a6ae)
> >    - libs/kotext/KoTextShapeContainerModel.cpp (00ca9b5)
> >    - plugins/textshape/CMakeLists.txt (a23ecc3)
> >    - plugins/textshape/Layout.h (5e42b7a)
> >    - plugins/textshape/Layout.cpp (e1228e4)
> >    - plugins/textshape/TextAnchorStrategy.h (PRE-CREATION)
> >    - plugins/textshape/TextAnchorStrategy.cpp (PRE-CREATION)
> >    - words/part/CMakeLists.txt (2d5c667)
> >    - words/part/frames/KWAnchorStrategy.h (b39f377)
> >    - words/part/frames/KWAnchorStrategy.cpp (c168962)
> >    - words/part/frames/KWTextDocumentLayout.h (59add4f)
> >    - words/part/frames/KWTextDocumentLayout.cpp (15a8803)
> > 
> > View Diff <http://git.reviewboard.kde.org/r/100442/diff/>
> > 
> > _______________________________________________
> > calligra-devel mailing list
> > calligra-devel at kde.org
> > https://mail.kde.org/mailman/listinfo/calligra-devel