Modify

Opened 12 years ago

Closed 7 years ago

Last modified 3 years ago

#7427 closed enhancement (fixed)

Support reprojection (warping) of imagery layer

Reported by: bastiK Owned by: bastiK
Priority: major Milestone: 17.04
Component: Core imagery Version:
Keywords: projection warping Cc: A_Pirard, wiktorn

Description

It would be nice, if we could reproject tiles from a WMS/TMS layer (and other raster data) to the current projection, so that sources with different projections can be combined.

Attachments (7)

50.png (642.1 KB ) - added by A_Pirard 12 years ago.
Displaying Bing under Lambert screen
europe-northafrica_utm35.png (228.9 KB ) - added by bastiK 7 years ago.
reference.png (10.7 KB ) - added by bastiK 7 years ago.
test-output.png (10.6 KB ) - added by bastiK 7 years ago.
test-differences.png (1.4 KB ) - added by bastiK 7 years ago.
reprojection.patch (59.4 KB ) - added by bastiK 7 years ago.
DimmingTiles.png (1.0 MB ) - added by Klumbumbus 7 years ago.

Download all attachments as: .zip

Change History (132)

comment:1 by bastiK, 12 years ago

This should be possible with the GeoTools library. It is currently implemented by the ImportImage (JOSM-)plugin. In the plugin, the JAI lib is used, as well. However, as far as I can tell, JAI licenses aren't GPL compatible. According to this thread, it seams, that JAI is not a hard dependency for image warping with GeoTools, but only needed for very large images.

In any case, this would need to go into a plugin as GeoTools is very bloated.

comment:2 by joshdoe, 12 years ago

Cc: joshdoe added

comment:3 by stoecker, 12 years ago

Sounds like Piclayer and importimage should be joined and this functionality included there?

comment:4 by A_Pirard, 12 years ago

Cc: A_Pirard added; joshdoe removed

comment:5 by A_Pirard, 12 years ago

Cc: joshdoe added

comment:6 by A_Pirard, 12 years ago

This Ticket #7427 says in internals terms what ticket Ticket #5387 says in practical terms.
The subject of Ticket #5387 has been changed to report EPSG:3170 deficiencies.
Thus, my description for it no longer makes sense.
Why not have continued Ticket #5387 and open a new ticket for 3170?

comment:7 by A_Pirard, 12 years ago

When the screen projection is set to EPSG:31370 and a Bing layer is opened, JOSM warns that Lambert 31370 is not supported by Bing. However, the layer is displayed and is almost usable. Just that small slits where the background shows through appear between tiles (see 50.png attachment). It looks like a simple vertical stretching (and maybe horizontal shrinking?) would be a very satisfactory solution for that map.
It sounds like, although displaying Lambert, JOSM is sending the correct WGS84 coordinates to Bing which returns the correct tiles but needing a small warp.

And now what if the screen were set to Mercator and we forced JOSM to translate the WGS84 coordinates to the projection of the &SRS parameter before sending a request? Would the display look about the same? Just that without the warping (yet) would be a great event.

I tried our Lambert servers on Mkt. JOSM is sooo more pleasurable to work with !!!

by A_Pirard, 12 years ago

Attachment: 50.png added

Displaying Bing under Lambert screen

comment:8 by skyper, 12 years ago

Priority: normalmajor

+10

This would solve many problems including all tools which depend on projection and only work with "mercator" right now.

comment:9 by A_Pirard, 11 years ago

Would there be any news here?
I just explained a major OSM mapper that this Ticket is the reason why he can't use the server he wants.
How many mappers do turn away?
Thanks for all the good work.

in reply to:  9 ; comment:10 by bastiK, 11 years ago

Replying to A_Pirard:

Would there be any news here?
I just explained a major OSM mapper that this Ticket is the reason why he can't use the server he wants.

Care to elaborate? You can always switch to the server projection, this feature is just a nice-to-have when working with multiple sources of different projections.

comment:11 by A_Pirard, 11 years ago

This feature is not "nice to have" but necessary when working with several servers using different projections.
This was the reason for opening #5387 which was set to fixed when it is not.
I was repeatedly reading that javascript is too slow to reproject when I stumbled upon this
http://dev.openlayers.org/sandbox/camptocamp/canvas/openlayers/examples/raster-reprojection.html
So, I wondered if anything could be new in the abovURL. Just guessing.
User confusion probably stems from reading "the projection" when there is the projection used in server requests, that of the data returned, that of coordinates in source codes, displayed on screen, input from the keyboard and, maybe most unexpected, that of the screen. Explanations are often understood only by those who have already understood them. For example, a lot of Open Overlays code speaks of converting in to out projections without saying what they're for.

