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
