Modify

Opened 8 years ago

Last modified 5 years ago

#13386 new enhancement

[Patch draft] Make imagery support multiple map views

Reported by: michael2402 Owned by: team
Priority: normal Milestone:
Component: Core imagery Version:
Keywords: gsoc-core Cc: Don-vip, bastiK, stoecker

Description

This patch moves most of the paint code away from AbstractTileSourceLayer. A warning ahead: The patch has more than 4500 lines. A lot of this is moving things around, but there are some new things:

  • Changed the rendering code to use a Stream of tile positions that should be painted.
  • Use an AffineTransform to translate the images.
  • Make the projection not natively supported warning a simple text on the screen - anyone who is playing with such wild projections that the images are off by a lot probably knows what he is doing
  • A hint will appear when you right-click a tile
  • Tiles are identified by a TilePosition. No more fake tiles are used

Some things might need improvement:

  • Imagery offset is stored in east/north. This means that the offset is wrong after a projection change. Even worse, offset does not work correctly for non-orthogonal projections.
  • The precache task is not working
  • Native scale is not working
  • Probably has some bugs, I have not tested everything yet (Only Bing+OSM)


Attachments (2)

patch-mapview-extract-imagery-rendering.patch (175.1 KB ) - added by michael2402 8 years ago.
patch-mapview-extract-imagery-rendering.png (94.9 KB ) - added by michael2402 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by michael2402, 8 years ago

Summary: [Patch draft][Patch draft] Make imagery support multiple map views

comment:2 by Don-vip, 8 years ago

Milestone: 16.0816.09

The patch is too big and risky to include it 5 days before the planned release, it will have to wait the next one.

comment:3 by michael2402, 8 years ago

I thought of that, this is why I did not want to push you ;-). I may even find time to finish the continuous projections until the next release (although I won't have as much time as I did the last weeks).

comment:4 by Don-vip, 8 years ago

I know :) Don't worry the GSOC-Core is already a success! Thanks for your hard work :)

comment:5 by Don-vip, 8 years ago

Milestone: 16.09

I think we must put this one aside for now. There are yet too many gsoc-core regressions to fix in JOSM core and plugins before starting such a massive change.

comment:6 by bastiK, 7 years ago

I wasn't aware of this patch while working on #7427. It's a pity, really - it would have made my task easier if it was applied first. Some ideas from the patch may have been already implemented independently, for example AffineTransform for tile placement.

Changes I like:

  • TilePosition instead of fake tile
  • AbstractTileSourceLayer is too large, extract parts of it into separate classes
  • rework algorithm for finding replacement tiles from other zoom levels for tiles currently downloaded

Obviously, it is horribly outdated now, not sure how to proceed.

comment:7 by michael2402, 7 years ago

Hi,

I thought you knew this one, since you are in CC ;-).

If you want to, I can try to merge it. There were some problems I stumbled upon during coordinate conversion (especially supporting multiple projections/map views at the same time). This is why I focused on that first.

The biggest change is TileSourcePainter: It is created for each map view. That way, you can support displaying the same layer multiple times.

in reply to:  7 comment:8 by bastiK, 7 years ago

Replying to michael2402:

If you want to, I can try to merge it. There were some problems I stumbled upon during coordinate conversion (especially supporting multiple projections/map views at the same time). This is why I focused on that first.

Yes, if you can split it into multiple commits, that would be helpful! The image looks interesting, this patch would have solved #7427 at higher zoom level?

One thing I aimed for, was to do all tile related coordinate calculation in EastNorth space. For reprojection, that means translation from EastNorth of the server projection to EastNorth of the display projection, disregarding the intermediate LatLon coordinates.

comment:9 by michael2402, 7 years ago

I was aiming for making this independend of the representation. This is why I used intermediate Lat/Lon: I know the server projection, I can easily map them to LatLon. I know the current projection, so I can compute the position on the screen.
One idea behind this is to add the offset to Lat/Lon (should be fine for small distances and small areas). Offset support is not so easy then.
The second, more important problem is, that a eastnorth -> latlon conversion is not injective. The same point may be displayed multiple times on screen. I experimented a bit with this. I introduced a new Projecting class, which is a stripped down projection (Projection extends it, so everything is fine again). You can then get a list of Projecting objects for the view space with their bounding box. Drawing code then iterates over all (bounds, projecting) pairs for the draw space and draws the object. I made drawing work that way, but there are still many issues with that (lines crossing from one draw space to the other, ...). But at least it made a continuous drawing over the 180th meridian possible.

