-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
way_area conditions for all ST_PointOnSurface layers #4042
Conversation
I am strictly against introducing a lower (starting) way_area limit for features where there is no visual feedback on the geometry - in this case place=square. Regarding changing In the single case of the |
I am not introducing it - it already exists: Lines 3307 to 3309 in e7de1cd
For the other limits, I believe it is important that the feature selection and queries are the same independent of scale factor. If you want to get rid of the limit, that belongs in a different PR. |
Ah, sorry. See #4043.
Well - as explained the reasoning behind these limits is not independent of the scale factor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I support most of these changes, but would like to keep using pixel_width/pixel_height for the 1 pixel and sub-pixel rendering layers, where the limit is supposed to represent polygons which are too small to affect the final rendering. This depends on rendering resolution, so scale_denominator would not be the best idea.
!pixel_height! and !pixel_width! do not work for area-based filtering when using high dpi rendering. Instead, !scale_denominator! needs to be used.
We don't want to display points for giant polygons, both because it's bad cartographically and because it hurts performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks fine, though I haven't had a chance to do a test rendering with the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test renderings are fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not specifically tested all the thresholds but in general the code works fine.
Fixes #4030
I used 768000 for POIs, which is about 12 tiles, and 4000000, which is about 64 tiles for cases like countries where we want to show larger objects.