[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Review Request: Kword drawing order fix
From: "Thomas Zander" <zander () kde ! org>
Date: 2010-06-20 21:24:12
Message-ID: 20100620212412.27269.6426 () localhost
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4173/#review6201
-----------------------------------------------------------
> > 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 ap=
plications 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. The=
re just is a zindex as defined and used by all of koffice and flake using a=
pps.
=
> I like the KoShape::compareShapeZIndex solution, but if it is necessary f=
or
> accepting this patch to implement your solution I will do it. Should I?
The solution of changing koshape means making the compareShapeZIndex be mor=
e 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 compo=
nents 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.
- Thomas
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 />
<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> <br />
<p>- Thomas</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