[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: bug fix "Text displayed at wrong place in this
From:       "Matus Hanzes" <matus.hanzes () ixonos ! com>
Date:       2010-05-14 8:09:51
Message-ID: 20100514080951.15247.3002 () localhost
[Download RAW message or body]



> On 2010-05-11 14:43:05, Thomas Zander wrote:
> > On Tuesday 11. May 2010 15.09.22 you wrote:
> > > With the old approach there are 50 vertical enums needed and 91 horizontal
> > > enums.
> > 
> > Thats a rather staggering amount. Could you give some idea where this comes from? \
> > A couple of examples. 
> > > My proposal to move the layout recalculation code from KoAnchorStrategy to
> > > KWTextDocumentLayout, is because I think there is no enough information in
> > > KoAnchorStrategy to avoid unnecessary layout recalculations.  For example
> > > when the anchored shape is in background there is no need for layout
> > > recalculation.
> > 
> > While layout speed is important (though not nearly as important as painting \
> > speed) the maintainability of the code is much more important still. Did you \
> > write a lot of unit tests for this new design? 
> > > The reason for it is that I need to support odf standard because of my bug.
> > Fixing your not-publicly visible bug is a really unconvincing reason, to be \
> > honest... 

> Thats a rather staggering amount. Could you give some idea where this comes from? A \
> couple of examples.

New design :

enum HorizontalPos {
    HCenter,
    HFromInside,
    HFromLeft,
    HInside,
    HLeft,
    HOutside,
    HRight
};

enum HorizontalRel {
    HChar,
    HPage,
    HPageContent,
    HPageStartMargin,
    HPageEndMargin,
    HFrame,
    HFrameContent,
    HFrameEndMargin,
    hFrameStartMargin,
    HParagraph,
    HParagraphContent,
    HParagraphEndMargin,
    hParagraphStartMargin
};

enum VerticalPos {
    VBelow,
    VBottom,
    VFromTop,
    VMiddle,
    VTop
};

enum VerticalRel {
    VBaseline,
    VChar,
    VFrame,
    VFrameContent,
    VLine,
    VPage,
    VPageContent,
    VParagraph,
    VParagraphContent,
    VText
};

Old design :

enum AnchorVertical {
    BaselineBelow
    BaselineBottom
    BaselineFromTop
    BaselineMiddle
    BaselineTop
    CharBelow
    CharBottom
    CharFromTop
    CharMiddle
    CharTop
    FrameBelow
    FrameBottom
    FrameFromTop
    FrameMiddle
    FrameTop
    FrameContentBelow
    FrameContentBottom
    FrameContentFromTop
    FrameContentMiddle
    FrameContentTop
    LineBelow
    LineBottom
    LineFromTop
    LineMiddle
    LineTop
    PageBelow
    PageBottom
    PageFromTop
    PageMiddle
    PageTop
    PageContentBelow
    PageContentBottom
    PageContentFromTop
    PageContentMiddle
    PageContentTop
    ParagraphBelow
    ParagraphBottom
    ParagraphFromTop
    ParagraphMiddle
    ParagraphTop
    ParagraphContentBelow
    ParagraphContentBottom
    ParagraphContentFromTop
    ParagraphContentMiddle
    ParagraphContentTop
    TextBelow
    TextBottom
    TextFromTop
    TextMiddle
    TextTop
}

enum AnchorHorizontal {
    CharCenter
    CharFromInside
    CharFromLeft
    CharInside
    CharLeft
    CharOutside
    CharRight
    PageCenter
    PageFromInside
    PageFromLeft
    PageInside
    PageLeft
    PageOutside
    PageRight
    PageContentCenter
    PageContentFromInside
    PageContentFromLeft
    PageContentInside
    PageContentLeft
    PageContentOutside
    PageContentRight
    PageStartMarginCenter
    PageStartMarginFromInside
    PageStartMarginFromLeft
    PageStartMarginInside
    PageStartMarginLeft
    PageStartMarginOutside
    PageStartMarginRight
    PageEndMarginCenter
    PageEndMarginFromInside
    PageEndMarginFromLeft
    PageEndMarginInside
    PageEndMarginLeft
    PageEndMarginOutside
    PageEndMarginRight
    FrameCenter
    FrameFromInside
    FrameFromLeft
    FrameInside
    FrameLeft
    FrameOutside
    FrameRight
    FrameContentCenter
    FrameContentFromInside
    FrameContentFromLeft
    FrameContentInside
    FrameContentLeft
    FrameContentOutside
    FrameContentRight
    FrameEndMarginCenter
    FrameEndMarginFromInside
    FrameEndMarginFromLeft
    FrameEndMarginInside
    FrameEndMarginLeft
    FrameEndMarginOutside
    FrameEndMarginRight
    FrameStartMarginCenter
    FrameStartMarginFromInside
    FrameStartMarginFromLeft
    FrameStartMarginInside
    FrameStartMarginLeft
    FrameStartMarginOutside
    FrameStartMarginRight
    ParagraphCenter
    ParagraphFromInside
    ParagraphFromLeft
    ParagraphInside
    ParagraphLeft
    ParagraphOutside
    ParagraphRight
    ParagraphContentCenter
    ParagraphContentFromInside
    ParagraphContentFromLeft
    ParagraphContentInside
    ParagraphContentLeft
    ParagraphContentOutside
    ParagraphContentRight
    ParagraphEndMarginCenter
    ParagraphEndMarginFromInside
    ParagraphEndMarginFromLeft
    ParagraphEndMarginInside
    ParagraphEndMarginLeft
    ParagraphEndMarginOutside
    ParagraphEndMarginRight
    ParagraphStartMarginCenter
    ParagraphStartMarginFromInside
    ParagraphStartMarginFromLeft
    ParagraphStartMarginInside
    ParagraphStartMarginLeft
    ParagraphStartMarginOutside
    ParagraphStartMarginRight
}

> While layout speed is important (though not nearly as important as painting speed) \
> the maintainability of the code is much more important still. Did you write a lot \
> of unit tests >for this new design?

I will write them after design approval.


- Matus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3699/#review5601
-----------------------------------------------------------


On 2010-05-07 15:20:10, Matus Hanzes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3699/
> -----------------------------------------------------------
> 
> (Updated 2010-05-07 15:20:10)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> To place draw objects properly in kword document I needed to add more support for \
> anchoring in kword.(style:vertical-pos,style:vertical-rel,style:horizontal-pos,style:vertical-rel)
>  I tried to make anchoring backward compatible.
> 
> Comments are welcome.
> 
> 
> Diffs
> -----
> 
> trunk/koffice/kword/part/KWPageStyle.h 1123976 
> trunk/koffice/kword/part/KWPageStyle.cpp 1123976 
> trunk/koffice/kword/part/KWPageStyle_p.h 1123976 
> trunk/koffice/kword/part/KWPageTextInfo.h 1123976 
> trunk/koffice/kword/part/KWPageTextInfo.cpp 1123976 
> trunk/koffice/kword/part/frames/KWAnchorStrategy.h 1123976 
> trunk/koffice/kword/part/frames/KWAnchorStrategy.cpp 1123976 
> trunk/koffice/kword/part/frames/KWFrameLayout.cpp 1123976 
> trunk/koffice/kword/part/frames/KWTextDocumentLayout.cpp 1123976 
> trunk/koffice/libs/kotext/KoTextAnchor.h 1123976 
> trunk/koffice/libs/kotext/KoTextAnchor.cpp 1123976 
> trunk/koffice/libs/kotext/opendocument/KoTextLoader.cpp 1123976 
> 
> Diff: http://reviewboard.kde.org/r/3699/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matus
> 
> 

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]