You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2008/07/28 15:03:41 UTC
svn commit: r680339 - in
/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render:
AbstractPathOrientedRenderer.java intermediate/IFGraphicContext.java
intermediate/IFRenderer.java
Author: jeremias
Date: Mon Jul 28 06:03:40 2008
New Revision: 680339
URL: http://svn.apache.org/viewvc?rev=680339&view=rev
Log:
Fixed block containers now working properly.
Modified:
xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/AbstractPathOrientedRenderer.java
xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFGraphicContext.java
xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java
Modified: xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/AbstractPathOrientedRenderer.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/AbstractPathOrientedRenderer.java?rev=680339&r1=680338&r2=680339&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/AbstractPathOrientedRenderer.java (original)
+++ xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/AbstractPathOrientedRenderer.java Mon Jul 28 06:03:40 2008
@@ -469,7 +469,8 @@
}
- private static final QName FOX_TRANSFORM
+ /** Constant for the fox:transform extension attribute */
+ protected static final QName FOX_TRANSFORM
= new QName(ExtensionElementMapping.URI, "fox:transform");
/** {@inheritDoc} */
Modified: xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFGraphicContext.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFGraphicContext.java?rev=680339&r1=680338&r2=680339&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFGraphicContext.java (original)
+++ xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFGraphicContext.java Mon Jul 28 06:03:40 2008
@@ -19,11 +19,19 @@
package org.apache.fop.render.intermediate;
+import java.awt.Dimension;
+import java.awt.Rectangle;
+import java.awt.geom.AffineTransform;
+import java.util.List;
+
import org.apache.xmlgraphics.java2d.GraphicContext;
+/**
+ * Specialized graphic context class for the intermediate format renderer.
+ */
public class IFGraphicContext extends GraphicContext {
- private int groupDepth;
+ private List groupList = new java.util.ArrayList();
/**
* Default constructor.
@@ -46,15 +54,103 @@
return new IFGraphicContext(this);
}
- public void startGroup() {
- this.groupDepth++;
+ public void pushGroup(Group group) {
+ //this.groupDepth++;
+ this.groupList.add(group);
+ for (int i = 0, c = group.getTransforms().length; i < c; i++) {
+ transform(group.getTransforms()[i]);
+ }
+ }
+
+ public Group[] getGroups() {
+ return (Group[])this.groupList.toArray(new Group[getGroupStackSize()]);
}
- public void endGroup() {
- this.groupDepth--;
+ public Group[] dropGroups() {
+ Group[] groups = getGroups();
+ this.groupList.clear();
+ return groups;
}
- public int getGroupDepth() {
- return this.groupDepth;
+ public int getGroupStackSize() {
+ return this.groupList.size();
+ }
+
+ public static class Group {
+
+ private AffineTransform[] transforms;
+
+ public Group(AffineTransform[] transforms) {
+ this.transforms = transforms;
+ }
+
+ public Group(AffineTransform transform) {
+ this(new AffineTransform[] {transform});
+ }
+
+ public AffineTransform[] getTransforms() {
+ return this.transforms;
+ }
+
+ public void start(IFPainter painter) throws IFException {
+ painter.startGroup(transforms);
+ }
+
+ public void end(IFPainter painter) throws IFException {
+ painter.endGroup();
+ }
+
+ /** {@inheritDoc} */
+ public String toString() {
+ StringBuffer sb = new StringBuffer("group: ");
+ AbstractXMLWritingIFPainter.toString(getTransforms(), sb);
+ return sb.toString();
+ }
+
}
+
+ public static class Viewport extends Group {
+
+ private Dimension size;
+ private Rectangle clipRect;
+
+ public Viewport(AffineTransform[] transforms, Dimension size, Rectangle clipRect) {
+ super(transforms);
+ this.size = size;
+ this.clipRect = clipRect;
+ }
+
+ public Viewport(AffineTransform transform, Dimension size, Rectangle clipRect) {
+ this(new AffineTransform[] {transform}, size, clipRect);
+ }
+
+ public Dimension getSize() {
+ return this.size;
+ }
+
+ public Rectangle getClipRect() {
+ return this.clipRect;
+ }
+
+ public void start(IFPainter painter) throws IFException {
+ painter.startViewport(getTransforms(), size, clipRect);
+ }
+
+ public void end(IFPainter painter) throws IFException {
+ painter.endViewport();
+ }
+
+ /** {@inheritDoc} */
+ public String toString() {
+ StringBuffer sb = new StringBuffer("viewport: ");
+ AbstractXMLWritingIFPainter.toString(getTransforms(), sb);
+ sb.append(", ").append(getSize());
+ if (getClipRect() != null) {
+ sb.append(", ").append(getClipRect());
+ }
+ return sb.toString();
+ }
+
+ }
+
}
Modified: xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java?rev=680339&r1=680338&r2=680339&view=diff
==============================================================================
--- xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java (original)
+++ xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java Mon Jul 28 06:03:40 2008
@@ -34,6 +34,7 @@
import org.xml.sax.SAXException;
+import org.apache.batik.parser.AWTTransformProducer;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -89,6 +90,7 @@
private Stack graphicContextStack = new Stack();
private Stack viewportDimensionStack = new Stack();
private IFGraphicContext graphicContext = new IFGraphicContext();
+ //private Stack groupStack = new Stack();
private Metadata documentMetadata;
@@ -293,24 +295,45 @@
/** {@inheritDoc} */
protected void restoreGraphicsState() {
- while (graphicContext.getGroupDepth() > 0) {
- try {
- painter.endGroup();
- } catch (IFException e) {
- handleIFException(e);
+ while (graphicContext.getGroupStackSize() > 0) {
+ IFGraphicContext.Group[] groups = graphicContext.dropGroups();
+ for (int i = groups.length - 1; i >= 0; i--) {
+ try {
+ groups[i].end(painter);
+ } catch (IFException ife) {
+ handleIFException(ife);
+ }
}
- graphicContext.endGroup();
}
graphicContext = (IFGraphicContext)graphicContextStack.pop();
}
+ private void pushGroup(IFGraphicContext.Group group) {
+ graphicContext.pushGroup(group);
+ try {
+ group.start(painter);
+ } catch (IFException ife) {
+ handleIFException(ife);
+ }
+ }
+
/** {@inheritDoc} */
protected List breakOutOfStateStack() {
log.debug("Block.FIXED --> break out");
List breakOutList = new java.util.ArrayList();
while (!this.graphicContextStack.empty()) {
+ //Handle groups
+ IFGraphicContext.Group[] groups = graphicContext.getGroups();
+ for (int j = groups.length - 1; j >= 0; j--) {
+ try {
+ groups[j].end(painter);
+ } catch (IFException ife) {
+ handleIFException(ife);
+ }
+ }
+
breakOutList.add(0, this.graphicContext);
- restoreGraphicsState();
+ graphicContext = (IFGraphicContext)graphicContextStack.pop();
}
return breakOutList;
}
@@ -319,25 +342,36 @@
protected void restoreStateStackAfterBreakOut(List breakOutList) {
log.debug("Block.FIXED --> restoring context after break-out");
for (int i = 0, c = breakOutList.size(); i < c; i++) {
- saveGraphicsState();
+ graphicContextStack.push(graphicContext);
this.graphicContext = (IFGraphicContext)breakOutList.get(i);
+
+ //Handle groups
+ IFGraphicContext.Group[] groups = graphicContext.getGroups();
+ for (int j = 0, jc = groups.length; j < jc; j++) {
+ try {
+ groups[j].start(painter);
+ } catch (IFException ife) {
+ handleIFException(ife);
+ }
+ }
}
+ log.debug("restored.");
}
/** {@inheritDoc} */
protected void concatenateTransformationMatrix(AffineTransform at) {
if (!at.isIdentity()) {
+ concatenateTransformationMatrixMpt(ptToMpt(at));
+ }
+ }
+
+ private void concatenateTransformationMatrixMpt(AffineTransform at) {
+ if (!at.isIdentity()) {
if (log.isDebugEnabled()) {
log.debug("-----concatenateTransformationMatrix: " + at);
}
- AffineTransform atmpt = ptToMpt(at);
- graphicContext.transform(atmpt);
- graphicContext.startGroup();
- try {
- painter.startGroup(atmpt);
- } catch (IFException e) {
- handleIFException(e);
- }
+ IFGraphicContext.Group group = new IFGraphicContext.Group(at);
+ pushGroup(group);
}
}
@@ -361,9 +395,120 @@
/** {@inheritDoc} */
protected void renderBlockViewport(BlockViewport bv, List children) {
+ //Essentially the same code as in the super class but optimized for the IF
+
+ //This is the content-rect
Dimension dim = new Dimension(bv.getIPD(), bv.getBPD());
viewportDimensionStack.push(dim);
- super.renderBlockViewport(bv, children);
+
+ // save positions
+ int saveIP = currentIPPosition;
+ int saveBP = currentBPPosition;
+
+ CTM ctm = bv.getCTM();
+ int borderPaddingStart = bv.getBorderAndPaddingWidthStart();
+ int borderPaddingBefore = bv.getBorderAndPaddingWidthBefore();
+
+ if (bv.getPositioning() == Block.ABSOLUTE
+ || bv.getPositioning() == Block.FIXED) {
+
+ //For FIXED, we need to break out of the current viewports to the
+ //one established by the page. We save the state stack for restoration
+ //after the block-container has been painted. See below.
+ List breakOutList = null;
+ if (bv.getPositioning() == Block.FIXED) {
+ breakOutList = breakOutOfStateStack();
+ }
+
+ AffineTransform positionTransform = new AffineTransform();
+ positionTransform.translate(bv.getXOffset(), bv.getYOffset());
+
+ //"left/"top" (bv.getX/YOffset()) specify the position of the content rectangle
+ positionTransform.translate(-borderPaddingStart, -borderPaddingBefore);
+
+ //Free transformation for the block-container viewport
+ String transf;
+ transf = bv.getForeignAttributeValue(FOX_TRANSFORM);
+ if (transf != null) {
+ AffineTransform freeTransform = AWTTransformProducer.createAffineTransform(transf);
+ positionTransform.concatenate(freeTransform);
+ }
+
+ saveGraphicsState();
+ //Viewport position
+ concatenateTransformationMatrixMpt(positionTransform);
+
+ //Background and borders
+ float bpwidth = (borderPaddingStart + bv.getBorderAndPaddingWidthEnd());
+ float bpheight = (borderPaddingBefore + bv.getBorderAndPaddingWidthAfter());
+ drawBackAndBorders(bv, 0, 0,
+ (dim.width + bpwidth) / 1000f, (dim.height + bpheight) / 1000f);
+
+ //Shift to content rectangle after border painting
+ AffineTransform contentRectTransform = new AffineTransform();
+ contentRectTransform.translate(borderPaddingStart, borderPaddingBefore);
+ concatenateTransformationMatrixMpt(contentRectTransform);
+
+ //Clipping
+ Rectangle clipRect = null;
+ if (bv.getClip()) {
+ clipRect = new Rectangle(0, 0, dim.width, dim.height);
+ //clipRect(0f, 0f, width, height);
+ }
+
+ //saveGraphicsState();
+ //Set up coordinate system for content rectangle
+ AffineTransform contentTransform = ctm.toAffineTransform();
+ //concatenateTransformationMatrixMpt(contentTransform);
+ startViewport(contentTransform, clipRect);
+
+ currentIPPosition = 0;
+ currentBPPosition = 0;
+ renderBlocks(bv, children);
+
+ endViewport();
+ //restoreGraphicsState();
+ restoreGraphicsState();
+
+ if (breakOutList != null) {
+ restoreStateStackAfterBreakOut(breakOutList);
+ }
+
+ currentIPPosition = saveIP;
+ currentBPPosition = saveBP;
+ } else {
+
+ currentBPPosition += bv.getSpaceBefore();
+
+ //borders and background in the old coordinate system
+ handleBlockTraits(bv);
+
+ //Advance to start of content area
+ currentIPPosition += bv.getStartIndent();
+
+ CTM tempctm = new CTM(containingIPPosition, currentBPPosition);
+ ctm = tempctm.multiply(ctm);
+
+ //Now adjust for border/padding
+ currentBPPosition += borderPaddingBefore;
+
+ Rectangle2D clippingRect = null;
+ if (bv.getClip()) {
+ clippingRect = new Rectangle(currentIPPosition, currentBPPosition,
+ bv.getIPD(), bv.getBPD());
+ }
+
+ startVParea(ctm, clippingRect);
+ currentIPPosition = 0;
+ currentBPPosition = 0;
+ renderBlocks(bv, children);
+ endVParea();
+
+ currentIPPosition = saveIP;
+ currentBPPosition = saveBP;
+
+ currentBPPosition += (int)(bv.getAllocBPD());
+ }
viewportDimensionStack.pop();
}
@@ -380,40 +525,45 @@
if (log.isDebugEnabled()) {
log.debug("startVParea() ctm=" + ctm + ", clippingRect=" + clippingRect);
}
- saveGraphicsState();
AffineTransform at = new AffineTransform(ctm.toArray());
- graphicContext.transform(at);
- try {
- Rectangle clipRect = null;
- if (clippingRect != null) {
- clipRect = new Rectangle(
- (int)clippingRect.getMinX() - currentIPPosition,
- (int)clippingRect.getMinY() - currentBPPosition,
- (int)clippingRect.getWidth(), (int)clippingRect.getHeight());
- }
- painter.startViewport(at, (Dimension)viewportDimensionStack.peek(), clipRect);
- } catch (IFException e) {
- handleIFException(e);
+ Rectangle clipRect = null;
+ if (clippingRect != null) {
+ clipRect = new Rectangle(
+ (int)clippingRect.getMinX() - currentIPPosition,
+ (int)clippingRect.getMinY() - currentBPPosition,
+ (int)clippingRect.getWidth(), (int)clippingRect.getHeight());
}
+ startViewport(at, clipRect);
if (log.isDebugEnabled()) {
log.debug("startVPArea: " + at + " --> " + graphicContext.getTransform());
}
}
- /** {@inheritDoc} */
- protected void endVParea() {
- log.debug("endVParea()");
+ private void startViewport(AffineTransform at, Rectangle clipRect) {
+ saveGraphicsState();
try {
- painter.endViewport();
+ IFGraphicContext.Viewport viewport = new IFGraphicContext.Viewport(
+ at, (Dimension)viewportDimensionStack.peek(), clipRect);
+ graphicContext.pushGroup(viewport);
+ viewport.start(painter);
} catch (IFException e) {
handleIFException(e);
}
- restoreGraphicsState();
+ }
+
+ /** {@inheritDoc} */
+ protected void endVParea() {
+ log.debug("endVParea()");
+ endViewport();
if (log.isDebugEnabled()) {
log.debug("endVPArea() --> " + graphicContext.getTransform());
}
}
+ private void endViewport() {
+ restoreGraphicsState();
+ }
+
/*
protected void renderReferenceArea(Block block) {
// TODO Auto-generated method stub
---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
Re: Code Duplication [Re: svn commit: r680339 - in /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render: AbstractPathOrientedRenderer.java intermediate/IFGraphicContext.java intermediate/IFRenderer.java]
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 09.09.2008 17:02:22 Vincent Hennebert wrote:
> Jeremias Maerki wrote:
> > On 09.09.2008 13:01:54 Vincent Hennebert wrote:
> <snip/>
> >>> /** {@inheritDoc} */
> >>> protected void renderBlockViewport(BlockViewport bv, List children) {
> >>> + //Essentially the same code as in the super class but optimized for the IF
> >> More than 80% of the code of this method is completely identical to the
> >> overridden method. Surely there’s a way to optimize it and avoid that
> >> code duplication in the same time...
> >
> > I've spent a lot of time in implementations of renderBlockViewport in
> > the past. I've been able to make small improvements over time but I've
> > never found a solution that fits all output formats so far. Without any
> > major refactoring of the code anyway, which is basically what I'm doing
> > in the branch right now.
> >
> >> I can’t think of any good reason for copy-pasting code. That
> >> artificially increases the size of the codebase, that makes the code
> >> more difficult to understand and much more tricky to maintain. As soon
> >> as a change is made somewhere, you’re almost sure that at least one
> >> duplicated part will be forgotten somewhere.
> >
> > Don't you think I know that copy/paste programming is bad?
>
> That’s precisely why I don’t understand why there’s so much of that to
> be found in the current code. Performance is not an argument if you ask
> me. We’re not developing a software that will run on a micro-controller,
> with a 68000 processor and where memory is counted in Kb instead of Gb.
> The complexity of the topic makes it much more important to have
> a codebase that is clean and conceptually simple. Anyway, there’s IMO
> much more to achieve in terms of performance by working on the software
> architecture rather than replacing a few method calls, early exiting
> a loop, or whatever.
>
I think I'm missing something. Who brought up performance as an argument?
BTW, "XSL-FO" and "conceptually simple" are mutually exclusive. ;-)
> >> There is already way too much code duplication in the present codebase.
> >> Let’s avoid introducing even more duplication in new commits, shall we?
> >
> > I'm trying all the time. Please note: the branch is work in progress and
> > once it's done many renderer implementations will get deprecated to
> > lighten the maintainance burden. Eventually they'll get removed and the
> > copy/paste problem goes away.
>
> Then where is the TODO comment explaining exactly that? That would have
> saved me the time to write this message, and saved you the time to
> answer it. I can’t read in your mind...
The wiki page implies that this will happen. It makes no sense to keep
two parallel implementations for each output format. So I guess I relied
on common sense. I'll add a TODO on the Wiki.
Jeremias Maerki
Re: Code Duplication [Re: svn commit: r680339 - in /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render:
AbstractPathOrientedRenderer.java intermediate/IFGraphicContext.java intermediate/IFRenderer.java]
Posted by Vincent Hennebert <vh...@gmail.com>.
Jeremias Maerki wrote:
> On 09.09.2008 13:01:54 Vincent Hennebert wrote:
<snip/>
>>> /** {@inheritDoc} */
>>> protected void renderBlockViewport(BlockViewport bv, List children) {
>>> + //Essentially the same code as in the super class but optimized for the IF
>> More than 80% of the code of this method is completely identical to the
>> overridden method. Surely there’s a way to optimize it and avoid that
>> code duplication in the same time...
>
> I've spent a lot of time in implementations of renderBlockViewport in
> the past. I've been able to make small improvements over time but I've
> never found a solution that fits all output formats so far. Without any
> major refactoring of the code anyway, which is basically what I'm doing
> in the branch right now.
>
>> I can’t think of any good reason for copy-pasting code. That
>> artificially increases the size of the codebase, that makes the code
>> more difficult to understand and much more tricky to maintain. As soon
>> as a change is made somewhere, you’re almost sure that at least one
>> duplicated part will be forgotten somewhere.
>
> Don't you think I know that copy/paste programming is bad?
That’s precisely why I don’t understand why there’s so much of that to
be found in the current code. Performance is not an argument if you ask
me. We’re not developing a software that will run on a micro-controller,
with a 68000 processor and where memory is counted in Kb instead of Gb.
The complexity of the topic makes it much more important to have
a codebase that is clean and conceptually simple. Anyway, there’s IMO
much more to achieve in terms of performance by working on the software
architecture rather than replacing a few method calls, early exiting
a loop, or whatever.
>> There is already way too much code duplication in the present codebase.
>> Let’s avoid introducing even more duplication in new commits, shall we?
>
> I'm trying all the time. Please note: the branch is work in progress and
> once it's done many renderer implementations will get deprecated to
> lighten the maintainance burden. Eventually they'll get removed and the
> copy/paste problem goes away.
Then where is the TODO comment explaining exactly that? That would have
saved me the time to write this message, and saved you the time to
answer it. I can’t read in your mind...
Vincent
Re: Code Duplication [Re: svn commit: r680339 - in /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render: AbstractPathOrientedRenderer.java intermediate/IFGraphicContext.java intermediate/IFRenderer.java]
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
On 09.09.2008 13:01:54 Vincent Hennebert wrote:
> Hi,
>
> (Still catching up with the commits of the past month...)
>
> I’d like to take this commit as an opportunity to launch the debate
> about copy-pasting:
>
> > Date: Mon Jul 28 06:03:40 2008
> > New Revision: 680339
> <snip/>
> > Modified:
> > xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java
> > URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java?rev=680339&r1=680338&r2=680339&view=diff
> > ==============================================================================
> > --- xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java (original)
> > +++ xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java Mon Jul 28 06:03:40 2008
> > @@ -361,9 +395,120 @@
> >
> > /** {@inheritDoc} */
> > protected void renderBlockViewport(BlockViewport bv, List children) {
> > + //Essentially the same code as in the super class but optimized for the IF
>
> More than 80% of the code of this method is completely identical to the
> overridden method. Surely there’s a way to optimize it and avoid that
> code duplication in the same time...
I've spent a lot of time in implementations of renderBlockViewport in
the past. I've been able to make small improvements over time but I've
never found a solution that fits all output formats so far. Without any
major refactoring of the code anyway, which is basically what I'm doing
in the branch right now.
> I can’t think of any good reason for copy-pasting code. That
> artificially increases the size of the codebase, that makes the code
> more difficult to understand and much more tricky to maintain. As soon
> as a change is made somewhere, you’re almost sure that at least one
> duplicated part will be forgotten somewhere.
Don't you think I know that copy/paste programming is bad?
> There is already way too much code duplication in the present codebase.
> Let’s avoid introducing even more duplication in new commits, shall we?
I'm trying all the time. Please note: the branch is work in progress and
once it's done many renderer implementations will get deprecated to
lighten the maintainance burden. Eventually they'll get removed and the
copy/paste problem goes away.
Jeremias Maerki
Code Duplication [Re: svn commit: r680339 - in /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render:
AbstractPathOrientedRenderer.java intermediate/IFGraphicContext.java intermediate/IFRenderer.java]
Posted by Vincent Hennebert <vh...@gmail.com>.
Hi,
(Still catching up with the commits of the past month...)
I’d like to take this commit as an opportunity to launch the debate
about copy-pasting:
> Date: Mon Jul 28 06:03:40 2008
> New Revision: 680339
<snip/>
> Modified:
> xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java?rev=680339&r1=680338&r2=680339&view=diff
> ==============================================================================
> --- xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java (original)
> +++ xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/intermediate/IFRenderer.java Mon Jul 28 06:03:40 2008
> @@ -361,9 +395,120 @@
>
> /** {@inheritDoc} */
> protected void renderBlockViewport(BlockViewport bv, List children) {
> + //Essentially the same code as in the super class but optimized for the IF
More than 80% of the code of this method is completely identical to the
overridden method. Surely there’s a way to optimize it and avoid that
code duplication in the same time...
I can’t think of any good reason for copy-pasting code. That
artificially increases the size of the codebase, that makes the code
more difficult to understand and much more tricky to maintain. As soon
as a change is made somewhere, you’re almost sure that at least one
duplicated part will be forgotten somewhere.
There is already way too much code duplication in the present codebase.
Let’s avoid introducing even more duplication in new commits, shall we?
Thanks,
Vincent