+1 here too Cheers Andrea
On Mon, Sep 6, 2021 at 7:01 PM Nuno Oliveira < nuno.olive...@geosolutionsgroup.com> wrote: > +1 on the reviewed proposal > > On Mon, Sep 6, 2021 at 4:10 PM Fernando Mino < > fernando.m...@geosolutionsgroup.com> wrote: > >> Thanks Jody for your hints. >> >> Indeed, using rendering hints to pass the pre-process behavior to >> GeoTools makes sense, it helps us to avoid adding a new extension point and >> I added this approach to the proposal. Since we need different behavior >> per-workspace we'll make rendering evaluate the current feature, and make >> GeoServer cache the per-feature-namespace result during the current >> rendering task in progress for performance optimization. >> >> The proposals were updated for your revision and feedback: >> >> https://github.com/geotools/geotools/wiki/Rendering-pre-process-Mark-Factories-Hint >> https://github.com/geoserver/geoserver/wiki/GSIP-204 >> >> >> Regards, >> >> Fernando Mino >> >> == >> >> GeoServer Professional Services from the experts! >> >> Visit http://bit.ly/gs-services-us for more information. >> >> == >> >> Fernando Mino >> >> Software Engineer >> >> @fmy2kec >> >> GeoSolutions Group >> phone: +39 0584 962313 >> >> fax: +39 0584 1660272 >> >> https://www.geosolutionsgroup.com/ >> >> http://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. >> >> >> On Fri, Aug 13, 2021 at 12:30 AM Jody Garnett <jody.garn...@gmail.com> >> wrote: >> >>> Right, sorry I missed that requirement earlier. >>> >>> For the workspace, or GetMap, use case that is a clear need for using a >>> Hint (pass your comparator in as a hint and make a getMarkFactories( hints >>> ) method; it probably already should of had one. >>> >>> Jody >>> >>> On Thu, Aug 12, 2021 at 2:34 PM Fernando Mino < >>> fernando.m...@geosolutionsgroup.com> wrote: >>> >>>> Hi Jody, >>>> >>>> Thanks for the insight on Factory registry as a better alternative. >>>> Indeed the DynamicSymbolFactoryFinder.getMarkFactories() -> >>>> FactoryRegister.setOrdering(..) approach will do the trick for one of the >>>> scenarios we want to support (I will amend the proposal to add this to >>>> support the global configuration use case) but we also have the >>>> per-workspace use case that allows: >>>> - To have several different MarkFactory ordering and filtering on >>>> several workspaces. >>>> - To have several WMS GetMap requests (and rendering threads) in >>>> parallel, rendering Mark shapes with different MarkFactory implementations >>>> order and availability profiles. >>>> >>>> Probably I'm wrong (and please let me know if indeed there is a way to >>>> achieve this with DynamicSymbolFactoryFinder.getMarkFactories() -> >>>> FactoryRegister.setOrdering), but looks like the per-workspace >>>> configuration + multithreading support is not possible with FactoryRegister >>>> due to its not-thread-safe, globally cached and synchronized-static-methods >>>> caller(DynamicSymbolFactoryFinder). >>>> >>>> To use FactoryRegistry we would need to change >>>> DynamicSymbolFactoryFinder to allow us to store and index different >>>> FactoryRegistry instances per workspace, but the problem here is GeoTools >>>> Rendering module is not aware of GeoServer workspaces, and that is why I >>>> proposed the on-the-fly filtering and ordering interceptor extension point, >>>> to allow us to control this filtering from GeoServer code during request >>>> runtime. >>>> >>>> Naturally any better idea to solve this second use case is welcome. >>>> >>>> Regards, >>>> >>>> Fernando Mino >>>> >>>> == >>>> >>>> GeoServer Professional Services from the experts! >>>> >>>> Visit http://bit.ly/gs-services-us for more information. >>>> >>>> == >>>> >>>> Fernando Mino >>>> >>>> Software Engineer >>>> >>>> @fmy2kec >>>> >>>> GeoSolutions Group >>>> phone: +39 0584 962313 >>>> >>>> fax: +39 0584 1660272 >>>> >>>> https://www.geosolutionsgroup.com/ >>>> >>>> http://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. >>>> >>>> >>>> On Wed, Aug 11, 2021 at 8:11 PM Jody Garnett <jody.garn...@gmail.com> >>>> wrote: >>>> >>>>> Thanks for doing this up as a proposal, it really helps with >>>>> discussion. >>>>> >>>>> I did not think that MarkFactory or ExternalGraphicFactory needed to >>>>> be aware of priority as this is handled by FactoryRegistry; but you are >>>>> correct none of the existing implementations extend AbstractFactory. >>>>> >>>>> Your proposal is on MarkFactory; do you need a similar change for >>>>> ExternalGraphicFactory? >>>>> >>>>> I am not quite sure how MarkFactoriesProcessor is intended to work: >>>>> >>>>> - processFactories( Iterator<MarkFactory> factories ): >>>>> Iterator<MarkFactory> >>>>> Processing an iterator on the fly is good, it amounts to doing >>>>> pair wise ordering, or a comparator, which is already supported by >>>>> FactoryRegistry. >>>>> - priority(): int >>>>> What is the int for? Is it to order between competing instances >>>>> of MarkFactoriesProcessor ? >>>>> >>>>> >>>>> My hesitation is seeing more than one way to do things, especially if >>>>> it is a one-off for a specific factory. So I would like to see what the >>>>> limitation you are running into with the existing setup, and if your >>>>> proposal is an improvement we could do it for all FactoryFinders. We can >>>>> look at other factory finders for examples of helper methods used to >>>>> address common challenges (factory iterator providers for injecting >>>>> instances, establishing pairwise ordering >>>>> <https://github.com/geotools/geotools/blob/06d379a2fdbfdbf94641c16813c37ef02aa7f63c/modules/library/referencing/src/main/java/org/geotools/referencing/ReferencingFactoryFinder.java#L546>, >>>>> ...). I know that establishing pairwise ordering is a pain, and there is >>>>> already a helper method that takes a comparator >>>>> <https://github.com/geotools/geotools/blob/main/modules/library/metadata/src/main/java/org/geotools/util/factory/FactoryRegistry.java#L1265> >>>>> to make this easier. >>>>> >>>>> Checking your proposal you reference propose changing the code here >>>>> <https://github.com/geotools/geotools/blob/2c0f22f0a5a13885aa1d85faa6f715ea87f3c90e/modules/library/render/src/main/java/org/geotools/renderer/style/SLDStyleFactory.java#L1362> >>>>> : >>>>> >>>>> *Iterator<MarkFactory> it = >>>>> DynamicSymbolFactoryFinder.getMarkFactories();* >>>>> >>>>> >>>>> I would ask instead that you change DynamicSymbolFactoryFinder >>>>> directly; that interface is already responsible for SPI discovery, >>>>> >>>>> Here is a proposed alternative approach: >>>>> >>>>> 1. Leave StyleFactory alone (it already has too much logic) and >>>>> trust DynamicSymbolFactoryFinder.getMarkFactories() to respect your >>>>> ordering, especially since you are trying to work around how factory >>>>> discovery is based on classpath order. >>>>> >>>>> 2. Create a method DynamicSymbolFactoryFinder setMarkFactoryOrder( >>>>> Comparator<MarkFactory> comparator ): >>>>> >>>>> public static synchronized >>>>> Iterator<MarkFactory> setMarkFactoryOrder( Comparator<MarkFactory> >>>>> comparator ) { >>>>> getServiceRegistry().setOrdering( MarkFactory.class, >>>>> comparator ); >>>>> } >>>>> >>>>> 4. This calls FactoryRegister.setOrdering( category, comparator ) >>>>> <https://github.com/geotools/geotools/blob/main/modules/library/metadata/src/main/java/org/geotools/util/factory/FactoryRegistry.java#L1265> >>>>> which >>>>> should do the trick. >>>>> >>>>> >>>>> -- >>>>> Jody Garnett >>>>> >>>> -- >>> -- >>> Jody Garnett >>> >> _______________________________________________ >> GeoTools-Devel mailing list >> GeoTools-Devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/geotools-devel >> > > > -- > > Regards, > > Nuno Oliveira > > == > GeoServer Professional Services from the experts! > > Visit http://bit.ly/gs-services-us for more information. > == > > Nuno Miguel Carvalho Oliveira > @nmcoliveira > Technical Lead / Project Manager > > > GeoSolutions Group > phone: +39 0584 962313 > fax: +39 0584 1660272 > > https://www.geosolutionsgroup.com/ > http://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. > _______________________________________________ > GeoTools-Devel mailing list > GeoTools-Devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geotools-devel > -- Regards, Andrea Aime == GeoServer Professional Services from the experts! Visit http://bit.ly/gs-services-us for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions Group phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 333 8128928 https://www.geosolutionsgroup.com/ http://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
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