in reply to:  10 ; comment:12 by A_Pirard, 11 years ago

Replying to bastiK:

this feature is just a nice-to-have ... different projections

"just" only if you're fast enough to switch between two+ different projection images so that they appear to overlay...
The whole France is badly longing for reprojection, Belgium too, ...?
The French are allowed to use the government's data. Well, almost...

FYI: OSM-talk-fr: Cadastre et reprojection des images
PVI: Same text in FrEnGooglish

C'est déjà galère de devoir changer de projection tout le temps
(mercator pour les images satellite, lambert4 si le cadastre est raster,
lambert9 pour avoir le patelin d'à côté déjà vectorisé) ;

À ce propos ce serait vraiment fantastique si JOSM pouvait automatiquement
reprojeter vers une même projection commune. Merkaartor fait ça et c'est
absolument fantastique pour afficher simultanément Bing et le cadastre en semi-
transparence. C'est vraiment *la* chose qui me manque dans JOSM.

My short translation:
It's a PITA to have to constantly change projection: Mercator, Lambert4, lambert9, [Belgian Lambert72]...
It would truly be fantastic if JOSM could reproject...
Mercator does that and it's absolutely fantastic to display semi-transparently ...
It's really THE missing feature in JOSM.

Is that "elaborating" enough?
Thanks for JOSM !!!

in reply to:  12 comment:13 by bastiK, 11 years ago

Replying to A_Pirard:

Is that "elaborating" enough?

Oui merci, ça suffit.

Still, someone has to take the time and implement it. :)

comment:14 by bastiK, 7 years ago

Cc: wiktorn added
Owner: changed from team to bastiK
Status: newassigned

I've got a quick-and-dirty patch to support warping for TMS. Generally, I wouldn't mind starting with a plugin, but it requires so many changes to the core code, that it makes not much sense.

As for an image library, that can deform images according to a custom mathematical formula: I couldn't find any. Geotools warping indeed depends on JAI, namely javax.media.jai.Warp, so nothing we can use. An implementation from scratch shouldn't be too hard.

It also affects the cache, as warping is too slow, to do it on the fly with each paint cycle. The reprojected images are placed in the MemoryTileCache. The original downloaded images are written to disk by the JCS cache, then dropped from memory. The download dialog still displays the tiles in Mercator, so we need a second MemoryTileCache instance for the second projection in use.

in reply to:  14 comment:15 by wiktorn, 7 years ago

Replying to bastiK:

The download dialog still displays the tiles in Mercator, so we need a second MemoryTileCache instance for the second projection in use.

We use separate MemoryTileCache instances per all layers. See the comment:
https://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/gui/layer/AbstractTileSourceLayer.java#L151

So the download box and imagery layer have already separate instances.

comment:16 by bastiK, 7 years ago

Thanks for the hint, this makes it much simpler.

in reply to:  14 comment:17 by Don-vip, 7 years ago

Replying to bastiK:

I've got a quick-and-dirty patch to support warping for TMS.

Wow, great !

As for an image library, that can deform images according to a custom mathematical formula: I couldn't find any. Geotools warping indeed depends on JAI, namely javax.media.jai.Warp, so nothing we can use. An implementation from scratch shouldn't be too hard.

If I understand correctly, your current patch relies on GeoTools but it should be doable to get rid of it?

comment:18 by bastiK, 7 years ago

It already uses a very simple custom warp algorithm. The quality is so-so, but it should improve with bilinear or bicubic interpolation.

comment:19 by Don-vip, 7 years ago

OK. Did you look at jt-warp? It's licensed under Apache 2.0.

comment:20 by bastiK, 7 years ago

No, I haven't so far. It seems JAI-ext has Oracle JAI as a dependency (see imports). I'll have a look how deep this dependency is and if there is code that can be salvaged.

comment:21 by bastiK, 7 years ago

@wiktorn: I couldn't find any code that calls the method TMSCachedTileLoaderJob#getCachedTile(), can it be removed?

in reply to:  21 comment:22 by wiktorn, 7 years ago

Replying to bastiK:

@wiktorn: I couldn't find any code that calls the method TMSCachedTileLoaderJob#getCachedTile(), can it be removed?

I guess so. Looks like it got obsoleted?