There is the commit history for this patch:
https://github.com/michaelzangl/josm/commits/mapview-extract-imagery-rendering

I don't know which low zoom problem you are referring to.

I am currently working on integrating some of the experiments I did last summer, but currently priority is to:

  • Introduce per-dataset selection listeners (today)
  • Add ILatLon (to allow nodes to be used as LatLon objects and remove the getCoor() calls)
  • Make GPX objects listenable and remove the isChanged() method there.
Last edited 7 years ago by michael2402 (previous) (diff)

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

Replying to michael2402:

I was aiming for making this independend of the representation.

At some level you have to be aware of the fact whether the server projection matches the display projection or not.

This is why I used intermediate Lat/Lon: I know the server projection, I can easily map them to LatLon. I know the current projection, so I can compute the position on the screen.
One idea behind this is to add the offset to Lat/Lon (should be fine for small distances and small areas). Offset support is not so easy then.

Not sure, it is a good idea to keep the offset in Lat/Lon. Think of a map of the South Pole Station and its surroundings. Admittedly, not the most typical use case, but it works fine at the moment.

The second, more important problem is, that a eastnorth -> latlon conversion is not injective.

There is a one-to-one mapping between the EastNorth space of one projection and the EastNorth space of a second one. For this to work, you have to ensure that 2 points that are close together in EastNorth are also close together in LatLon. (Mathematically, this could be called a Riemann surface, but it probably boils down to simply not wrapping or clamping the intermediate longitude value.)

The same point may be displayed multiple times on screen. I experimented a bit with this. I introduced a new Projecting class, which is a stripped down projection (Projection extends it, so everything is fine again). You can then get a list of Projecting objects for the view space with their bounding box. Drawing code then iterates over all (bounds, projecting) pairs for the draw space and draws the object. I made drawing work that way, but there are still many issues with that (lines crossing from one draw space to the other, ...). But at least it made a continuous drawing over the 180th meridian possible.

Interesting, I have to look a bit more into this!

There is the commit history for this patch:
https://github.com/michaelzangl/josm/commits/mapview-extract-imagery-rendering

Thanks.

I don't know which low zoom problem you are referring to.

An affine transform is only a good approximation as long as the true transformation does not change too much within the bound of a single tile. Otherwise the pixels in the center of the tile may be displaced. This becomes very visible at low zoom.

I am currently working on integrating some of the experiments I did last summer, but currently priority is to:

  • Introduce per-dataset selection listeners (today)
  • Add ILatLon (to allow nodes to be used as LatLon objects and remove the getCoor() calls)
  • Make GPX objects listenable and remove the isChanged() method there.

Sure, no rush.

in reply to:  10 ; comment:11 by michael2402, 7 years ago

Replying to bastiK:

Replying to michael2402:

I was aiming for making this independend of the representation.

At some level you have to be aware of the fact whether the server projection matches the display projection or not.

You would only need this, if you have problems with rounding issues. As long as your computations are exact (and double is not 100% exact, but at least a lot exact), you should not have any problems from converting between different representations of a point on screen.

This is why I used intermediate Lat/Lon: I know the server projection, I can easily map them to LatLon. I know the current projection, so I can compute the position on the screen.
One idea behind this is to add the offset to Lat/Lon (should be fine for small distances and small areas). Offset support is not so easy then.

Not sure, it is a good idea to keep the offset in Lat/Lon. Think of a map of the South Pole Station and its surroundings. Admittedly, not the most typical use case, but it works fine at the moment.

I know it is not perfect. We could simply treat it as rotation and then use any format we want to represent that rotation. But east/north is fine, as long as it is in the original projection of the imagery layer. That way, we don't get invalid offsets on projection changes.

