https://issues.apache.org/bugzilla/show_bug.cgi?id=47359
--- Comment #5 from Helder Magalhães <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
