It's doable that way but I'm not really happy about storing a bunch of
algorithm specific information at the ContrastEnhancer level. Or it leads
to a nasty mix of org.opengis.style.ContrastMethod and
org.geotools.styling.ContrastMethod and a whole bunch of messing about to
try to make sure everyone is playing with the right one (I've just had a go
at this and it is messy).
I see your point about the StyleVisitors that need to be updated but this
way they will just silently fail when they try to visit a Contrast Enhancer
that has algorithm information in it that they know nothing about. This way
they see a clear API break that they can fix easily by either extending the
abstract visitor (that I've updated) or by implementing one new method
which may well have information in it that they care about or can cleanly
ignore.
This is a new release so some API changes can be expected, but I've done my
best to keep them to a minimum and make it painless for people.
On 4 August 2015 at 22:50, Jody Garnett <jody.garn...@gmail.com> wrote:
> Sorry Ian, getting muddle between your proposal text and the subsequent
> API change section.
>
> Reviewing both together, you are not just talking about changing from
> CodeList values to Strings, you are talking about changing ContrastMethod
> to a class ... with a type and some of those types need parameters so you
> have a couple of expressions.
>
> Rather than go turtles all the way down (since ContrastMethod was
> originally a String of just this nature for ContrastEnhancement) can we
> take this back up a notch. I ask this in part due to the change to the
> StyleVisitor required by your proposal. We have a lot of style visitors not
> all of which are covered by abstract classes.
>
> Going to revise my vote to -1 due to the change to StyleVisitor being too
> invasive.
>
> Here is an alternative for discussion:
>
> public interface ContrastEnhancement {
>
> public static String GAMMA = "GAMMA";
> public static String MIN_VALUE = "minValue"; // used for ConstrastMethod.
> NORMALIZE
> public static String MAX_VALUE = "maxValue"; // used for ConstrastMethod.
> NORMALIZE
> public static String NORMALIZATION_FACTOR"= "normalizationFactor";
> public static String CORRETION_FACTOR = "correctionFactor";
>
> public ContrastMethod getMethod(); // Literal value of
> getConstrastMethod() or NONE
> public Expression getConstractMethod();
> public Expression getGammaValue(); // short cut for
> getParameters().get("GAMMA")
> public Expression getAlgorithm(); // one of StretchToMinimumMaximum,
> ClipToMinimumMaximum, ClipToZeroMaximum
> public Map<String, Expression> getParameters();
> }
>
> Where ContrastMethod remains a code list:
>
> public final class ContrastMethod extends CodeList<ContrastMethod> {
> public static final ContrastMethod NORMALIZE = new
> ContrastMethod("NORMALIZE");
> public static final ContrastMethod HISTOGRAM = new
> ContrastMethod("HISTOGRAM");
> public static final ContrastMethod NONE = new ContrastMethod("NONE");
> public static final ContrastMethod LOGARITHMIC = new
> ContrastMethod("LOGARITHMIC");
> public static final ContrastMethod EXPONENTIAL = new
> ContrastMethod("EXPONENTIAL");
> }
>
> Writing this out the result is much less invasive, I think you could
> introduce ContrastMethod.NORMALIZE (this is stretch to min /
> max), ContrastMethod.NORMALIZE_CLIP (this is clip to min max),
> ContrastMethod.NORMALIZE_CLIP_FROM_ZERO (this is eclipse to zero max) and
> avoid the getAlgorithm() method completely with no loss of expressive power.
>
> Please consider the above suggestion, it is a shame you were not in the
> meeting today - let me know if you would like a quick Skype chat to burn
> through this.
>
--
Ian Turton
------------------------------------------------------------------------------
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
