Point moves off to the side with "3+++++"...

[jidanni]

Select a point and hit '3+++++'...

Notice the point slowly moves off to the side. This should not happen. I believe it must be due to rounding errors.

If one part of the software cannot deal with the output of another part, then maybe zooming should be halted when it hits a certain level.

One might say who would ever want to zoom so far... but that rules out unknown future use.

[stoecker]

Works in r11833, fails in r11839. It is not so easy to compile old version due to external dependencies, but I assume without testing, that r11835 is the reason.

[bastiK]

This is pretty much the expected behavior after [11835]. The position of the map is moved by up to 0.5 pixels for alignment. So "3+++++++" will not zoom in to the node, but to a point very near the node. In fact, you may have found the only way a user could notice.

[jidanni]

And... this might have destroyed the only way to really zoom in on a point.

The user reaches for a "3" again to get the point back to the center, but that will zoom him back out!

[Don-vip]

At very high zoom the displacement is far more than 0.5 pixel, I agree the behaviour is strange.

[bastiK]

The displacement is 0.5 pixels after pressing "3" and effectively doubles each time you zoom in.

[bastiK]

Attached patch fixes the problem. It doesn't exactly improve the maintainability of the code, though.

[stoecker]

No. That's ugly for a single obscure use-case. I tested it and the normal zoom-wheel is not affected and the center of larger selections is anyway not that exact. Adding a workaround for the singular case of zooming to a single node is not helpful.

[michael2402]

We should decide on a pixel-policy before trying to fix this. Otherwise, we will run into a lot of issues with it.

I'd suggest:

• Don't force a pixel alignment for the map view state object. That object is always using the exact point as center.
• Add a new method to the map view state: mapViewState.getAlignedState(). This does the same as alignToPixelGrid and gives you the state the map view would be in if it were aligned to that pixel grid.
• Use that state explicitly when drawing things that need to be pixel aligned (osm layer, ...). The "normal" state is used on default.

The user won't notice the sub-pixel difference when clicking on a node or doing other things and the map drawing will be aligned to pixels. We ma have a small offset to e.g. the imagery layers. This is not a problem, since you can't move a node less than one pixel when having zoomed out and on zooming in, that error reduces.

A problem I see is that we might run into rounding issues with mapViewState.getAlignedState(). So a "right" move of 2.5 might move the map by 2 pixels, then by 3, then by 2, ... But this can be seen as intended behavior.

[bastiK]

Replying to stoecker:

No. That's ugly for a single obscure use-case. I tested it and the normal zoom-wheel is not affected and the center of larger selections is anyway not that exact. Adding a workaround for the singular case of zooming to a single node is not helpful.

Basically, I agree with this assessment. The patch can be brushed up a little, but this is the best I can do without removing the alignment code. The pixel alignment is necessary for seamless display of reprojected tiles.

[bastiK]

Replying to michael2402:

We should decide on a pixel-policy before trying to fix this. Otherwise, we will run into a lot of issues with it.

I'd suggest:

• Don't force a pixel alignment for the map view state object. That object is always using the exact point as center.
• Add a new method to the map view state: mapViewState.getAlignedState(). This does the same as alignToPixelGrid and gives you the state the map view would be in if it were aligned to that pixel grid.
• Use that state explicitly when drawing things that need to be pixel aligned (osm layer, ...). The "normal" state is used on default.

The user won't notice the sub-pixel difference when clicking on a node or doing other things and the map drawing will be aligned to pixels. We ma have a small offset to e.g. the imagery layers. This is not a problem, since you can't move a node less than one pixel when having zoomed out and on zooming in, that error reduces.

A problem I see is that we might run into rounding issues with mapViewState.getAlignedState(). So a "right" move of 2.5 might move the map by 2 pixels, then by 3, then by 2, ... But this can be seen as intended behavior.

There may be drawbacks when background imagery and Node pixels can be offset anywhere between -0.5 to +0.5 pixels. It means that a Node can move by 1 pixel in relation to the background when you move the map or zoom in and out again. My guess is that this would be noticeable.

A uniform consistent state and a "silent" unaligned tracking state (called centerCarry in the patch) is the cleaner concept IMO. But the overall structure gets messy with the patch, that's why I put you in cc. :)

[bastiK]

Maybe close the ticket as "wontfix" and wait for another user to report this problem?

[bastiK]

michael2402, to reiterate my idea, it is pretty close to what you have outlined: Instead of using mapViewState.getAlignedState() explicitly/selectively, I'd use it everywhere except in NavigatableComponent (Zoom history, zoom in, zoom out, ...).

Naturally you would rename/reorganize the methods, so only the code in MapViewState and NavigatableComponent needs to be touched. It is a matter of viewpoint if you consider the "true" state to be the aligned or the unaligned one.

[michael2402]

One idea how we could work around this one pretty easily is to zoom in on repeated 3 presses (so pressing 33333 zooms to the node). But it would not be intuitive to use.

As long as there are no real uses for this, closing as wontfix is good. If someone requests it, we can always decide to implement it or closing that ticket ("closed as duplicate") :-).