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

Misrendered line objects when .omap file is used as a template #1186

Closed
lpechacek opened this issue Nov 29, 2018 · 10 comments

Comments

@lpechacek
Copy link
Member

commented Nov 29, 2018

Steps to reproduce

  1. Add .omap file as a template
  2. Zoom out a bit (approx. 2x)

Actual behaviour

Line objects are misrendered. A fence became a tag-less line, erosion gully disappeared completely.

Example - see control site with code 41:
2x zoom:
image

4x zoom:
image

Expected behaviour

The map should render in the same way as if it is opened directly.

The template at 2x zoom when opened directly:
image

Configuration

Mapper Version: origin/master
Operating System: Android 7

@dg0yt

This comment has been minimized.

Copy link
Member

commented Nov 30, 2018

I'm only aware of the spot which you already touched in #1049. This is one of the "hottest" paths in Mapper. The extra short cut for Android is meant to speed up rendering on the low power devices by leaving out details when they are barely visible. (I do need that when zooming out in maps dominated by vine yards, orchards, and cultivated land.) Maybe some other factors, such as screen resolution need to be considered as well.

@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 2, 2018

PainterConfig::activateState() isn't the worst place to sort out small items, in contrast to Renderable::render() implementations.

However, only the pen width is considered for short cuts there, i.e. it affects line renderables. And I can't find an example of missing fence tags.

On the other hand, I can't reproduce loosing dot renderables.

difference

@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 5, 2018

This is the second spot where we filter out small renderables:

#ifdef Q_OS_ANDROID
const QRectF& extent = renderable->getExtent();
if (extent.width() < min_dimension && extent.height() < min_dimension)
continue;
#endif
if (renderable->intersects(config.bounding_box))
{
renderable->render(*painter, config);
}

And min_dimension is the number of mm covered by one pixel, which eventually depends on the screen resolution as presented in the settings dialog.

However, this is mostly obsolete if 1a74180 works well enough. With 1a74180, the (approximate) area extent size is stored as (otherwise unused) pen width in the PainterConfig, so that these renderables may be skipped already in the context of state.activate(...), before even starting to loop over the renderables. Now it all depends on tuning qreal proxyPenWidth(const QRectF& extent)...

@lpechacek

This comment has been minimized.

Copy link
Member Author

commented Dec 5, 2018

In the meantime I had another look at the bug from outside. I can replicate with stock Mapper 0.8.3. The missing fence tag gives a hint that mid-symbols get omitted at certain zoom level. Further playing with the zoom level I found out that vegetation boundary dots disappear around level 3x on 1:10 000 map, erosion gully dots disappear at 2.55x, fence (516) tag lines at 1.45x. Impassable fence (518) tags disappear between 1.3x and 1.2x depending on their direction. :) It points to the code spot you are mentioning.

I agree that the recent renderer changes bring significant difference in behavior. I'll try compiling Android version of the current master and re-test.

@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 5, 2018

I still don't understand how this could become visible when it is all designed to be relevant below 1 px. There must be a serious miscalculation about the size of one pixel. If you want to debug it, you can perhaps add this info to the zoom factor toast message.

Anyway, I will try to simplify, and probably add a simple configuration parameter, 0-100%. which is the pixel width where to cut off the details - at 0%, everything is always drawn.

@lpechacek

This comment has been minimized.

Copy link
Member Author

commented Dec 5, 2018

I've finally managed to build latest master for Android and re-tested with the recent fix. No change. .omap file rendered as a template is still missing details. I've sent you the map via e-mail.

Good night!

@lpechacek lpechacek changed the title Misrendered line objects when .omap file is used a a template Misrendered line objects when .omap file is used as a template Dec 5, 2018
@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

Whatever I try on the desktop (with Android behaviour enabled), the dots are removed only when they get below one pixel, as expected.

I'll try compiling Android version of the current master and re-test.

What version of Qt are you building with? Does it uses some High DPI scaling on Android? What is the screen resolution displayed in the settings? Is the zoom correct?

@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

I just noticed that I didn't read the template part of the story. Still unsure how this can make a difference when the template uses the same scale as the map.

@dg0yt

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

The template map is drawn with config.scale being the zoom factor, not the number of pixels per mm. This is why we end up with a wrong min_dimension.

dg0yt added a commit that referenced this issue Dec 6, 2018
When drawing the map, PainterConfig::scaling must be set to the number
of pixels per mm. Fixes #1186.
While this was noticed on screen, it was also wrong for printing and
export. For targets other than the screen, we take the physical
resolution from the paint device.
@lpechacek

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2018

Bulls eye! The fix from rendering branch fixes this issue! Thanks!

dg0yt added a commit that referenced this issue Dec 6, 2018
When drawing the map, RenderConfig::scaling must be set to the number
of pixels per mm. Fixes #1186.
Document explicitly that scaling means pixel per mm.
According to the documentation, this variable is used in connection
with RenderConfig::ForceMinSize which is normally not used for
printing or export. However this change still sets the pixel per mm
from the physical (alternatively logical) resolution of the paint
device.
@dg0yt dg0yt closed this in #1190 Dec 9, 2018
dg0yt added a commit that referenced this issue Dec 9, 2018
When drawing the map, RenderConfig::scaling must be set to the number
of pixels per mm. Fixes #1186.
Document explicitly that scaling means pixel per mm.
According to the documentation, this variable is used in connection
with RenderConfig::ForceMinSize which is normally not used for
printing or export. However this change still sets the pixel per mm
from the physical (alternatively logical) resolution of the paint
device.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.