Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Sorry for the delay. On 12.10.2004 01:20:54 Glen Mazza wrote: --- Jeremias Maerki [EMAIL PROTECTED] wrote: Glen, I don't particularly like selecting renderers by integer constant. FOP has been using integers for six years now, and as I explained earlier [1], MIME types don't make a very good primary key for renderers, because not all renderers have a MIME type, also multiple renderers can share the same MIME type (e.g. our PDF renderer and Finn's version of it.). In some cases, we would may indeed need a RENDER_PDF_V14 and an RENDER_PDF_V15, for example. I would disagree with that. There are ways to do all that pretty cleanly. The point is that with your approach you always (!) have to work in Java code to use a non-standard renderer. It can't be plugged in from the command-line for example. Again, integer constants also work well in arrays, they are ultra-fast, and, as a bonus, new Fop(RENDER_PFD) is a ultra-quick-to-catch compile-time error, while new FOP(application/pfd) would turn into a pesky run-time error (and even a ML question). The integer constants are also much easier to remember than MIME types: RENDER_ followed by the desired render type. Performance is really unimportant when we're talking about choosing the renderer. The compile-time checking argument is valid, however. [1] http://marc.theaimsgroup.com/?l=fop-devm=109261374110951w=2 I like pluggability. We sufficiently have that for our next release, where I define sufficiently as as much as we have already in 0.20.5, and basically what AH/RX offers. We have already learned that adding too many hooks, interfaces, visitor patterns, *before* the layout/renderer work is done, results in FOP never getting finished because the code becomes too hard to work through. So let's get the code working first (moving the renderers over, whitespace handling, PDF bookmarks, layout), *then* put it into Mandarin with the advanced API many desire. As you like. But this will stay on my personal todo list. I share your impatience with the next release not coming out yet, but we have to keep the code simple to get more people to look at and help in the code. I agree, but IMHO it's confusing that we're choosing standard renderers in one place (Fop.java) but provide a possibility to set non-standard renderer in another (FOUserAgent.java). I'd prefer to register all our renderers in a central registry. Integrators could plug in their renderers using the service interface just as the FOP extension can. That's beautiful post-0.30/1.0 talk that should be added to our roadmap. But, in the meantime, we already have a perfectly satisfactory FOUserAgent.setRendererOverride() that will satisfy the users that currently have renderer overrides. I would prefer a minimal API with as much black-boxed as possible to allow future implementors as much architectural freedom as possible. Besides, what you prefer now is not what the team preferred six months ago, or a year ago, or 18 months ago, etc., etc. Our API/internal architecture desires keep changing. Are they? Is that how you justify ignoring API discussions held earlier? You could even override renderers (for application/postscript for example) We already have that with setRendererOverride(Renderer). You can't use MIME type for reasons given above. I can't override a Renderer without changing Java code. if you have a renderer with some custom extensions. IMO FOUserAgent is already overloaded with too many different things. That is because we tend to, whenever any user might want something, give him/her an API method for it. The only way we can satisfy these many needs without forever locking the FOP architecture (each of these switches internally modifying the behavior of one internal class or another) is to add the method to FOUserAgent, and have internal classes read it, rather than the other way around. Furthermore, we have one class--FOUserAgent--for these parameters to make the embedded/servlet API easier for non-advanced users. (BTW, FOUserAgent should be reduced somewhat anyway, when we create fox:metadata or whatever.) Means keep Fop.java as lean as possible but no problem with FOUserAgent getting bloated. I don't get it. I think that the overrides don't belong there, or rather they are probably unnecessary given a better renderer selection mechanism. Oh, I think they do, it is sufficient for our next release. Nothing is easier, nothing is simpler for embedded/servlet users. Take a look at the 1.0 embedded examples. My desires for the API don't complicate things for the embedded examples. What triggered the creation of the RendererFactory was not IoC but the other Avalon concept: Separation of concerns. I think that handling renderer and FOEventHandler creation in a separate class rather than in the quite crowded FOTreeBuilder and
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Glen, I don't want a RendererFactory.setRendererOverride(). I don't particularly like selecting renderers by integer constant. I like pluggability. I'd prefer to register all our renderers in a central registry. Integrators could plug in their renderers using the service interface just as the FOP extension can. You could even override renderers (for application/postscript for example) if you have a renderer with some custom extensions. IMO FOUserAgent is already overloaded with too many different things. I think that the overrides don't belong there, or rather they are probably unnecessary given a better renderer selection mechanism. What triggered the creation of the RendererFactory was not IoC but the other Avalon concept: Separation of concerns. I think that handling renderer and FOEventHandler creation in a separate class rather than in the quite crowded FOTreeBuilder and RenderPagesModel improves code readability. Don't get me wrong. I can live with the current situation as long as I can plug in my custom FOEventHandler. You don't object to that, do you? The RendererFactory simply came out of the fact that I need to plug in my FOEventHandler and I found that the code was IMHO a bit unclean. By the way, my change didn't change the end-user API, so I don't see any reason why RendererFactory is a bad idea. Do I miss your point? On 11.10.2004 01:46:54 Glen Mazza wrote: Jeremias, The reason for getFOEventHandlerOverride()/getRendererOverride() in FOUserAgent is to better black box the FOP system. This gives us a ton of freedom internally of where we implement FOEventHandlers and Renderers inside FOP. This has been a perfect example: So far over the past few months, we've been setting the renderer in apps.Driver(), then in fo.FOTreeControl, then in area.AreaTreeHandler, and then in RenderPagesModel, and now RendererFactory. But note that during all these movements, FOUserAgent.getRendererOverride() never had to change! I don't know if this is what is meant by Avalon's Inversion of Control, or if this is a separate concept, but it is providing us wonderful flexibility. If, instead, we provided a user interface that had the outside user *directly* call RendererFactory.setRendererOverride(), we would be forever stuck with having this code in a RendererFactory class. Good for you, but some preferred it in AreaTreeModel, and some wanted it in apps.Document or apps.Driver. Point is, future committers, because of backwards compatibility issues, would be forever frozen with a single design. Jeremias Maerki
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
--- Jeremias Maerki [EMAIL PROTECTED] wrote: Glen, I don't particularly like selecting renderers by integer constant. FOP has been using integers for six years now, and as I explained earlier [1], MIME types don't make a very good primary key for renderers, because not all renderers have a MIME type, also multiple renderers can share the same MIME type (e.g. our PDF renderer and Finn's version of it.). In some cases, we would may indeed need a RENDER_PDF_V14 and an RENDER_PDF_V15, for example. Again, integer constants also work well in arrays, they are ultra-fast, and, as a bonus, new Fop(RENDER_PFD) is a ultra-quick-to-catch compile-time error, while new FOP(application/pfd) would turn into a pesky run-time error (and even a ML question). The integer constants are also much easier to remember than MIME types: RENDER_ followed by the desired render type. [1] http://marc.theaimsgroup.com/?l=fop-devm=109261374110951w=2 I like pluggability. We sufficiently have that for our next release, where I define sufficiently as as much as we have already in 0.20.5, and basically what AH/RX offers. We have already learned that adding too many hooks, interfaces, visitor patterns, *before* the layout/renderer work is done, results in FOP never getting finished because the code becomes too hard to work through. So let's get the code working first (moving the renderers over, whitespace handling, PDF bookmarks, layout), *then* put it into Mandarin with the advanced API many desire. I share your impatience with the next release not coming out yet, but we have to keep the code simple to get more people to look at and help in the code. I'd prefer to register all our renderers in a central registry. Integrators could plug in their renderers using the service interface just as the FOP extension can. That's beautiful post-0.30/1.0 talk that should be added to our roadmap. But, in the meantime, we already have a perfectly satisfactory FOUserAgent.setRendererOverride() that will satisfy the users that currently have renderer overrides. I would prefer a minimal API with as much black-boxed as possible to allow future implementors as much architectural freedom as possible. Besides, what you prefer now is not what the team preferred six months ago, or a year ago, or 18 months ago, etc., etc. Our API/internal architecture desires keep changing. You could even override renderers (for application/postscript for example) We already have that with setRendererOverride(Renderer). You can't use MIME type for reasons given above. if you have a renderer with some custom extensions. IMO FOUserAgent is already overloaded with too many different things. That is because we tend to, whenever any user might want something, give him/her an API method for it. The only way we can satisfy these many needs without forever locking the FOP architecture (each of these switches internally modifying the behavior of one internal class or another) is to add the method to FOUserAgent, and have internal classes read it, rather than the other way around. Furthermore, we have one class--FOUserAgent--for these parameters to make the embedded/servlet API easier for non-advanced users. (BTW, FOUserAgent should be reduced somewhat anyway, when we create fox:metadata or whatever.) I think that the overrides don't belong there, or rather they are probably unnecessary given a better renderer selection mechanism. Oh, I think they do, it is sufficient for our next release. Nothing is easier, nothing is simpler for embedded/servlet users. Take a look at the 1.0 embedded examples. What triggered the creation of the RendererFactory was not IoC but the other Avalon concept: Separation of concerns. I think that handling renderer and FOEventHandler creation in a separate class rather than in the quite crowded FOTreeBuilder and RenderPagesModel improves code readability. FOTreeBuilder and RenderPagesModel IMO aren't that crowded, they are the size they need to be for the business logic they are responsible for. Still, I've bent over backwards to remove as many classes and unneeded methods to simplify the code as much as possible. Furthermore, bouncing around classes IMO raises stress rather than lowering it. As for Avalon SoC, the concerns *were* properly separated: the FOTreeBuilder chose the FOEventHandler based on the desired render type, and RenderPagesModel chooses its renderer also on the desired render type. It is business logic purely the departments of the FOTreeBuilder and RenderPagesModel. Still, I'm not objecting to this change as long as it's not the external API. (Indeed, I support it, because it makes you happy, and when you're happy you're more productive.) Don't get me wrong. I can live with the current situation as long as I can plug in my custom FOEventHandler. You don't object to that, do you? Not at all, because it does not directly touch internal code. Future
Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java
Jeremias, The reason for getFOEventHandlerOverride()/getRendererOverride() in FOUserAgent is to better black box the FOP system. This gives us a ton of freedom internally of where we implement FOEventHandlers and Renderers inside FOP. This has been a perfect example: So far over the past few months, we've been setting the renderer in apps.Driver(), then in fo.FOTreeControl, then in area.AreaTreeHandler, and then in RenderPagesModel, and now RendererFactory. But note that during all these movements, FOUserAgent.getRendererOverride() never had to change! I don't know if this is what is meant by Avalon's Inversion of Control, or if this is a separate concept, but it is providing us wonderful flexibility. If, instead, we provided a user interface that had the outside user *directly* call RendererFactory.setRendererOverride(), we would be forever stuck with having this code in a RendererFactory class. Good for you, but some preferred it in AreaTreeModel, and some wanted it in apps.Document or apps.Driver. Point is, future committers, because of backwards compatibility issues, would be forever frozen with a single design. Thanks, Glen [EMAIL PROTECTED] schrieb: jeremias2004/10/10 05:24:03 Modified:src/java/org/apache/fop/fo FOTreeBuilder.java src/java/org/apache/fop/area RenderPagesModel.java src/java/org/apache/fop/apps FOUserAgent.java Added: src/java/org/apache/fop/render RendererFactory.java Log: Centralized Renderer and FOEventHandler creation in the RenderFactory class. Provide a similar mechanism as for the Renderers to override the FOEventHandler being used to be able to plug in custom FOEventHandlers. (I don't particularly like this approach but so be it for the moment.) Javadocs updates in FOUserAgent. Revision ChangesPath 1.1 xml-fop/src/java/org/apache/fop/render/RendererFactory.java Index: RendererFactory.java === /* * Copyright 2004 The Apache Software Foundation. * * Licensed under the Apache License, Version 2.0 (the License); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an AS IS BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ /* $Id: RendererFactory.java,v 1.1 2004/10/10 12:24:02 jeremias Exp $ */ package org.apache.fop.render; import java.io.OutputStream; //Avalon import org.apache.avalon.framework.configuration.Configuration; import org.apache.avalon.framework.configuration.ConfigurationException; import org.apache.avalon.framework.container.ContainerUtil; //FOP import org.apache.fop.apps.FOPException; import org.apache.fop.apps.FOUserAgent; import org.apache.fop.area.AreaTreeHandler; import org.apache.fop.fo.Constants; import org.apache.fop.fo.FOEventHandler; import org.apache.fop.render.mif.MIFHandler; import org.apache.fop.render.rtf.RTFHandler; /** * Factory for FOEventHandlers and Renderers. */ public class RendererFactory { /** * Creates a Renderer object based on render-type desired * @param renderType the type of renderer to use * @return the new Renderer instance * @throws IllegalArgumentException if an unsupported renderer type was requested */ private static Renderer newInstance(int renderType) throws IllegalArgumentException { switch (renderType) { case Constants.RENDER_PDF: return new org.apache.fop.render.pdf.PDFRenderer(); case Constants.RENDER_AWT: return new org.apache.fop.render.awt.AWTRenderer(); case Constants.RENDER_PRINT: return new org.apache.fop.render.awt.AWTPrintRenderer(); case Constants.RENDER_PCL: return new org.apache.fop.render.pcl.PCLRenderer(); case Constants.RENDER_PS: return new org.apache.fop.render.ps.PSRenderer(); case Constants.RENDER_TXT: return new org.apache.fop.render.txt.TXTRenderer(); case Constants.RENDER_XML: return new org.apache.fop.render.xml.XMLRenderer(); case Constants.RENDER_SVG: return new org.apache.fop.render.svg.SVGRenderer(); default: throw new IllegalArgumentException(Invalid renderer type + renderType); } } /** * Creates a Renderer object based on render-type desired * @param userAgent the user agent for access to configuration * @param renderType the type of renderer to use * @return the new Renderer instance *