[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Review Request: Kword drawing order fix
From: "Matus Hanzes" <matus.hanzes () ixonos ! com>
Date: 2010-06-21 9:20:15
Message-ID: 20100621092015.7954.21632 () localhost
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On 2010-06-20 21:24:20, Thomas Zander wrote:
> > > > This is something that can be done inside the text shape making the
> > > > changes be completely invisible to kword and kpresenter so only one
> > > > component has to be changed and all apps benefit.
> > > =
> > > There is no need for it in kpresenter because background is drawn the=
re in
> > > separate shape not in text shape.
> > =
> > I didn't say kpresenter requires this. I said the change is invisible t=
o applications by fixing it in the text plugin. Which is a clear architectu=
ral and maintenance win.
> > =
> > > So you suggest that the page background in kword should be draw in he=
ader
> > > text shape ?
> > =
> > I don't understand that question.
> > =
> > > > I'm thinking there must be a simpler way. What about if you define =
(in
> > > > KoText.h) the z index of the text layer (say, 10000) and all anchor=
ed
> > > > shapes either have a zindex in front or below based on their run
> > > > through style.
> > > > How would that work then? Sounds like it would be simpler, no?
> > > =
> > > We are still talking if it is better to have change in
> > > KoShape::compareShapeZIndex or to move text shape to the same layer t=
han
> > > anchored shapes, create artificial zIndex and maintain this artificial
> > > zIndex.
> > =
> > First, there is no artificial zindex in the solution I suggested above.=
There just is a zindex as defined and used by all of koffice and flake usi=
ng apps.
> > =
> > > I like the KoShape::compareShapeZIndex solution, but if it is necessa=
ry for
> > > accepting this patch to implement your solution I will do it. Should =
I?
> > =
> > The solution of changing koshape means making the compareShapeZIndex be=
more complicated for *all* usages. Thats like adding some code to QWidget =
for your application widget.
> > It may be nice, but its in the wrong place and affects many different c=
omponents that all use the zIndex.
> > =
> > As far as I can read your replies to my request I have not heard compel=
ling reasons why its a better solution. It looks like you choose it because=
of a communication issue. Which is certainly unfortunate.
> > So you suggest that the page background in kword should be draw in head=
er
> > text shape ?
> I don't understand that question.
The drawing order that I want to implement is
page background
header with anchored shapes
footer with anchored shapes
main text with anchored shapes
If we want to place page background drawing in text shape it is necessary t=
o place it into header text shape.
When we place it for example in main text shape, header and footer will be =
not visible.
> The solution of changing koshape means making the compareShapeZIndex be m=
ore complicated for *all* usages. Thats like adding some code to QWidget fo=
r your application widget.
> It may be nice, but its in the wrong place and affects many different com=
ponents that all use the zIndex.
It is possible to change default QWidget behavior we can do the same for ko=
shape. =
Lets discuss details on irc.
- Matus
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4173/#review6201
-----------------------------------------------------------
On 2010-06-07 12:31:47, Matus Hanzes wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4173/
> -----------------------------------------------------------
> =
> (Updated 2010-06-07 12:31:47)
> =
> =
> Review request for KOffice.
> =
> =
> Summary
> -------
> =
> KWOdfLoader
> Added support for loading of frame properties for objects anchored in hea=
der or footer, but I am not sure it is the right way to do it.
> =
> KOshape
> Added support for something like layers, so that shape placed in lower la=
yer are drawn earlyer than shape in higher layer.
> =
> KWFrame
> Have added support for placing shapes in layers.
> =
> RunThrough-Backgrond is placed in layer -1.
> RunThrough-Foreground is placed in layer 1.
> All others runAround properties are placed in layer 0.
> =
> =
> Diffs
> -----
> =
> trunk/koffice/kword/part/CMakeLists.txt 1135394 =
> trunk/koffice/kword/part/KWOdfLoader.cpp 1135394 =
> trunk/koffice/kword/part/KWOdfSharedLoadingData.cpp 1135394 =
> trunk/koffice/kword/part/KWPageBackground.h PRE-CREATION =
> trunk/koffice/kword/part/KWPageBackground.cpp PRE-CREATION =
> trunk/koffice/kword/part/KWord.h 1135394 =
> trunk/koffice/kword/part/KWord.cpp 1135394 =
> trunk/koffice/kword/part/frames/KWFrame.h 1135394 =
> trunk/koffice/kword/part/frames/KWFrame.cpp 1135394 =
> trunk/koffice/kword/part/frames/KWFrameLayout.h 1135394 =
> trunk/koffice/kword/part/frames/KWFrameLayout.cpp 1135394 =
> trunk/koffice/kword/part/frames/KWTextFrameSet.cpp 1135394 =
> trunk/koffice/libs/flake/KoShape.h 1135394 =
> trunk/koffice/libs/flake/KoShape.cpp 1135394 =
> trunk/koffice/libs/flake/KoShape_p.h 1135394 =
> =
> Diff: http://reviewboard.kde.org/r/4173/diff
> =
> =
> Testing
> -------
> =
> =
> Thanks,
> =
> Matus
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4173/">http://reviewboard.kde.org/r/4173/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On June 20th, 2010, 9:24 p.m., <b>Thomas \
Zander</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre>> > This is something that can be done \
inside the text shape making the > > changes be completely invisible to kword \
and kpresenter so only one > > component has to be changed and all apps \
benefit. >
> There is no need for it in kpresenter because background is drawn there in
> separate shape not in text shape.
I didn't say kpresenter requires this. I said the change is invisible to \
applications by fixing it in the text plugin. Which is a clear architectural and \
maintenance win.
> So you suggest that the page background in kword should be draw in header
> text shape ?
I don't understand that question.
> > I'm thinking there must be a simpler way. What about if you define (in
> > KoText.h) the z index of the text layer (say, 10000) and all anchored
> > shapes either have a zindex in front or below based on their run
> > through style.
> > How would that work then? Sounds like it would be simpler, no?
>
> We are still talking if it is better to have change in
> KoShape::compareShapeZIndex or to move text shape to the same layer than
> anchored shapes, create artificial zIndex and maintain this artificial
> zIndex.
First, there is no artificial zindex in the solution I suggested above. There just is \
a zindex as defined and used by all of koffice and flake using apps.
> I like the KoShape::compareShapeZIndex solution, but if it is necessary for
> accepting this patch to implement your solution I will do it. Should I?
The solution of changing koshape means making the compareShapeZIndex be more \
complicated for *all* usages. Thats like adding some code to QWidget for your \
application widget. It may be nice, but its in the wrong place and affects many \
different components that all use the zIndex.
As far as I can read your replies to my request I have not heard compelling reasons \
why its a better solution. It looks like you choose it because of a communication \
issue. Which is certainly unfortunate.</pre> </blockquote>
</blockquote>
<pre>> > So you suggest that the page background in kword should be draw in \
header > > text shape ?
> I don't understand that question.
The drawing order that I want to implement is
page background
header with anchored shapes
footer with anchored shapes
main text with anchored shapes
If we want to place page background drawing in text shape it is necessary to place it \
into header text shape. When we place it for example in main text shape, header and \
footer will be not visible.
> The solution of changing koshape means making the compareShapeZIndex be more \
complicated for *all* usages. Thats like adding some code to QWidget for your \
application widget. > It may be nice, but its in the wrong place and affects many \
different components that all use the zIndex.
It is possible to change default QWidget behavior we can do the same for koshape.
Lets discuss details on irc.
</pre>
<br />
<p>- Matus</p>
<br />
<p>On June 7th, 2010, 12:31 p.m., Matus Hanzes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for KOffice.</div>
<div>By Matus Hanzes.</div>
<p style="color: grey;"><i>Updated 2010-06-07 12:31:47</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0;">KWOdfLoader
Added support for loading of frame properties for objects anchored in header or \
footer, but I am not sure it is the right way to do it.
KOshape
Added support for something like layers, so that shape placed in lower layer are \
drawn earlyer than shape in higher layer.
KWFrame
Have added support for placing shapes in layers.
RunThrough-Backgrond is placed in layer -1.
RunThrough-Foreground is placed in layer 1.
All others runAround properties are placed in layer 0.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/koffice/kword/part/CMakeLists.txt <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/KWOdfLoader.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/KWOdfSharedLoadingData.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/KWPageBackground.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>trunk/koffice/kword/part/KWPageBackground.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>trunk/koffice/kword/part/KWord.h <span style="color: grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/KWord.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/frames/KWFrame.h <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/frames/KWFrame.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/frames/KWFrameLayout.h <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/frames/KWFrameLayout.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/kword/part/frames/KWTextFrameSet.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/libs/flake/KoShape.h <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/libs/flake/KoShape.cpp <span style="color: \
grey">(1135394)</span></li>
<li>trunk/koffice/libs/flake/KoShape_p.h <span style="color: \
grey">(1135394)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4173/diff/" style="margin-left: 3em;">View \
Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic