[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Review Request: Allow painting text outside the shape
From: "Sebastian Sauer" <mail () dipe ! org>
Date: 2010-08-11 15:09:13
Message-ID: 20100811150913.21313.98839 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On 2010-08-03 12:07:37, Thomas Zander wrote:
> > I must say I was rather fond of the simplicity of a shape having a size and it \
> > its content is limited to that. This makes repainting faster, it makes things \
> > consistent with other frameworks and predictable/understandable. I think going \
> > away from that by introducing the possibility to ignore the actual size by having \
> > a second size is not something that I like very much.
> > Negative margins have never worked in KWord, it has in its 12 ½ yeras never given \
> > us a significant amount of bugreports and I feel its not a feature I value enough \
> > to break the flake design. I was under the impression I closed all bugreports for \
> > this on bugzilla as 'wontfix' if I missed one I apologize for causing you to work \
> > on this while I never intended KWord to go in that direction.
> > One solution may be that we make the auto-generated frames in KWord be full \
> > page-width and use the shape inset to represent the kword-page margins. But this \
> > solution feels like we are contorting flake for a misfeature that we can do \
> > without just fine.
> > As an extended rationale for why flake should not go in this direction; Flake is \
> > bigger than KWord or KOffice. It will be used by other applications that want an \
> > ODF rendering engine and it will be very hard to explain that the shape size is \
> > not enough for a canvas implementor to look at when determining how to position \
> > the (text) shape in 3rd party applications canvas'.
> > So, bottom line; please discard this review for the reasons given above.
>
> Suresh Chande wrote:
> Please re-consider your decision choice on this Thomas. We have some real world \
> documents we are facing issues related to this. This would be good to get Kword to \
> work please propose a way to address this issue. This is relevant for us to see \
> work in KOffice. How come you have not faced such documents in 12.5 years ? I \
> guess we are now heading towards end user readiness and you should consider real \
> world documents to mature KOffice and get this issue fixed. Lets find a solution to \
> this one.
> Thomas Zander wrote:
> Thanks for the post Suresh.
> I took another look and considered this a bit more and one thing stands out quite \
> clearly; the power of KOffice stands and falls with the ability of 3rd parties to \
> write plugins for it. Using flake. So the power and simplicity of Flake is super \
> important for the future of KOffice. The added complexity of the flake concepts \
> introduced for this feature is going to have a negative impact on that. So it has \
> to be really really valuable to end users.
> I'm a bit confused about your point of end-user-readyness. Which end user are you \
> thinking about? If we look at the KDE-wide personas (i.e. expected users) \
> http://wiki.koffice.org/index.php?title=KWord/EndUserReady/Personas this feature \
> would be used by Berna, and it makes no sense to focus on her while we have not yet \
> managed to make KOffice usable to the least demanding user; Susan. If we want any \
> significant number of users of KOffice, lets focus on getting it usable and ready \
> for Susan first. I expect that if usage takes up we can expect more developers \
> coming in too. So if you want that too, then please focus on features required for \
> Susan and not so much on features that are really not used by a significant number \
> of users.
> I also repeat the point that users that want to get the effect achieved by this \
> feature can get that result really easy without this feature being present in \
> KOffice.
> Thorsten Zachmann wrote:
> I think it is important to not that Suresh who speeks for Nokia are a 3th party who \
> wants to use flake. The way Boemann has implemented the feature works quite nicely \
> and the change to flake is rather minimal. I see no problem to include this code.
> It is possible in current kword to create a negative indent. However the result is \
> not shown correctly.
> If it is not possible to extend flake with the above this I don't see why any 3rd \
> parties would be interested in using flake at all.
> So I think the patch should be committed.
>
>
> Thomas Zander wrote:
> > If it is not possible to extend flake with the above this I don't see why any 3rd \
> > parties would be interested in using flake at all.
>
> Please explain what leads you to conclude this. I don't understand how you got to \
> that conclusion.
> Inge Wallin wrote:
> Einstein is reported to have said: "We should always simplify things as much as \
> possible, but no more than that". I think this is a case of simplifying too much. \
> The fact is that ODF supports negative indents, which means that we should too. It \
> was decided at the last community sprint that "ODF is the way to go, i.e. priority \
> 1." [1] It was also decided that "Missing [ODF] features are bugs". This means \
> that this patch should be committed unless there are any particular technical \
> objections.
> [1] http://wiki.koffice.org/index.php?title=Meetings/Mid_2010_meeting/Minutes#compatibility_.2F_interoperability_.28ODF_Support.29
>
> Suresh Chande wrote:
> I am not so sure of the personas and how we have created them and what sort of \
> evidence we have about those personas and the kind of documents they use Koffice \
> with. This bugs is coming from real documents studied inside Nokia and this is seen \
> as hinderance. This issue was also highlighted recently when we got some documents \
> from ODF commitee who asked us to test how well the Office engine works with some \
> specific documents. please find a solution for this.
> Thomas Zander wrote:
> Guys; I know and understand you have commercial interests to close your bugs and \
> for some reason you want this feature that is not actually used all that much in \
> the real world. I am not very happy with all the subcontractors coming with \
> non-technical arguments to somehow try to persuade me. Thats not how things get \
> decided in an open source project. Please keep strictly to technical arguments. As \
> an aside; I strongly suggest you work *with* the community and don't spent all the \
> good will on pushing through a change to Flake that will hurt KOffice (and thus you \
> as well) in the long run.
> This is my last reply on this topic; this should not be committed for the reasons I \
> stated in the first and second reply of mine.
> Sebastian Sauer wrote:
> There would be an easy solution to progress. Split the patch into 2 parts. One \
> containing the KWord related changes in kword/part/KWGui.cpp and \
> kword/part/tests/TestDocumentLayout.cpp and the others the remaining files. Then \
> the biggest part can be applied (Thomas is only maintaining KWord and not libs and \
> plugins) and we can try to work around the KWord-related part somehow...
Before it's understood wrong; with "just applied" I mean "if noone (else) provides \
*technical* reasons why not till then".
- Sebastian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4861/#review6768
-----------------------------------------------------------
On 2010-08-03 11:31:02, Casper Boemann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4861/
> -----------------------------------------------------------
>
> (Updated 2010-08-03 11:31:02)
>
>
> Review request for KOffice.
>
>
> Summary
> -------
>
> When you have negative indents you want to be able to paint text outside the frame \
> of the textshape.
> This patch accomplishes that.
>
> It also changes the way Rulers work regarding active ranges. First it now has the \
> notion of an override range which the rulermanager gets from resourcemanager. This \
> way the tools are now responsible for setting the (override) active range. The \
> default active range is still set by the apps, (and kword now also defaults to the \
> entire paper like kpresenter)
> As to the draw outside i've introduced an outlineRect() method similar to outline() \
> but just a rect rather than a path.
>
> Diffs
> -----
>
> trunk/koffice/kword/part/KWGui.cpp 1158585
> trunk/koffice/kword/part/tests/TestDocumentLayout.cpp 1158585
> trunk/koffice/libs/flake/KoResourceManager.h 1158585
> trunk/koffice/libs/flake/KoShape.h 1158585
> trunk/koffice/libs/flake/KoShape.cpp 1158585
> trunk/koffice/libs/kotext/KoTextDocumentLayout.h 1158585
> trunk/koffice/libs/kotext/KoTextDocumentLayout.cpp 1158585
> trunk/koffice/libs/main/KoRuler.h 1158585
> trunk/koffice/libs/main/KoRuler.cpp 1158585
> trunk/koffice/libs/main/KoRulerController.cpp 1158585
> trunk/koffice/libs/main/KoRuler_p.h 1158585
> trunk/koffice/plugins/textshape/Layout.h 1158585
> trunk/koffice/plugins/textshape/Layout.cpp 1158585
> trunk/koffice/plugins/textshape/TextShape.h 1158585
> trunk/koffice/plugins/textshape/TextShape.cpp 1158585
> trunk/koffice/plugins/textshape/TextTool.cpp 1158585
> trunk/koffice/plugins/textshape/tests/TestDocumentLayout.cpp 1158585
>
> Diff: http://reviewboard.kde.org/r/4861/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Casper
>
>
[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/4861/">http://reviewboard.kde.org/r/4861/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 3rd, 2010, 12:07 p.m., <b>Thomas \
Zander</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I must say I was rather fond of the simplicity of a shape having a size \
and it its content is limited to that. This makes repainting faster, it makes things \
consistent with other frameworks and predictable/understandable. I think going away \
from that by introducing the possibility to ignore the actual size by having a second \
size is not something that I like very much.
Negative margins have never worked in KWord, it has in its 12 ½ yeras never given us \
a significant amount of bugreports and I feel its not a feature I value enough to \
break the flake design. I was under the impression I closed all bugreports for this \
on bugzilla as 'wontfix' if I missed one I apologize for causing you to work \
on this while I never intended KWord to go in that direction.
One solution may be that we make the auto-generated frames in KWord be full \
page-width and use the shape inset to represent the kword-page margins. But this \
solution feels like we are contorting flake for a misfeature that we can do without \
just fine.
As an extended rationale for why flake should not go in this direction; Flake is \
bigger than KWord or KOffice. It will be used by other applications that want an ODF \
rendering engine and it will be very hard to explain that the shape size is not \
enough for a canvas implementor to look at when determining how to position the \
(text) shape in 3rd party applications canvas'.
So, bottom line; please discard this review for the reasons given above.</pre>
</blockquote>
<p>On August 10th, 2010, 6:50 p.m., <b>Suresh Chande</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please re-consider your \
decision choice on this Thomas. We have some real world documents we are facing \
issues related to this. This would be good to get Kword to work please propose a way \
to address this issue. This is relevant for us to see work in KOffice. How come you \
have not faced such documents in 12.5 years ? I guess we are now heading towards end \
user readiness and you should consider real world documents to mature KOffice and get \
this issue fixed. Lets find a solution to this one.</pre> </blockquote>
<p>On August 11th, 2010, 9:01 a.m., <b>Thomas Zander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for the post \
Suresh. I took another look and considered this a bit more and one thing stands out \
quite clearly; the power of KOffice stands and falls with the ability of 3rd parties \
to write plugins for it. Using flake. So the power and simplicity of Flake is super \
important for the future of KOffice. The added complexity of the flake concepts \
introduced for this feature is going to have a negative impact on that. So it has to \
be really really valuable to end users.
I'm a bit confused about your point of end-user-readyness. Which end user are you \
thinking about? If we look at the KDE-wide personas (i.e. expected users) \
http://wiki.koffice.org/index.php?title=KWord/EndUserReady/Personas this feature \
would be used by Berna, and it makes no sense to focus on her while we have not yet \
managed to make KOffice usable to the least demanding user; Susan. If we want any \
significant number of users of KOffice, lets focus on getting it usable and ready for \
Susan first. I expect that if usage takes up we can expect more developers coming in \
too. So if you want that too, then please focus on features required for Susan and \
not so much on features that are really not used by a significant number of users.
I also repeat the point that users that want to get the effect achieved by this \
feature can get that result really easy without this feature being present in \
KOffice.</pre> </blockquote>
<p>On August 11th, 2010, 9:41 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think it is important \
to not that Suresh who speeks for Nokia are a 3th party who wants to use flake. The \
way Boemann has implemented the feature works quite nicely and the change to flake is \
rather minimal. I see no problem to include this code.
It is possible in current kword to create a negative indent. However the result is \
not shown correctly.
If it is not possible to extend flake with the above this I don't see why any 3rd \
parties would be interested in using flake at all.
So I think the patch should be committed.
</pre>
</blockquote>
<p>On August 11th, 2010, 10:21 a.m., <b>Thomas Zander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> If it is not \
possible to extend flake with the above this I don't see why any 3rd parties \
would be interested in using flake at all.
Please explain what leads you to conclude this. I don't understand how you got to \
that conclusion.</pre> </blockquote>
<p>On August 11th, 2010, 11:37 a.m., <b>Inge Wallin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Einstein is reported to \
have said: "We should always simplify things as much as possible, but no more \
than that". I think this is a case of simplifying too much. The fact is that \
ODF supports negative indents, which means that we should too. It was decided at the \
last community sprint that "ODF is the way to go, i.e. priority 1." [1] It \
was also decided that "Missing [ODF] features are bugs". This means that \
this patch should be committed unless there are any particular technical objections.
[1] http://wiki.koffice.org/index.php?title=Meetings/Mid_2010_meeting/Minutes#compatibility_.2F_interoperability_.28ODF_Support.29</pre>
</blockquote>
<p>On August 11th, 2010, 11:49 a.m., <b>Suresh Chande</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I am not so sure of the \
personas and how we have created them and what sort of evidence we have about those \
personas and the kind of documents they use Koffice with. This bugs is coming from \
real documents studied inside Nokia and this is seen as hinderance. This issue was \
also highlighted recently when we got some documents from ODF commitee who asked us \
to test how well the Office engine works with some specific documents. please find a \
solution for this.</pre> </blockquote>
<p>On August 11th, 2010, 12:27 p.m., <b>Thomas Zander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Guys; I know and \
understand you have commercial interests to close your bugs and for some reason you \
want this feature that is not actually used all that much in the real world. I am \
not very happy with all the subcontractors coming with non-technical arguments to \
somehow try to persuade me. Thats not how things get decided in an open source \
project. Please keep strictly to technical arguments. As an aside; I strongly \
suggest you work *with* the community and don't spent all the good will on \
pushing through a change to Flake that will hurt KOffice (and thus you as well) in \
the long run.
This is my last reply on this topic; this should not be committed for the reasons I \
stated in the first and second reply of mine.</pre> </blockquote>
<p>On August 11th, 2010, 3 p.m., <b>Sebastian Sauer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">There would be an easy \
solution to progress. Split the patch into 2 parts. One containing the KWord related \
changes in kword/part/KWGui.cpp and kword/part/tests/TestDocumentLayout.cpp and the \
others the remaining files. Then the biggest part can be applied (Thomas is only \
maintaining KWord and not libs and plugins) and we can try to work around the \
KWord-related part somehow... </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Before it's \
understood wrong; with "just applied" I mean "if noone (else) provides \
*technical* reasons why not till then". </pre>
<br />
<p>- Sebastian</p>
<br />
<p>On August 3rd, 2010, 11:31 a.m., Casper Boemann 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 Casper Boemann.</div>
<p style="color: grey;"><i>Updated 2010-08-03 11:31:02</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; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">When you have negative indents you want to be able to paint text outside \
the frame of the textshape.
This patch accomplishes that.
It also changes the way Rulers work regarding active ranges. First it now has the \
notion of an override range which the rulermanager gets from resourcemanager. This \
way the tools are now responsible for setting the (override) active range. The \
default active range is still set by the apps, (and kword now also defaults to the \
entire paper like kpresenter)
As to the draw outside i've introduced an outlineRect() method similar to \
outline() but just a rect rather than a path.</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/KWGui.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/kword/part/tests/TestDocumentLayout.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/flake/KoResourceManager.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/flake/KoShape.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/flake/KoShape.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/kotext/KoTextDocumentLayout.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/kotext/KoTextDocumentLayout.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/main/KoRuler.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/main/KoRuler.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/main/KoRulerController.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/libs/main/KoRuler_p.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/Layout.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/Layout.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/TextShape.h <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/TextShape.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/TextTool.cpp <span style="color: \
grey">(1158585)</span></li>
<li>trunk/koffice/plugins/textshape/tests/TestDocumentLayout.cpp <span style="color: \
grey">(1158585)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4861/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