So what is next? The GeoServer pull request (
https://github.com/geoserver/geoserver/pull/3051) seems nice and minimal?

Happy to start JTS RC when you say go, do not want to hold up the release
train...
--
Jody Garnett


On Thu, 16 Aug 2018 at 03:52, Nuno Oliveira <[email protected]>
wrote:

> Hi all, Jody,
> pull requests updated according to feedback \ last changes.
>
> Thank you !
>
> Nuno Oliveira
>
> On 08/15/2018 01:18 AM, Jody Garnett wrote:
>
> Thanks Nuno, comments/discussion inline:
>
>> First comment, the current API breaks backward compatibility by requiring
>> that a measure should always be provided when building a coordinate
>> sequence, IMHO it should have a fallback method that assumes zero measures
>> if none if provided ... otherwise changes in jaitools will also be required.
>>
>
> The CoordinateSequenceFactory provides both create( dimension ) and
> create( dimension, measure ). Your link [1] is to a specific
> implementation, PackedCoordinateSequence.java
> <https://github.com/locationtech/jts/blob/master/modules/core/src/main/java/org/locationtech/jts/geom/impl/PackedCoordinateSequence.java#L48>,
> it would be wise to use the factory if you can?
>
> When investigating a failing test I step on a visibility change [1] that
>> doesn't allow us to change LiteCoordinateSequence coordinates\dimension in
>> place anymore.
>>
>
> If I understand correctly LiteCoordinateSequence is willing to swap out
> its array on the fly right, and as part of that the dimensions and measures
> used to navigate the array.
>
> This is not a use-case I have inside JTS, but it is one that we can
> support ... will mark the field "protected" rather than "final protected".
>
> Another issues, the following test [3] is failing when invoking the
>> normalize function [4] on polygon:
>>
>> Caused by: java.lang.IllegalArgumentException: Invalid ordinate index: 2
>>     at
>> org.locationtech.jts.geom.CoordinateXY.getOrdinate(CoordinateXY.java:77)
>>     at
>> org.locationtech.jts.geom.impl.CoordinateArraySequence.getOrdinate(CoordinateArraySequence.java:260)
>>     at
>> org.locationtech.jts.geom.CoordinateSequences.swap(CoordinateSequences.java:47)
>>     at
>> org.locationtech.jts.geom.CoordinateSequences.reverse(CoordinateSequences.java:32)
>>     at org.locationtech.jts.geom.Polygon.normalize(Polygon.java:429)
>>     at org.locationtech.jts.geom.Polygon.normalized(Polygon.java:416)
>>     at org.locationtech.jts.geom.Polygon.normalize(Polygon.java:374)
>>     at
>> it.geosolutions.jaiext.vectorbin.ROIGeometry.<init>(ROIGeometry.java:269)
>>     at
>> it.geosolutions.jaiext.vectorbin.ROIGeometry.<init>(ROIGeometry.java:195)
>>     at
>> it.geosolutions.jaiext.vectorbin.ROIGeometry.<init>(ROIGeometry.java:150)
>>     at
>> org.geotools.coverage.grid.io.footprint.MultiLevelROIGeometry$FastClipROIGeometry.<init>(MultiLevelROIGeometry.java:162)
>>     at
>> org.geotools.coverage.grid.io.footprint.MultiLevelROIGeometry.getTransformedROI(MultiLevelROIGeometry.java:128)
>>     at
>> org.geotools.coverage.grid.io.footprint.MultiLevelROIGeometry.getTransformedROI(MultiLevelROIGeometry.java:45)
>>     at
>> org.geotools.gce.imagemosaic.GranuleDescriptor.loadRaster(GranuleDescriptor.java:1348)
>>     at
>> org.geotools.gce.imagemosaic.GranuleLoader.call(GranuleLoader.java:108)
>>     ... 44 more
>>
>> I'm not familiar with JTS code, so I'm clueless in find out what may be
>> causing this :(
>>
>
> This is the kind of feedback I needed thank you. Looking at the swap
> method it seems fine:
>
>   public static void swap(CoordinateSequence seq, int i, int j)
>   {
>     if (i == j) return;
>     for (int dim = 0; dim < seq.getDimension(); dim++) {
>       double tmp = seq.getOrdinate(i, dim);
>       seq.setOrdinate(i, dim, seq.getOrdinate(j, dim));
>       seq.setOrdinate(j, dim, tmp);
>     }
>   }
>
> Something has gotten in consistent, the dimension of the coordinate
> sequence must have changed from 2 (producing a cache of CoordinateXY) to 3
> (producing a reference to getOrdinate( x, 2 ).
>
> I will try and reproduce the failure locally...
>
>
> Last finding, the previous coordinate object, even if only a dimension of
>> 2 was provided, was always allowing to access or set a Z value, this is not
>> the case anymore in the new API [5].
>> I agree that the behavior of the new API is the correct behavior, but
>> unfortunately it breaks existing expectations. The issue here is that again
>> this will prevent us to change geometries in place, which means that some
>> not so trivial re-factor will need to be done in a few places.
>>
>
> Let's figure out what is needed so you can change geometries in place and
> avoid a refactor unless there is a benefit.
>
> This are the issues I found when using the new JTS API, all of the found
>> issues (at exception the polygon normalize) are the consequence of the new
>> JTS API breaking backward compatibility.
>>
>
> I am going to make an issue for the polygon normalize now (
> https://github.com/locationtech/jts/issues/296) and pull request (
> https://github.com/locationtech/jts/pull/297).
>
>
> --
> Regards,
> Nuno Oliveira
> ==
> GeoServer Professional Services from the experts!
> Visit http://goo.gl/it488V for more information.
> ==
>
> Nuno Miguel Carvalho Oliveira
> @nmcoliveira
> Software Engineer
>
> GeoSolutions S.A.S.
> Via di Montramito 3/A
> 55054  Massarosa (LU)
> Italy
> phone: +39 0584 962313
> fax:      +39 0584 1660272
> http://www.geo-solutions.ithttp://twitter.com/geosolutions_it
>
> -------------------------------------------------------
>
> Con riferimento alla normativa sul trattamento dei dati
> personali (Reg. UE 2016/679 - Regolamento generale sulla
> protezione dei dati “GDPR”), si precisa che ogni
> circostanza inerente alla presente email (il suo contenuto,
> gli eventuali allegati, etc.) è un dato la cui conoscenza
> è riservata al/i solo/i destinatario/i indicati dallo
> scrivente. Se il messaggio Le è giunto per errore, è
> tenuta/o a cancellarlo, ogni altra operazione è illecita.
> Le sarei comunque grato se potesse darmene notizia.
>
> This email is intended only for the person or entity to
> which it is addressed and may contain information that
> is privileged, confidential or otherwise protected from
> disclosure. We remind that - as provided by European
> Regulation 2016/679 “GDPR” - copying, dissemination or
> use of this e-mail or the information herein by anyone
> other than the intended recipient is prohibited. If you
> have received this email by mistake, please notify
> us immediately by telephone or e-mail.
>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to