There is a really different track here on how to modify the style interfaces in a safe manner (between this proposals and anime's. I made the suggestion for FeatureTypeStyle.getSortBy() ... and the answer was to consistently use the vendor options parameter map. Down a few levels here to RasterSymbolizer and we have this proposal that is far more invasive.
Can we try for a middle ground ... can RasterSymbolizer make use of vendor options for this functionality? At least the getParameters() is acting like RasterSymbolizer.getOptions(). I notice that RasterSymbolizer already has the string based methods you are after - but are deprecated from when we moved from a String to a CodeList. Un deprecating this setType method and using getOptions() is the way to go. -- Jody Garnett On 5 August 2015 at 03:20, Ian Turton <[email protected]> wrote: > 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 <[email protected]> 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 [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
