On Fri, May 27, 2011 at 5:02 PM, Justin Deoliveira <jdeol...@opengeo.org> wrote: > Hi all, > A while back we had a client who was interested in funding some labelling > work to solve a problem they were having with labels being drawn over > external graphics. As an example: > https://skitch.com/jdeolive/fbiss/label-before > Andrea had the idea of introducing the concept of labelling obstacles. More > specifically marking a symbolizer as a "label obstacle" via a vendor option. > And when present such symbolizers would be factored into the labelling > routine. Basically it would mark the area taken by the symbolizer as "busy" > so that no label would be drawn over top of it. > I have taken a crack at implementing this. The initial results look > promising: > https://skitch.com/jdeolive/fbish/label-after > The current patch can be found on this jira issue: > http://jira.codehaus.org/browse/GEOT-3610 > > The patch is still rough at this point, but I wanted to post it to > supplement some of my questions: > 1. I had to utilize the deprecated constructor on StyledShapePainter that > accepts the LabelCache as a parameter, however that constructor has been > deprecated. Is that because rendering is now multi threaded and that the > painting actually goes on in a separate thread? So access to the label cache > should be limited to the primary/data loading thread?
It because the painter had no use for the label cache until now, so that one was a dead method. > I am trying to analyze the rendering code to figure out if actually any race > conditions are introduced. And if I understand correctly I don't think there > are but I could definitely be missing something. With the new code the only > time the painting thread has to update the label cache is to put a new > reserved rectangle into the cache... which simply updates the reserved list. > And as far as I can tell this value is not accessed by the primary thread > until after it has joined with the paint thread... Yep, you're right, in practice what you're doing is not going to cause a thread safety issue. uDig is hitting the label cache from multiple threads in parallel, but afaik they are talking to a synchronized wrapper of it. > Anyways, some guidance here on how best to proceed would be appreciated. > 2. The patch adds support for marking a Line and Poly symbolizer as a label > obstacle. Not sure if this is a great idea since the method is bounding box > based hence we have to reserve the entire bounding box of lines and > polygons. Which you can imagine can lead to (depending on the shape of a > geometry) large spaces with no labels. So I am wondering if label obstacles > should be limited to point symbolizers? Eh, good question, not sure how to answer this one... for polygons it may be acceptable or not, depends a lot on the polygons, for lines indeed it's going to cause some grief... I don't have a good answer at the moment, the best way would be to rethink the collision algorithm to use a raster based approach instead of a spatial index, but it would also take some time to implement. In this approach the put(Rectangle) method would be replaced by a put(Shape, float strokeThickness, boolean fill) method that would paint on a black and white "busy area" surface as big as the map we're drawing. > That is about it for now... general feedback appreciated... but be gentle... > this is my first rendering patch :) I know it's just a rough patch, yet let me also point out minor things I've seen while looking at the code: - text symbolizer vendor option map just become redundant, it can be removed - vendorOptionString in SLDParser should be capitalized (yeah, I know there are other constant there not following the proper naming convention, they should be fixed as well) - painting with graphic strokes with the conflict option turned on will add a lot of entries to the conflict map... I may be worrying too much, yet with small symbols we easily end up adding several thousands of entries to the conflict map... - at the moment there are no tests. Now, I know the testing situation in renderer land is scary, however the new ImageAssert facility should make it possible to add a test without too much effort, just use a built in font like Dialog and allow for pixel difference in the assert to account for platform dependencies. Cheers Andrea -- ------------------------------------------------------- Ing. Andrea Aime GeoSolutions S.A.S. Tech lead Via Poggio alle Viti 1187 55054 Massarosa (LU) Italy phone: +39 0584 962313 fax: +39 0584 962313 http://www.geo-solutions.it http://geo-solutions.blogspot.com/ http://www.youtube.com/user/GeoSolutionsIT http://www.linkedin.com/in/andreaaime http://twitter.com/geowolf ------------------------------------------------------- ------------------------------------------------------------------------------ vRanger cuts backup time in half-while increasing security. With the market-leading solution for virtual backup and recovery, you get blazing-fast, flexible, and affordable data protection. Download your free trial now. http://p.sf.net/sfu/quest-d2dcopy1 _______________________________________________ Geotools-devel mailing list Geotools-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