Looks like src.org.openstreetmap.gui.jmapviewer.interfaces.TileJob.getTile() could also be removed. This will entail update of some plugins though.

comment:23 by Don-vip, 7 years ago

Component: CoreCore imagery
Milestone: 17.04

comment:24 by bastiK, 7 years ago

Okay, I'll keep that in mind.

by bastiK, 7 years ago

comment:25 by bastiK, 7 years ago

Another edition of fun with projections:

Mercator Carto tiles reprojected to UTM 35 + rotated 8 degrees. (Notice how on the standard Mercator map, the Kola peninsula is as big as Turkey? It isn't, really...)

comment:26 by bastiK, 7 years ago

In the imagery preferences, we have a setting Fade Color and Fade amount. Is this still a useful feature or is it obsoleted by per layer opacity setting? I'm asking because the current Fade implementation doesn't work with warped tiles and would need to be adapted a bit.

comment:27 by stoecker, 7 years ago

I'd drop it. Two options for the same feature are usually not that helpful at all.

comment:28 by bastiK, 7 years ago

In 11785/josm:

Imagery: remove fade setting (superseded by layer opacity), see #7427

comment:29 by stoecker, 7 years ago

We can also drop the 4326/3857 workaround when we have reprojection...

in reply to:  29 ; comment:30 by Don-vip, 7 years ago

Replying to stoecker:

the 4326/3857 workaround

What's this?

comment:31 by Don-vip, 7 years ago

In 11786/josm:

see #7427 - checkstyle

in reply to:  30 comment:32 by stoecker, 7 years ago

Replying to Don-vip:

Replying to stoecker:

the 4326/3857 workaround

What's this?

Maps entry epsg4326to3857Supported.

in reply to:  30 comment:33 by wiktorn, 7 years ago

Replying to Don-vip:

Replying to stoecker:

the 4326/3857 workaround

What's this?

Using WMS servers to rescale tiles from 4326 to Mercator. I'll happily remove this logic.

comment:34 by bastiK, 7 years ago

In 11788/josm:

make it possible to switch between 2 supported projections for a given WMTS layer (see #7427)

comment:35 by bastiK, 7 years ago

In 11790/josm:

WMTS: fix case with multiple tilesets per projection and layer (regression from [11788]), see #7427, see #10623)

comment:36 by Don-vip, 7 years ago

In 11806/josm:

see #7427 - checkstyle

comment:37 by bastiK, 7 years ago

We have 2 fairly elaborate algorithms to display tiles from other zoom levels when the current tile is still loading or has an error:

  • Tile.loadPlaceholderFromCache
  • AbstractTileSourceLayer.drawInViewArea (loop over otherZooms)

As far as I can tell, both do essentially the same thing. Is there a reason it is done this way?

(Btw., Tile.loadPlaceholderFromCache uses tile index arithmetic that probably doesn't work for general WMTS tile matrix set.)

comment:38 by bastiK, 7 years ago

In 11829/josm:

see #7427 - move TileRange class to jmapviewer

comment:39 by bastiK, 7 years ago

In [o33206] - [jmapviewer] move TileRange? class to jmapviewer
In [o33207] - see ​#7427 - add interface and class for projected coordinates; add several helper functions to TileSource interface

comment:40 by bastiK, 7 years ago

In 11830/josm:

see #7427 - add implementations for new TileSource methods

comment:41 by bastiK, 7 years ago

In 11831/josm:

see #7427 - use integer operations on tile index when possible (instead of EastNorth or LatLon rectangles)

comment:42 by bastiK, 7 years ago

In [o33208] - [jmapviewer] see ​#7427 - add copy constructor

comment:43 by bastiK, 7 years ago

In 11832/josm:

see #7427 - remove the tile.loadPlaceholderFromCache mechanism from AbstractTileSourceLayer

There are 2 systems to display replacement tiles from other zoom levels, when the current
tile is not available (yet). This commit turns off one of these systems and relies fully on
the code in AbstractTileSourceLayer.drawInViewArea / paintTileImages.

in reply to:  43 ; comment:44 by wiktorn, 7 years ago

Replying to bastiK:

In 11832/josm:

see #7427 - remove the tile.loadPlaceholderFromCache mechanism from AbstractTileSourceLayer

There are 2 systems to display replacement tiles from other zoom levels, when the current
tile is not available (yet). This commit turns off one of these systems and relies fully on
the code in AbstractTileSourceLayer.drawInViewArea / paintTileImages.

Are there no performance drawbacks (esp. with partially loaded imageries, or on places, where max zoom differs for example in Bing)?

loadPlaceholderFromCache only checks in tileCache (memory, with BufferedImages).

The other thing that drawInViewArea / paintTileImages does not do is to create tile images based on data from different zoom level.

in reply to:  44 comment:45 by bastiK, 7 years ago

Replying to wiktorn:

Replying to bastiK:

In 11832/josm:

Are there no performance drawbacks (esp. with partially loaded imageries, or on places, where max zoom differs for example in Bing)?

Can you explain in more detail? There are no significant performance drawbacks, that I'm aware of. What do you mean by max zoom differs?

loadPlaceholderFromCache only checks in tileCache (memory, with BufferedImages).

In drawInViewArea, below the line ts.loadAllTiles(false);, it does exactly the same, i.e. check tileCache for images of different zoom level (no new jobs submitted).

The other thing that drawInViewArea / paintTileImages does not do is to create tile images based on data from different zoom level.

The tiles from different zoom levels are drawn on the fly, which should be about as fast.

comment:46 by wiktorn, 7 years ago

I'm worried about missing this logic

                /*
                 * use LazyTask for graphics to avoid evaluation of tmpImage, until we have
                 * something to draw
                 */
                CachedCallable<Graphics2D> graphics = new CachedCallable<>(new Callable<Graphics2D>() {
                    @Override
                    public Graphics2D call() throws Exception {
                        Graphics2D g = (Graphics2D) tmpImage.call().getGraphics();
                        g.setTransform(AffineTransform.getScaleInstance(scale, scale));
                        return g;
                    }
                });

                int paintedTileCount = 0;
                for (int x = 0; x < factor; x++) {
                    for (int y = 0; y < factor; y++) {
                        Tile tile = cache.getTile(source, xtileHigh + x, ytileHigh + y, zoomHigh);
                        if (tile != null && tile.isLoaded()) {
                            paintedTileCount++;
                            tile.paint(graphics.call(), x * source.getTileSize(), y * source.getTileSize());
                        }
                    }
                }
                if (paintedTileCount == factor * factor) {
                    image = tmpImage.call();
                    return;
                }
            }

            int zoomLow = zoom - zoomDiff;
            if (zoomLow >= JMapViewer.MIN_ZOOM) {
                int xtileLow = xtile >> zoomDiff;
                int ytileLow = ytile >> zoomDiff;
                final int factor = 1 << zoomDiff;
                final double scale = factor;
                CachedCallable<Graphics2D> graphics = new CachedCallable<>(new Callable<Graphics2D>() {
                    @Override
                    public Graphics2D call() throws Exception {
                        Graphics2D g = (Graphics2D) tmpImage.call().getGraphics();
                        AffineTransform at = new AffineTransform();
                        int translateX = (xtile % factor) * source.getTileSize();
                        int translateY = (ytile % factor) * source.getTileSize();
                        at.setTransform(scale, 0, 0, scale, -translateX, -translateY);
                        g.setTransform(at);
                        return g;
                    }

                });

                Tile tile = cache.getTile(source, xtileLow, ytileLow, zoomLow);
                if (tile != null && tile.isLoaded()) {
                    tile.paint(graphics.call(), 0, 0);
                    image = tmpImage.call();
                    return;
                }
            }

Check for example area like this:
https://www.openstreetmap.org/#map=19/53.969564628111144/21.554929044008773

Goto from zoom 19 to zoom 12 and compare both versions. This is the place, that I guess, that we are loading tiles from different zoom levels.

in reply to:  46 comment:47 by bastiK, 7 years ago

Replying to wiktorn:

I'm worried about missing this logic
[...]

From my understanding, the code in drawInViewArea / paintTileImages reproduces this logic (in a slightly different way but with same outcome).

Check for example area like this:
https://www.openstreetmap.org/#map=19/53.969564628111144/21.554929044008773

Goto from zoom 19 to zoom 12 and compare both versions. This is the place, that I guess, that we are loading tiles from different zoom levels.

I did, but don't know what to look out for. In both version, it shows the loaded tiles from level 19 in the center temporarily, then downloads and displays tiles from the current level.

comment:48 by bastiK, 7 years ago

In 11834/josm:

see #7427 - change priorities for temporary replacements from other zoom levels: prefer higher zoom over lower

comment:49 by bastiK, 7 years ago

In 11835/josm:

see #7427 - alignment to the pixelgrid for precise tile placement

comment:50 by joshdoe, 7 years ago

Cc: joshdoe removed

comment:51 by bastiK, 7 years ago

In 11838/josm:

see #7427 - fix unit tests

by bastiK, 7 years ago

Attachment: reference.png added

by bastiK, 7 years ago

Attachment: test-output.png added

by bastiK, 7 years ago

Attachment: test-differences.png added

comment:52 by bastiK, 7 years ago

For future reference, as result of [11835], the viewport in MapCSSRendererTest moved by about 0.3 pixel in both x and y:

previous: now: difference:

comment:53 by bastiK, 7 years ago

wiktorn, if there is something wrong with [11832] I would appreciate your help to understand what is the problem (and fix it). Right now, I'm still convinced it is fine and will proceed.

in reply to:  53 comment:54 by wiktorn, 7 years ago

Replying to bastiK:

wiktorn, if there is something wrong with [11832] I would appreciate your help to understand what is the problem (and fix it). Right now, I'm still convinced it is fine and will proceed.

Please continue. You've convinced me that this is ok. Happy to see that code gone away :-)

I'll take [11839] and give it some testing.

Last edited 7 years ago by wiktorn (previous) (diff)

comment:55 by bastiK, 7 years ago

In 11840/josm:

see #7427 - small fix for [11835] + change order in zoomNoUndoTo

comment:56 by bastiK, 7 years ago

In [o33209] - [jmapviewer] see ​#7427 - adapt the Tile class a bit, so it can store reprojected tiles.

comment:57 by bastiK, 7 years ago

In 11841/josm:

see #7427 - rework the way screen pixel coordinates for tiles are calculated

comment:58 by bastiK, 7 years ago

In 11842/josm:

see #7427 - clean up private method parameter list

comment:59 by bastiK, 7 years ago

In [o33210] - [jmapviewer] see ​#7427 - remove unnecessary API

comment:60 by bastiK, 7 years ago

In 11843/josm:

see #7427 - remove unused code

comment:61 by bastiK, 7 years ago

I was a bit reluctant to delete TMSCachedTileLoaderJob#getCachedTile(), as loading a tile from disk cache (without http request) seems like a useful functionality. But if it's not used, it has no place in the codebase, I guess.

Most API breaking changes should be done now.

comment:62 by bastiK, 7 years ago

In 11844/josm:

see #7427 - avoid roundtrip conversion EastNorth -> LatLon -> EastNorth

This circumvents LatLon clamping, so may have an impact on display near lon=+/-180 degrees

comment:63 by bastiK, 7 years ago

In 11845/josm:

see #7427 - better return type

in reply to:  61 comment:64 by wiktorn, 7 years ago

Replying to bastiK:

I was a bit reluctant to delete TMSCachedTileLoaderJob#getCachedTile(), as loading a tile from disk cache (without http request) seems like a useful functionality. But if it's not used, it has no place in the codebase, I guess.

Most API breaking changes should be done now.

I guess this method is from time when I tried to remove TileCache and rely only on JCS Cache but I had to abandon this approach due to performance reasons.

comment:65 by bastiK, 7 years ago

In [o33211] - [jmapviewer] class TileAnchor not needed in jmapviewer -> move to JOSM

comment:66 by bastiK, 7 years ago

In 11846/josm:

see #7427 - move class TileAnchor from jmapviewer to JOSM

by bastiK, 7 years ago

Attachment: reprojection.patch added

comment:67 by bastiK, 7 years ago

Attached patch adds reprojection support for TMS, WMS + WMTS.

comment:68 by bastiK, 7 years ago

In [o33219] - [jmapviewer] see ​#7427 - remove 'final' + extend toString()

comment:69 by bastiK, 7 years ago

Resolution: fixed
Status: assignedclosed

In 11858/josm:

fixed #7427 - Support reprojection (warping) of imagery layer

comment:70 by bastiK, 7 years ago

Please test and report any problems!

comment:71 by bastiK, 7 years ago

In 11859/josm:

see #7427 - fix test-compile

in reply to:  70 comment:72 by Don-vip, 7 years ago

Replying to bastiK:

Please test and report any problems!

Can you please add Javadoc for ImageryLayer.getNativeProjections()? It's now missing for some plugins but I don't know if it's mandatory, optional, null-safe, etc.

comment:73 by Don-vip, 7 years ago

In 11860/josm:

see #7427 - checkstyle

comment:74 by Don-vip, 7 years ago

15 tests are failing:

 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testParserForMissingStyleIdentifier	0.82 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testWIEN	0.69 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testGeoportalORTOPL2180	0.73 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testGeoportalORTOPL4326	0.7 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testParserForDuplicateTags	0.72 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testGeoportalTOPOPL	0.67 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testTicket12168	0.82 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testPseudoMercator	0.63 s	1
 org.openstreetmap.josm.data.imagery.WMTSTileSourceTest.testWALLONIE	0.58 s	1
 org.openstreetmap.josm.gui.NavigatableComponentTest.testZoomToFactor	1.2 s	1
 org.openstreetmap.josm.gui.NavigatableComponentTest.testZoomToFactorCenter	1 s	1
 org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[node-shapes]	3.3 s	1
 org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[node-text]	0.71 s	1
 org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[area-icon]	0.39 s	1
 org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[order]	0.57 s	1

comment:75 by Don-vip, 7 years ago

In 11862/josm:

see #7427 - sonar - squid:S2259 - Null pointers should not be dereferenced

comment:76 by Don-vip, 7 years ago

In 11863/josm:

see #7427 - sonar - squid:S1449 - Locale should be used in String operations

comment:77 by Don-vip, 7 years ago

In 11864/josm:

see #7427 - sonar - squid:UselessImportCheck - Useless imports should be removed

comment:78 by Don-vip, 7 years ago

In 11865/josm:

see #7427 - checkstyle

comment:79 by Don-vip, 7 years ago

In 11866/josm:

see #7427 - PMD

comment:80 by bastiK, 7 years ago

In 11867/josm:

see #7427 - fix some unit tests

comment:81 by bastiK, 7 years ago

In 11868/josm:

see #7427 - add javadoc for getNativeProjections() + move it one class up

comment:82 by bastiK, 7 years ago

In 11877/josm:

see #7427 - WMTS: if no default layer specified, choose first available (fixes tests)

comment:83 by bastiK, 7 years ago

In 11882/josm:

see #7427 - minor style change

comment:84 by bastiK, 7 years ago

In 11883/josm:

see #7427 - actually use disk cache on scale change and do not force redownload

in reply to:  19 comment:85 by bastiK, 7 years ago

Replying to Don-vip:

OK. Did you look at jt-warp? It's licensed under Apache 2.0.

I had another look at this library and basically what they are doing, is to extend the JAI classes for nodata support. This is the main selling point of jai-ext: a magic pixel value or range, indicating there is no data for a region of an image. Not exactly something we need, but the code is still interesting:

  • It shows how to make use of the internal image data structure for maximum efficiency. This gets a bit unwieldy, with several cases and code duplication, so not something one can easily copy & paste.
  • It demonstrates some nice tricks and efficient data layout of the JAI library.

The new custom implementation ImageWarp does what it should, but in terms of runtime it is still pretty slow. Thanks to the caching, it doesn't have to be super efficient. But it may still be noticeably slow, especially for 512x512 tiles (test reports welcome).

The main reason for low efficiency is not pixel lookup and color interpolation, but coordinate conversion from one projection to another. Currently, this is done once for each pixel. An obvious optimization would be to take a grid with 10 px spacing, transform only every 10th pixel and interpolate in between (expected improvement: factor 100). In JAI, they have anticipated this problem, the class javax.media.jai.WarpGrid is precisely what would be needed here.

comment:86 by stoecker, 7 years ago

Can you check dropping "epsg4326to3857Supported" handling and using reprojection instead. That should mean that Mercator now looks good for lower zoom levels in case the WMS only supports EPSG:4326 (This is not the case with the workaround).

in reply to:  86 comment:87 by bastiK, 7 years ago

Replying to stoecker:

Can you check dropping "epsg4326to3857Supported" handling and using reprojection instead. That should mean that Mercator now looks good for lower zoom levels in case the WMS only supports EPSG:4326 (This is not the case with the workaround).

The 4326to3857 code should be gone already.

comment:88 by stoecker, 7 years ago

In 11884/josm:

see #7427 - drop old attribute

comment:89 by Don-vip, 7 years ago

In 11891/josm:

see #7427 - sonar - squid:S1226 - Method parameters, caught exceptions and foreach variables should not be reassigned

comment:90 by bastiK, 7 years ago

In 11894/josm:

see #7427 - make sure fixed scale is used for alignment + javadoc

comment:91 by bastiK, 7 years ago

In 11895/josm:

see #7427 - avoid code duplication (loop over colors)

comment:92 by bastiK, 7 years ago

In 11896/josm:

see #7427 - clean up comment

comment:93 by bastiK, 7 years ago

In 11897/josm:

see #7427 - optimize warp transformaion

performance problems should be resolved now

comment:94 by Don-vip, 7 years ago

2 new findbugs warnings:

ImageWarp.java:61, URF_UNREAD_FIELD
UrF: Unread field: org.openstreetmap.josm.tools.ImageWarp$GridTransform.deletedRows

ImageWarp.java:110, DLS_DEAD_LOCAL_STORE
DLS: Dead store to del in org.openstreetmap.josm.tools.ImageWarp$GridTransform.cleanUp(int)

comment:95 by bastiK, 7 years ago

Not sure what to do about it - nothing wrong with the code, as far as I can tell. :)

comment:96 by Don-vip, 7 years ago

Problem comes from private static final boolean CONSISTENCY_TEST = false;. Findbugs operate on bytecode, so it does not guess you want to sometimes set it to true for debugging purposes. Is it still needed?

comment:97 by bastiK, 7 years ago

Yes, it is fairly easy to misuse this class. The code would still run without error, but performance-wise it would have an adverse effect. The consistency test catches this and also documents the expected behavior.

Basically it is a poor man's unit test and only needs to be enabled for a bit whenever the related code changes. I can also activate the consistency test permanently - it shouldn't make much of a difference.

comment:98 by Don-vip, 7 years ago

Why do not enabling this feature in debug or trace mode, rather than through a static boolean? Everyone could enable it without recompiling JOSM, and Findbugs would be happy.

comment:99 by bastiK, 7 years ago

In 11915/josm:

see #7427 - run consistency test in debug mode

by Klumbumbus, 7 years ago

Attachment: DimmingTiles.png added

comment:100 by Klumbumbus, 7 years ago

Resolution: fixed
Status: closedreopened

When dimming a tile layer (here bing) lines appear at the tile borders. They also appear sometimes even without dimming.

(Position: https://www.openstreetmap.org/#map=19/53.03892/14.10883)


comment:101 by bastiK, 7 years ago

Are you using Mercator or a different projection?

comment:102 by Klumbumbus, 7 years ago

Mercator

in reply to:  100 comment:103 by Klumbumbus, 7 years ago

Replying to Klumbumbus:

They also appear sometimes even without dimming.

addition: they are black in this case, i.e. there is a little gap between the tiles

comment:104 by wiktorn, 7 years ago

I've noticed the same. This is especially visible with layers with monotone colours.

One of such examples is ISOK hillshade in Poland - mostly greyish.

WMTS: http://mapy.geoportal.gov.pl/wss/service/WMTS/guest/wmts/ISOK_CIEN?REQUEST=GetCapabilities&SERVICE=WMTS{header(User-Agent,Mozilla/5.0 (JOSM)}

comment:105 by bastiK, 7 years ago

@Klumbumbus: Thanks for reporting, this in not something I have seen so far. I wasn't able to reproduce. Could you start JOSM in debug mode and check again? There will be a thin red outline, but hopefully the seam is still visible underneath. I'm interested in the size of the boxes, i.e. is it 256x256 px or larger? Please add Tile info dialog from the right click menu if you make another screen shot.

@wiktorn: What is the server projection and what is the display projection?

comment:106 by Klumbumbus, 7 years ago

Currently I can't really reproduce this. Lines appear when panning and zooming around when tiles from another zoomlevel are displayed. But in the end when all tiles are loded the lines are gone.

(However when doing the screenshot the lines were permanent.)

comment:107 by wiktorn, 7 years ago

In my case: server projection is EPSG:2180 and display projection - Mercator.

And I've just run in the debug mode. The black lines are the same size as the tiles.

If I take Bing layer as a background and use display projection: EPSG:2180, then the lines are also visible (but far less than with ISOK hillshade at Mercator display projection).

in reply to:  105 comment:108 by Klumbumbus, 7 years ago

Replying to bastiK:

There will be a thin red outline, but hopefully the seam is still visible underneath.

It seems the red lines hide the other lines

in reply to:  106 comment:109 by bastiK, 7 years ago

Replying to Klumbumbus:

Currently I can't really reproduce this. Lines appear when panning and zooming around when tiles from another zoomlevel are displayed. But in the end when all tiles are loded the lines are gone.

Confirmed, I'll have a look.

(However when doing the screenshot the lines were permanent.)

If reproducible, this should be investigated.

in reply to:  107 comment:110 by bastiK, 7 years ago

Replying to wiktorn:

In my case: server projection is EPSG:2180 and display projection - Mercator.

And I've just run in the debug mode. The black lines are the same size as the tiles.

If I take Bing layer as a background and use display projection: EPSG:2180, then the lines are also visible (but far less than with ISOK hillshade at Mercator display projection).

When displaying the imagery in the native projection, it should work just as before, i.e. no black lines between tiles.

However, when reprojecting the tiles one by one, I wasn't able to avoid the seams so far and basically considered it an acceptable imperfection. Please note that the pixel positions are still accurate, i.e. there is no "physical gap" that would break a diagonal straight line.

Maybe there is a simple fix to improve this "feature". It can certainly be avoided by requesting pixel data from the 4 adjacent tiles during warp operation. But this seems excessively complicated.

comment:111 by wiktorn, 7 years ago

Sorry for the noise then. That was what I thought when I saw them for the first time. And I did not read Kulumbus comment carefully enough to notice that he was displaying tiles with no reprojection.

comment:112 by bastiK, 7 years ago

In 11953/josm:

see #7427 - make sure layer is painted only once after zoom change

comment:113 by bastiK, 7 years ago

In 11954/josm:

see #7427 - fix seams for reprojected tiles

comment:114 by bastiK, 7 years ago

In 11956/josm:

see #7427 - repaint after clear tile cache action

comment:115 by bastiK, 7 years ago

In 11958/josm:

see #7427 - clear memory cache on projection change

comment:116 by bastiK, 7 years ago

In 11961/josm:

see #7427 - avoid black lines at tile borders (primarily for native tiles)

comment:117 by bastiK, 7 years ago

The black lines should be pretty much gone now, you can test again.

comment:118 by Don-vip, 7 years ago

It seems to work fine :)

It would be interesting to have a unit test for ReprojectionTile, do you see how to write a meaningful one?

Last edited 7 years ago by Don-vip (previous) (diff)

comment:119 by bastiK, 7 years ago

In 11972/josm:

see #7427 - remove obsolete code

in reply to:  118 comment:120 by bastiK, 7 years ago

Resolution: fixed
Status: reopenedclosed

Replying to Don-vip:

It would be interesting to have a unit test for ReprojectionTile, do you see how to write a meaningful one?

Difficult for this class, but would be good for some of the other new methods.

comment:121 by stoecker, 7 years ago

I'm impressed what already has been done in this ticket. Display of reprojection and also non reprojected tiles is really seemless. Lots of cases with slight distortion at the tile boundary are gone. That's excellent!

Nevertheless there is always a bit to improve. :-) I tried downloading Berlin WMS and while reprojected data is seemless there are small black stripes when you zoom in and out. It seems that the display code for the tiles of up-/downscaled zoom is not as perfect as the current zoom level...