The second, more important problem is, that a eastnorth -> latlon conversion is not injective.

There is a one-to-one mapping between the EastNorth space of one projection and the EastNorth space of a second one. For this to work, you have to ensure that 2 points that are close together in EastNorth are also close together in LatLon. (Mathematically, this could be called a Riemann surface, but it probably boils down to simply not wrapping or clamping the intermediate longitude value.)

And you have to ensure, that every point that is represented in one east/north space has exactly one point in the other east/north space. There are problems as soon as the projection is infinite or as soon as it does not support the whole world.

The same point may be displayed multiple times on screen. I experimented a bit with this. I introduced a new Projecting class...

Interesting, I have to look a bit more into this!

Projecting#getProjectingsForArea ;-)

An affine transform is only a good approximation as long as the true transformation does not change too much within the bound of a single tile. Otherwise the pixels in the center of the tile may be displaced. This becomes very visible at low zoom.

No, I did not solve that. But I prepared getTransformForTile() to be used for smaller areas than the whole tile to allow for sub-tile painting.

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

Replying to michael2402:

Replying to bastiK:

At some level you have to be aware of the fact whether the server projection matches the display projection or not.

You would only need this, if you have problems with rounding issues. As long as your computations are exact (and double is not 100% exact, but at least a lot exact), you should not have any problems from converting between different representations of a point on screen.

The main motivation is the issue with wrapping around 180 degrees, that you mentioned. You can avoid this problem almost entirely, if you do not convert to LatLon (and back) in the first place.

There is a one-to-one mapping between the EastNorth space of one projection and the EastNorth space of a second one. For this to work, you have to ensure that 2 points that are close together in EastNorth are also close together in LatLon. (Mathematically, this could be called a Riemann surface, but it probably boils down to simply not wrapping or clamping the intermediate longitude value.)

And you have to ensure, that every point that is represented in one east/north space has exactly one point in the other east/north space.

No, you don't have to ensure that (and you can't). The mapping is one-to-one in the area where the two wold bounds overlap and not defined elsewhere. It can give some funny distortions at the world bounds of the display projection, but other than that, it is not a problem. And keeping the coordinates in LatLon does nothing to improve the situation, it is simply the best one can do.

An affine transform is only a good approximation as long as the true transformation does not change too much within the bound of a single tile. Otherwise the pixels in the center of the tile may be displaced. This becomes very visible at low zoom.

No, I did not solve that.

Right, but I did. ;) And you may need to accommodate your code a bit to that.

comment:13 by michael2402, 5 years ago

I would like to come back to this patch, since the current solution I found for painting a background imagery to two separate contexts is not really what I liked.
See: https://github.com/BBloggsbott/autobound/issues/1

What are the open issues with this patch? The main part of it (extracting the layer painting to a new class and not relying on the main mapView any more) is not applied yet, some minor changes are.

The problems I see that need to be solved:

  • Imagery offsets. They are currently stored in east/north in the original projection. So task to do: Apply that transform in the east/north space of the original projection. This is rather difficult for large displacements, since east/north space in the original projection has nothing to do with out current projection. Same goes for the action that sets the displacement. In the long run, I think we should store the displacement relative to lat/lon coordinate space and the origin point around which the displacement was defined.
  • Native scaling needs to be fixed again. The problem here is that this will only work when the user is using a projection that does not require tiles to be rotated / screwed.

in reply to:  13 ; comment:14 by Don-vip, 5 years ago

Replying to michael2402:

What are the open issues with this patch?

No idea. bastik made the review at the time but he's no longer active. If you submit an updated patch I can take a look with a fresh eye :)

in reply to:  14 ; comment:15 by michael2402, 5 years ago

Replying to Don-vip:

No idea. bastik made the review at the time but he's no longer active. If you submit an updated patch I can take a look with a fresh eye :)

Do you know which (maintained) plugins might be affected by this change? The only one I am aware of is imagery_offset_db.

in reply to:  15 comment:16 by Don-vip, 5 years ago

Replying to michael2402:

Do you know which (maintained) plugins might be affected by this change?

No but it's easy to find out: apply your patch and try to build plugins :)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to michael2402.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


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