Skip to content
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

Add more/better scaling methods to RasterSymbolizer #656

Closed
artemp opened this issue Oct 11, 2011 · 11 comments
Closed

Add more/better scaling methods to RasterSymbolizer #656

artemp opened this issue Oct 11, 2011 · 11 comments
Milestone

Comments

@artemp
Copy link
Member

artemp commented Oct 11, 2011

At present RasterSymbolizer only supports bilinear/bilinear8 scaling.

As mentioned in [http://trac.osgeo.org/gdal/ticket/3740 this gdal ticket] and the associated discussion on [http://article.gmane.org/gmane.comp.gis.gdal.devel/24723 gdal-dev], this tends to make map tiles look terrible for rasters when heavily-zoomed-out.

Some sample images are available on the gdal ticket; they look the same when produced via mapnik.

Until now I assumed that was because mapnik was just using gdal to render the raster symbolizer, but looking at the code it seems that's not the case.

Perhaps we could add cubic-spline or lanczos resampling in image_util.hpp, and expose it as a scaling option for RasterSymbolizer.

Happy to write a patch if it's likely to be accepted.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] Craig, absolutely. Higher quality resampling would be great. Artem has mentioned he would ideally like to leverage AGG as much as possible in this regard instead of rolling our own, like has been done so far in image_util.hpp.

So, my sense is that the ideal steps would be:

  1. Ensure we can hook into the AGG algorithms, which appear to be here: http://trac.mapnik.org/browser/trunk/agg/include/agg_image_filters.h#L337

  2. Benchmark AGG's bilinear resampling vs. the existing implementing in image_util.hpp. Assuming AGG is not significantly slower then:

  3. Add AGG lanczos/spline, etc

  4. consider replacing existing bilinear impl after checking in with Marcin

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] BTW, for examples of each algorithm available in AGG see 'Image_filters2' from:

http://www.crossgl.com/aggpas/aggpas-demo.htm

Source code here:

http://www.antigrain.com/demo/image_filters2.cpp.html

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] In addition, it seems alpha is handled incorrectly in mapnik's implementation of bilinear scaling (correct me if I'm wrong)

The weight for each pixel in bilinear interpolation should be multiplied by the alpha value for that pixel. That is, transparent pixels shouldn't contribute to the resultant pixel value.

Hopefully the AGG implementation handles this...

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] one more potential lead, the code here: http://trac.mapnik.org/browser/trunk/bindings/python/mapnik_image.cpp#L131
shows the simple conversion between mapnik::image_32 and agg rendering buffer, as I assume you'll need things in an agg buffer to do the scaling. let me know what you come up with.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] I've finally got this working how I expect. Here's [https://github.com/craigds/mapnik-trunk/compare/6797752a53282e7414c336cfb0ad8ab3...master the diff].

Some notes:

  • This diff includes fixes for the bits of AGG's resampling and compositing code that we're using. However I haven't implemented the fixes for all the different (similar) methods in AGG (that we're not using, as far as I can tell):
    • I fixed compositing in blender_rgba() but didn't test or fix blender_rgba_pre() or blender_rgba_plain()
    • I fixed resampling in span_image_resample_rgba_affine() but didn't test or fix span_image_resample_rgba()
  • This includes my fix for set_rectangle_alpha2() doesn't blend alpha correctly #674 , which is a separate bug but it's included here since it's a prerequisite.
    • Without the fix for set_rectangle_alpha2() doesn't blend alpha correctly #674, any semi-transparent regions in resampled images are much darker than they should be (their color values are pre-multiplied by their alpha.) This causes black shadows in the output, around the edges of reprojected source images.
  • Surprisingly, with filter_factor==2 I don't see any slowdown between bilinear_agg and bilinear. The output looks slightly improved.
  • Comparing with filter_factor >2 the new implementation is quite a lot slower (filter_factor==4 ~ 50% slowdown) and looks much nicer. But that's a useless comparison because the old implementation just skips lots of pixels in that case.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] All the changes until the most recent commit can be compared with previous behaviour using scaling=='bilinear' vs scaling=='bilinear_agg'.

In the latest commit I got rid of the old bilinear and replaced it with the AGG implementation.

Dane to review and point out all the nasties I have to clean up :)

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] Latest diff is [https://github.com/craigds/mapnik-trunk/compare/3ef64647c26f26ab2f5c214b7746be697cb8ded7...master here].

This includes a fix for incorrect (rounded, instead of expanded) subpixel offsets (see changes for cairo_renderer.cpp and agg/process_raster_symbolizer.cpp).

Because of this bug I had to either fix or remove scale_image() to use the correct scale level (since I changed target image dimensions, you can't scale to the entire target image anymore.) I chose to remove it since there's already a nearest-neighbour routine in AGG and we don't really need two. It seems about the same speed.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] [https://github.com/craigds/mapnik-trunk/compare/ae87eea152ec6ab90fba17b63d161089...master latest] - fixed compilation errors with clang++, thanks springmeyer.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] not ignoring this, in fact quite excited to dig in and get this applied, just been busy. I'm on the hook for reviewing and its on the list.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] Applied in r2543! Great work Craig. Seriously impressive. Only notable modification I made was to move the agg include you used in include/mapnik/image_util.hpp to the cpp file to hide it from the compiler so that c++ apps using image_util.hpp do not need the agg headers available.

Next steps on this work I think are refactoring. We should move to using mapnik::enumerators I think (before release) for different scaling methods/mode/etc which will clean up the code a bit.

Also before release I think it would be a good idea to comment out the various agg scaling methods that we don't know a lot about (yet). New users will want to know which is the fastest and the best and likely will not want to try them all. So, should we expose nearest, bilinear, and lanczos (until we know more)? What do you think?

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[cdestigter] No objections to commenting out the other methods; I'd maybe expose bicubic as well since it's quite well known and it's supported in some of the GDAL utilities.

Great work - Mapnik 2.0 is shaping up to be uber awesome :)

@artemp artemp closed this as completed Oct 11, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant