You are viewing a plain text version of this content. The canonical link for it is here.
Posted to general@xmlgraphics.apache.org by Vincent Hennebert <vh...@gmail.com> on 2010/02/22 13:41:01 UTC
Re: svn commit: r911864 - /xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java
Hi Jeremias,
> Author: jeremias
> Date: Fri Feb 19 16:37:29 2010
> New Revision: 911864
>
> URL: http://svn.apache.org/viewvc?rev=911864&view=rev
> Log:
> Bugfix: Use a List and a sort instead of a SortedSet to avoid problems with the equals() semantics. The result really depended on the entry order into the set rather than the order implied by the penalty value. This could lead to the wrong pipeline being used.
>
> Modified:
> xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java
>
> Modified: xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java?rev=911864&r1=911863&r2=911864&view=diff
> ==============================================================================
> --- xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java (original)
> +++ xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java Fri Feb 19 16:37:29 2010
> @@ -20,10 +20,11 @@
> @@ -143,7 +144,8 @@
> // --> List of resulting flavors, possibly multiple loaders
> loaderFactories = registry.getImageLoaderFactories(originalMime);
> if (loaderFactories != null) {
> - SortedSet candidates = new java.util.TreeSet(new PipelineComparator());
> + List candidates = new java.util.ArrayList();
> +
> //Find best pipeline -> best loader
> for (int i = 0, ci = loaderFactories.length; i < ci; i++) {
> ImageLoaderFactory loaderFactory = loaderFactories[i];
> @@ -158,9 +160,10 @@
> }
> }
>
> + Collections.sort(candidates, new PipelineComparator());
> //Build final pipeline
> if (candidates.size() > 0) {
> - pipeline = (ImageProviderPipeline)candidates.first();
> + pipeline = (ImageProviderPipeline)candidates.get(0);
> }
> }
> }
Why do you store all the pipelines in a collection if in the end you
need only the best one?
Also, do you have a test case to reproduce the issue? So that any
further change does not introduce a regression.
Thanks,
Vincent
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org
Re: svn commit: r911864 - /xmlgraphics/commons/trunk/src/java/org/apache/xmlgraphics/image/loader/pipeline/PipelineFactory.java
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Hi Vincent
On 22.02.2010 13:41:01 Vincent Hennebert wrote:
<snip/>
> Why do you store all the pipelines in a collection if in the end you
> need only the best one?
The original idea was to have a prioritized list of pipelines that can
be tried if the first n pipelines fail to load an image. But then I just
switched to using a single one for now which may explain that.
> Also, do you have a test case to reproduce the issue? So that any
> further change does not introduce a regression.
With my recent additions there are now tests that should cover that.
Jeremias Maerki
---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org