[Geotools-devel] Jenkins build is back to normal : GeoTools-Master #688

[winbuild]

See 


--
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

[Geotools-devel] Jenkins build is back to normal : GeoTools-Master » Sample data module #688

[winbuild]

See 



--
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Proposal to implement specific methods for Contrast Enhancement

[ian]

On Wed, 5 Aug, 2015 at 5:27 PM, Jody Garnett  
wrote:
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().


OK after a quick chat with Jody (which must have been really early for 
him, so thanks for getting up now). I'm going to go with a plan to use 
Vendor Options for this which is almost certainly a better way of doing 
it for interoperability. 

This will change the proposal a bit and I'll hack on it later to 
reflect the changes I actually make.


The plan is to use the existing (but deprecated) getType() in 
ContrastEnhancement to store a string or a code list ContrastMethod 
which will alert users of the class to go check for options like 
algorithm and parameters when they come to do some enhancement work.


Ian
--
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Re: [Geotools-devel] Proposal to implement specific methods for Contrast Enhancement

[Jody Garnett]

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  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  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 getParameters();
>> }
>>
>> Where ContrastMethod remains a code list:
>>
>> public final class ContrastMethod extends CodeList {
>> 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

Re: [Geotools-devel] Un-deprecating multi-font support in TextSymbolizer

[Jody Garnett]

Quick +1 for restoring the functionality.

Additional comments inline ...

The methods in questions are in this interface:
>
> https://github.com/geotools/geotools/blob/master/modules/library/api/src/main/java/org/geotools/styling/TextSymbolizer.java#L321
>

This is our GeoTools "shadow" read/write interface - so it is fairly easy
to use for extra/non-standard/vendor options :)

I would like to recommend the following:

/**
 * Returns a device independent Font object that is to be used to render
 * the label.
 *
 * @deprecated use getFont()
 */
Font[] getFonts();

/** Direct access to list of fonts used to render the label */
List fonts();

This approach is more consistent with our rewrite to make the geotools
style interfaces direct access collections (rather than array based).

Why this was a mistake:
> * SLD 1.0 is still supported, and so we should avoid throwing away the
> extra fonts that can be declared
>   as fallbacks like the current TextSymbolizerImpl does (in order to align
> with that deprecation)
> * SLD 1.0 is for once more expressive than SE 1.1, so deprecating those
> methods is a functional regression.
>

Yeah I am a bit werided out by that ... your explanation is sound. Here is
the example from the OGC docs:


  Arial
  Sans-Serif
  italic
  10


The second statement warrants an explanation. In SE 1.1 a Font can have
> multiple font families,
> but can still have only one of the other attributes, so all font families
> are cast to use the same
> size... which is pretty ridiculous, I believe that everybody ever using a
> word processor noticed
> how monospaced fonts tend to be bigger than non monospaced ones, don't
> know about you,
> but I normally bring monospaced fonts size down 1 or 2 points to
> compensate.
>

Is it worth considering the following:


Arial Black
Arial
Sans-Serif
normal
bold
bold
10
11



I have commented out the last font-size setting to show carrying forward
the last "setting" when defining the list of fonts.

We can still parse this into our Font[] internally.


> As a possible alternative, we could downgrade SLD 1.0 flexibility to SE
> 1.1 one, keep the
> deprecations, but TextSymbolizerImpl would still have to be modified not
> to eat away
> the extra fonts like it does today (SLDParser sets n fonts, TextSymbolizer
> retains only
> the first).
>

See above for idea on encoding a list of fonts in SE 1.1.


> CSS wise, there is not support for multiple fonts right now (not even for
> the family),
> not sure if I'll managed to work on that too, but it's unrelated (the
> limitation was
> already there) and the module is unsupported anyways.
>

Got it, if you don't have scope for SE 1.1 representation I would like to
table the approach for later.

Jody
--
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

[Geotools-devel] Un-deprecating multi-font support in TextSymbolizer

[Andrea Aime]

Hi,
I would like to un-deprecate all the multi-font methods that
have been deprecated for a while in GeoTools, and that
utterly broke multi-font support for a number of years.

The methods in questions are in this interface:
https://github.com/geotools/geotools/blob/master/modules/library/api/src/main/java/org/geotools/styling/TextSymbolizer.java#L321

Why this was a mistake:
* SLD 1.0 is still supported, and so we should avoid throwing away the
extra fonts that can be declared
  as fallbacks like the current TextSymbolizerImpl does (in order to align
with that deprecation)
* SLD 1.0 is for once more expressive than SE 1.1, so deprecating those
methods is a functional regression.

