Review Request: support rendering of separators of text columns

Mon Aug 13 23:50:19 BST 2012

> On Aug. 11, 2012, 12:41 a.m., C. Boemann wrote:
> > I like the idea of painting it in the bg shape. I see no reason why it should be only a temporary solution.
> > 
> > The size without margin, scares me (mostly with respect to mso files - i know writer is way too simple, and i read your mail to the odf tc :) )
> > 
> > We need to run a full cs tester session on this.
> > 
> > the redundant setting of background you remove. I'm not sure it's not needed, though i can't say where it wouldf be. Scares me though.
> 
> Friedrich W. H. Kossebau wrote:
>     Sorry for scaring you so much :)
>     
>     The reason why I think this to be just a temporary solution is that the background shape covers a large area, so will be triggered for repaint quite often, while the actual area which needs painting for the separators are just the small lines. So in an optimal solution the objects which care for the separators are just painting the lines if they really needs to.
>     
>     WRT the size without margin, I would do this change for compatibility. There are so many files out there which have been created by LO/OOo/AAO with the background not covering the margin by what was seen on screen/print-out, which make this kind-of official spec interpretation.
>     What do you mean with MSO files? Import? Or MSO-ODT variants? To have the background cover the whole page, the margin would need to be set to 0, and instead the padding set to get a proper spacing.
>     
>     Who could do us the favour and run a full cs tester session on this?
>     
>     WRT the removal of redundant background setting, I do not have a complete picture of the layouting data flow, I just saw it to be redundant where I looked at. And from testing could not yet see a problem. But I would not put my hand into the fire for this change :) so I do not mind if it stays as is. It would just be asymmetric to the setting of the separator data to the background shape. Perhaps just add a TODO for the future when I/someone will implement the config tool/dialog for separators?
> 
> Thorsten Zachmann wrote:
>     I have run cstester on this and have not found any changes.
> 
> Friedrich W. H. Kossebau wrote:
>     Good. Thanks for checking, Thorsten.
>     
>
> 
> C. Boemann wrote:
>     i don't think a big shape will cause more updates than a smaller shape - how do you figure that
>     
>     For removal of redundant setting yes add some kind of comment

I have no idea how repainting of dirty areas is done in the flake system, I just guessed that a big shape will be more often part of a dirty area then a small shape, and the painting routines of shapes I have seen do always paint the whole shape, relying on the QPainter and clipping to skip the unneeded paint instructions. E.g. when editing a text shape with columns with the current workaround solution in Calligra the background shape will be redrawn every time a new char is entered, because it is behind the text shape, even if it just exists for painting the column separators, which are not behind the text. Unless there is a clever system in the flake system which snapshots the shapes behind the active shape into a single image and just redraws that?

Any README or other pointers where I could learn more about that?

Concerning the patch, will add a comment in place of the redundant background setting and commit, thanks.

- Friedrich W. H.

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

On Aug. 10, 2012, 11:52 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105975/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2012, 11:52 p.m.)
> 
> 
> Review request for Calligra and C. Boemann.
> 
> 
> Description
> -------
> 
> I had the idea to render the column separators using the background shapes. What do you think? Would that be alright at least as a temporary solution?
> 
> Patch also fixes page background ignoring the margins. At least LO/AOO does not paint the background in the margin. Sadly the attribute "draw:background-size" is not available for <style:page-layout-properties>, only for <style:drawing-page-properties>. Filed a complain on the office-comment mailinglist.
>     
> And patch removes redundant setting of background to frame shapes in KWTextFrameSet::setPageStyle(...).
> 
> Okay to backport to 2.5?
> 
> 
> Diffs
> -----
> 
>   words/part/frames/KWFrameLayout.cpp d9675b0 
>   words/part/frames/KWTextFrameSet.cpp 7afa7bf 
> 
> Diff: http://git.reviewboard.kde.org/r/105975/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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