in reply to:  121 comment:122 by bastiK, 7 years ago

Replying to stoecker:

Nevertheless there is always a bit to improve. :-) I tried downloading Berlin WMS and while reprojected data is seemless there are small black stripes when you zoom in and out. It seems that the display code for the tiles of up-/downscaled zoom is not as perfect as the current zoom level...

Yes I'm aware of this, the thing is that reprojected tiles simply don't fit together seamlessly when one comes from a different zoom level and is up- or downscaled. This happens to work well for standard tiles, because they are square shaped and the tile size in pixels is a power of 2. All this is no longer true for a reprojected tile - you may have noticed that it is often slightly rotated and the tile boundary may be curved. I do not have a good solution for this right now.

comment:123 by stoecker, 7 years ago

If you can't fix it, can't you cheat? A black pixel is very good visible. A pixel with the same colour as its neighbor hardly.

comment:124 by bastiK, 7 years ago

If you increase the transparency, you'll notice there are not only gaps but also overlapping parts. So for the cosmetic fix you'd need a pixel-precise mask of the tile in current zoom level, then scale the replacement tile, clip the excess parts and fill in the gaps by extrapolating with the pixel color at the mask outline.

This may be too slow for event dispatch thread, which makes it even more awkward to implement.

Pretty complicated overall, I think, I'll postpone to next month and see if an easier solution emerges.

comment:125 by Don-vip, 3 years ago

Keywords: projection warping added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain bastiK.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.