The second statement warrants an explanation. In SE 1.1 a Font can have
multiple font families,
but can still have only one of the other attributes, so all font families
are cast to use the same
size... which is pretty ridiculous, I believe that everybody ever using a
word processor noticed
how monospaced fonts tend to be bigger than non monospaced ones, don't know
about you,
but I normally bring monospaced fonts size down 1 or 2 points to compensate.

Or, you could be referring a font that's italic/scripty in its normal state
(and don't want to slant it further), but as a fallback,
you might want to ask for a Serif italic instead.

Generally speaking, there are reasons why one would want to control size,
weight and slant
on a per fallback font basis, and SE does not allow that.

The change requires to un-deprecate the methods in the interface, fix the
implementation,
and some bits of code that don't have support for multiple fonts (like
DuplicatingStyleVisitor).
The single font method versions would remain as support for SE, and to
support the simple cases
where the extra flexibility is not needed/desired.

Btw, this is part of some work I'm doing to support multiscript labels,
e.g., in some parts of China
the OSM database has road names with a string having latin, chinese and
arab chars (in the
same string), and we'll leverage the ability to support multiple font
families to specify which
fonts to try in order to render the various parts of the string (this is
going to be automatic).
People using SE as the language will get a slightly crippled support, as
they won't be able
to fine tune the different font sizes and whatnot like those that use SLD
1.0

Of course, people using code to setup the styling won't have any problem :-p

As a possible alternative, we could downgrade SLD 1.0 flexibility to SE 1.1
one, keep the
deprecations, but TextSymbolizerImpl would still have to be modified not to
eat away
the extra fonts like it does today (SLDParser sets n fonts, TextSymbolizer
retains only
the first).

CSS wise, there is not support for multiple fonts right now (not even for
the family),
not sure if I'll managed to work on that too, but it's unrelated (the
limitation was
already there) and the module is unsupported anyways.

Cheers
Andrea



-- 
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
darcene notizia via e-mail e di procedere alla distruzione del messaggio
stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso,
divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od
utilizzarlo per finalità diverse, costituisce comportamento contrario ai
principi dettati dal D.Lgs. 196/2003.



The information in this message and/or attachments, is intended solely for
the attention and use of the named addressee(s) and may be confidential or
proprietary in nature or covered by the provisions of privacy act
(Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection
Code).Any use not in accord with its purpose, any disclosure, reproduction,
copying, distribution, or either dissemination, either whole or partial, is
strictly forbidden except previous formal approval of the named
addressee(s). If you are not the intended recipient, please contact
immediately the sender by telephone, fax or e-mail and delete the
information in this message that has been received in error. The sender
does not give any warranty or accept liability as the content, accuracy or
completeness of sent messages and accepts no responsibility  for changes
made after they were sent or for other risks which arise as a result of
e-mail 

Re: [Geotools-devel] Proposal to implement specific methods for Contrast Enhancement

[Daniele Romagnoli]

Hi Jody,
I see your point about StyleVisitor implementations not covered by Abstract
StyleVisitors which may break.
However, as far as I can see from Ian changes on his branch, he has already
made several visitors extending the AbstractStyleVisitor which implements
the new method so that change should be transparent. (Ian, could you check
if all StyleVisitor implementations are properly updated?).

I wonder if that "-1" should be revisited as "+0" provided these details.

I'm a bit against the idea of having parameter names in the
ContrastEnhancement Interface. This will require that whenever a new
algorithm is defined with new parameters, we have to define them in that
interface which in my opinion it shouldn't contain such kind of information
(again... that's only my opinion). They should be part of the specific
ContrastMethod implementation as in this example from Ian's code which
check the parameter names:
https://github.com/ianturton/geotools/blob/contrast/modules/library/main/src/main/java/org/geotools/styling/Exponential.java

Same approach can be used for algorithms.
Please, let us know what do you think about our feedbacks since we have to
complete this work before the end of the week (freeze is incoming). .

Daniele







On Tue, Aug 4, 2015 at 11:50 PM, Jody Garnett 
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 getParameters();
> }
>
> Where ContrastMethod remains a code list:
>
> public final class ContrastMethod extends CodeList {
> 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.
>
>
> --
>
> ___
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
>


-- 
==
GeoServer Professional Services from the experts! Visit
http://goo.gl/it488V for more information.
==

Ing. Daniele Romagnoli
Senior Software Engineer

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:  +39 0584 1660272

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

---

*AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*

Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il
loro utilizzo è consentito esclusivamente al destinatario del messaggio,
per le finalità indicate nel messaggio stesso. Qualora riceviate questo
messaggio

Re: [Geotools-devel] Proposal to implement specific methods for Contrast Enhancement

[Ian Turton]

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  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 getParameters();
> }
>
> Where ContrastMethod remains a code list:
>
> public final class ContrastMethod extends CodeList {
> 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