You are viewing a plain text version of this content. The canonical link for it is here.
Posted to batik-dev@xmlgraphics.apache.org by bu...@apache.org on 2009/06/12 09:41:54 UTC
DO NOT REPLY [Bug 47359] New: nested baseline-shift are not additive
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
Summary: nested baseline-shift are not additive
Product: Batik
Version: 1.7
Platform: All
OS/Version: All
Status: NEW
Severity: normal
Priority: P3
Component: Bridge
AssignedTo: batik-dev@xmlgraphics.apache.org
ReportedBy: nicolas.socheleau@quickoffice.com
Created an attachment (id=23803)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23803)
testcase for baseline-shift property
According to the spec
(http://www.w3.org/TR/SVG11/text.html#BaselineShiftProperty),
baseline-shift properties can nest and are additive but not inheritable.
the following example is expected to render text on the same line though the
tspan is back on the baseline:
<text baseline-shift="10pt">the quick <tspan fill="brown">brown</tspan>
fox</text>
the following example is expected to render the tspan within another tspan
higher up compared to the baseline:
<text> using <tspan baseline-shift="super">super <tspan
baseline-shift="5pt">super!</tspan> script</tspan></text>
Attached are testcases for the baseline-shift property
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #13 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 13:28:13 PST ---
>
> Apparently not those two items. ;-)
I hope I got it right this time :)
> > + for(int k = 0; k < values.size(); k++)
> > + {
> > + Object baselineValue = values.get(k);
>
> Could/should the "Object" variable be moved to outside the cycle? Although the
> nesting will probably never be more than one level...
I moved it outside of the loop
> > + //for the child of the element
>
> Seems that a superfluous (indenting) space slipped into that line. ;-)
>
you got me!
> > I've attached test file for animation and scripting as well for the review.
>
> Cool, I noticed the attachments but didn't have the chance to test them yet.
> Nevertheless, you didn't state whether the patch properly handled those
> situations (although you working suggests so).
>
the scripting and animation test files are working with the patch. Though, I'm
still working on some other animations where I don't get what I expect.
I'm starting to fix it is a different issue. I'll keep digging...
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #7 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 08:59:48 PST ---
Created an attachment (id=23811)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23811)
fix attempt 2
I updated the patch to take into account Helder Magalhães commments
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #11 from Helder Magalhães <he...@gmail.com> 2009-06-16 11:20:13 PST ---
(In reply to comment #8)
> > Missing space before "asb" in: [code snippet]
> > Missing space before "elementAttributes" in: [code snippet]
> I've addressed those issues in the 2nd submitted patch.
Apparently not those two items. ;-)
> + for(int k = 0; k < values.size(); k++)
> + {
> + Object baselineValue = values.get(k);
Could/should the "Object" variable be moved to outside the cycle? Although the
nesting will probably never be more than one level...
> + //for the child of the element
Seems that a superfluous (indenting) space slipped into that line. ;-)
> handleNestedBaselineShift( Map, Map ) is called from one location only and a
> few lines before the call, elementsAttributes is set to new HashMap().
> I assume it is safe not to check elementAttributes within that method.
Sounds reasonable. :-)
> I've attached test file for animation and scripting as well for the review.
Cool, I noticed the attachments but didn't have the chance to test them yet.
Nevertheless, you didn't state whether the patch properly handled those
situations (although you working suggests so).
I'd say that, even without the minor formatting stuff integrated, the patch is
ready for a deeper review from someone with a deeper knowledge on Batik's
internals (assuming that the animation cases are being properly handled, as
guessed in the previous paragraph). ;-)
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
Helder Magalhães <he...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
URL| |http://www.nabble.com/nesti
| |ng-baseline-shift-ts2387110
| |2.html
CC| |helder.magalhaes@gmail.com
Component|Bridge |GVT Text
Version|1.7 |1.8
Summary|nested baseline-shift are |[PATCH] Nested
|not additive |baseline-shift are not
| |additive
--- Comment #3 from Helder Magalhães <he...@gmail.com> 2009-06-13 01:07:25 PST ---
First of all, thanks for the report and suggested fix, Nicolas! ;-)
Few bug properties meta changes:
* Version moved to 1.8 as the issue was confirmed in trunk (revision 784345 ( https://svn.apache.org/viewcvs.cgi?view=rev&rev=784345 ));
* Component changed to GVT Text as, although Bridge also seems to be involved,
conceptually this concerns text handling (I'm not very confident about this
although IMO it sounds better);
* Added [PATCH] prefix and related keyword in order to reflect the fact that
there's a fix proposal available;
* Added the mailing list discussion prior to issue creation as the report's
URL (a bit of background for the curious). :-)
I'll try to follow-up with a set of comments regarding each of the attachments.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
Nicolas Socheleau <ni...@quickoffice.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #23814|aniamtion of |animation of
description|baseline-script property |baseline-script property
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
Nicolas Socheleau <ni...@quickoffice.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #23811|0 |1
is obsolete| |
--- Comment #12 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 13:24:15 PST ---
Created an attachment (id=23820)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23820)
minor formatting fix
- fix the 2 missing space between the comma and the parameter name (this time;)
- remove the extra space
- move the local variable outside of the loop
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #9 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 09:12:42 PST ---
Created an attachment (id=23814)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23814)
aniamtion of baseline-script property
testcase with <animate/> and <set/> to modify the baseline-shift property
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #6 from Helder Magalhães <he...@gmail.com> 2009-06-13 02:48:00 PST ---
(In reply to comment #4)
> As the test is basically a small extend of "textLayout2.svg", I'd propose
> merging the new sub-tests [...] into it ("textLayout2.svg") [...]
Oops, forgot to mention that, if one decides to keep the two tests separately,
the newly introduced should be also added to the test list
("test-resources/org/apache/batik/test/samplesRendering.xml") so regard [1] can
see it. A reference image needs to be generated also.
If the couple of introduced sub-tests are merged into the existing one, only
the reference image needs to be updated. ;-)
[1] http://xmlgraphics.apache.org/batik/dev/test.html#regard
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #5 from Helder Magalhães <he...@gmail.com> 2009-06-13 02:33:56 PST ---
(In reply to comment #2)
> Created an attachment (id=23805)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23805) [details]
> fix attempt
>
> this is a proposed patch, but I'm rusty and I haven't followed much lately any
> development on Batik, so I'm not sure it is valuable. Still, the rendering of
> baseline-shift seems correct with this patch.
Intuitively sounds good, although I haven't tested the patch myself yet. Few
comments first:
Batik uses the space character for indenting. Please update your patch (which
uses a mix of tabs and spaces) with this in mind. ;-)
Missing space before "asb" in:
+ fillAttributedStringBuffer(ctx, element, true, null, null, null,
null,asb);
Missing space before "elementAttributes" in:
+ parentAttributes =
handleNestedBaselineShift(parentAttributes,elementAttributes);
Is it safe to assume "elementAttributes" is always non-null? That is, should we
be making the same null check made for the "parentAttributes" nearby?
+ if ( elementAttributes.containsKey(BASELINE_SHIFT) )
I'd say the space before the semi-colon could be dropped in:
+ for(int k = 0 ; k < values.size(); k++)
Although this portion was adapted from previous code, I'd propose that spaces
were inserted between "==" and also the "+= -" was changed to "-=" in:
+ if
(baselineValue==TextAttribute.SUPERSCRIPT_SUPER) {
+ baselineAdjust += baselineAscent*0.5f;
+ } else if
(baselineValue==TextAttribute.SUPERSCRIPT_SUB) {
+ baselineAdjust += -baselineAscent*0.5f;
With these small changes addressed, I'd say the patch is ready for review. ;-)
Apart from the changes, I'd just like to leave a couple of thoughts which I
haven't been able to dig down properly (at least, not for now):
* Is this code ready for on-the-fly modification of the baseline-shift
property? By scripting or by placing a SMIL animation on the property, either
in the child or in the parent for a nesting sample, does the proposal keep up
with?
* Can the introduced memory allocations be introducing any memory leaks?
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] nested baseline-shift are not additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #1 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-12 01:59:49 PST ---
Created an attachment (id=23804)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23804)
better formatted testcase
testcase formatted for the batik test suite
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #8 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 09:04:59 PST ---
> Batik uses the space character for indenting. Please update your patch (which
> uses a mix of tabs and spaces) with this in mind. ;-)
>
>
> Missing space before "asb" in:
>
> + fillAttributedStringBuffer(ctx, element, true, null, null, null,
> null,asb);
>
>
> Missing space before "elementAttributes" in:
>
> + parentAttributes =
> handleNestedBaselineShift(parentAttributes,elementAttributes);
>
>
I've addressed those issues in the 2nd submitted patch.
> Is it safe to assume "elementAttributes" is always non-null? That is, should we
> be making the same null check made for the "parentAttributes" nearby?
>
> + if ( elementAttributes.containsKey(BASELINE_SHIFT) )
>
handleNestedBaselineShift( Map, Map ) is called from one location only and a
few lines before the call, elementsAttributes is set to new HashMap(). I assume
it is safe not to check elementAttributes within that method.
>
> I'd say the space before the semi-colon could be dropped in:
> + for(int k = 0 ; k < values.size(); k++)
>
>
> Although this portion was adapted from previous code, I'd propose that spaces
> were inserted between "==" and also the "+= -" was changed to "-=" in:
>
> + if
> (baselineValue==TextAttribute.SUPERSCRIPT_SUPER) {
> + baselineAdjust += baselineAscent*0.5f;
> + } else if
> (baselineValue==TextAttribute.SUPERSCRIPT_SUB) {
> + baselineAdjust += -baselineAscent*0.5f;
>
>
adjusted in the latest patch
> With these small changes addressed, I'd say the patch is ready for review. ;-)
>
>
> Apart from the changes, I'd just like to leave a couple of thoughts which I
> haven't been able to dig down properly (at least, not for now):
> * Is this code ready for on-the-fly modification of the baseline-shift
> property? By scripting or by placing a SMIL animation on the property, either
> in the child or in the parent for a nesting sample, does the proposal keep up
> with?
I've attached test file for animation and scripting as well for the review.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
Nicolas Socheleau <ni...@quickoffice.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #23805|0 |1
is obsolete| |
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #10 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-16 09:13:33 PST ---
Created an attachment (id=23815)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23815)
scripting of the baseline-shift property
testcase with onload ecmascript to modify the baseline-shift property
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] nested baseline-shift are not additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #2 from Nicolas Socheleau <ni...@quickoffice.com> 2009-06-12 02:02:36 PST ---
Created an attachment (id=23805)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23805)
fix attempt
this is a proposed patch, but I'm rusty and I haven't followed much lately any
development on Batik, so I'm not sure it is valuable. Still, the rendering of
baseline-shift seems correct with this patch.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org
DO NOT REPLY [Bug 47359] [PATCH] Nested baseline-shift are not
additive
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #4 from Helder Magalhães <he...@gmail.com> 2009-06-13 02:02:23 PST ---
(In reply to comment #1)
> Created an attachment (id=23804)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23804) [details]
> better formatted testcase
>
> testcase formatted for the batik test suite
This should apparently be placed in "samples/tests/spec/text" (quick tip to
save time). ;-)
As the test is basically a small extend of "textLayout2.svg", I'd propose
merging the new sub-tests ('<text> baseline-shift="-5pt" /
baseline-shift="+/-10pt"' and 'nested <tspan>') into it ("textLayout2.svg"),
maybe by using two or more columns such as in "textLayout.svg", and replace
this attachment for a patch against that test ("textLayout2.svg"). Note that,
for coherency with "textLayout2.svg", I'd propose dropping the guide lines used
(or adding them consistently in "textLayout2.svg" also).
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: batik-dev-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: batik-dev-help@xmlgraphics.apache.org