Clipping outline polygons drawing issue

[stleusc]

I decided to open a second issue here because after all I've seen it seems to be another issue. Here a screenshot, then some explaining.

https://cloud.githubusercontent.com/assets/1132106/4892614/abd1f1ba-63b4-11e4-837d-8494f48998c9.png

In #122 the part that was 'fixed' by changing the max zoom were always parallel lines within a tile that started to show up at zoom level 18. This issue here is different and not fixed so far.

Here a theory: It appears to me that certain roads, when forming a loop are drawn by an outline polygon. When the polygon intersects a tile it gets clipped and a connecting line is drawn between the 2 clipped ends. The clipping starts at around zoom 11 (where the simplification ends), then fades in, stays and fades out again at high zoom levels. Not sure if the fade in/out is due to the poly not intersecting tile edges at these zoom levels.

Any advice?

[stleusc]

Reading through https://github.com/opensciencemap/vtm/blob/master/vtm/src/org/oscim/utils/geom/TileClipper.java seems to confirm my idea. It seems that polygons are simply clipped which works for filled polygons but not for outline polygons. I guess clipping does not happen right on edge of tile but a few pixels outside. This would explain why at lower zoom I don't see the issue since the width of the outline is thinner it might just get clipped off when zoom is low enough. When zooming in, width increases and it shows more of the line INSIDE the tile.

[stleusc]

I'm guessing we need to convert outline polygons that are clipped into polyline since we don't want them to be closed....

[hjanetzek]

This is really a problem in MapDatabase: it always interprets closed ways as polygons... not really sure how to fix this other than hardcoding that closed ways with highway, but without area=yes tag should not be interpreted as polygon

[stleusc]

can we not detect if the polygon is filled or not? If it's not filled then clipping should produced a List of Ploylines after clipping instead of a single polygon. Do you support polylines with gaps?

[stleusc]

And since other mapping libs can render it correctly I'm sure VTM can do the same ;-) VTM is way better than other libs in so many ways. I know we can find a way....

[hjanetzek]

the clipping needs to be applied in MapDatabase, at least when a tile is made up of subtiles. This is before the theme is evaluated.

[stleusc]

Subtiles? Didn't know that existed... another idea, not sure how performance would be and if it can be done....

private final TileClipper mTileClipper = new TileClipper(-2, -2, Tile.SIZE + 2, Tile.SIZE + 2);

I assume that the 2 extra pixels are never rendered on the screen, right? If that is the case, then extending the clipping bounds could fix the issue. The bounds would need to be at least extended by half the max width of outlines that we draw. Then, when clipped we draw the outline at the edge of the clip but outside the visible area.

I know, not very nice but I guess the +/-2 that you added are there for a similar reason?

[hjanetzek]

yea that's a hack to hide polygon outlines on tile boundaries. add more offset as needed ;)

[stleusc]

for (int wayDataBlock = 0; wayDataBlock < wayDataBlocks; wayDataBlock++) {
    mElem.clear();

    if (!processWayDataBlock(featureWayDoubleDeltaEncoding))
        return false;

    if (mElem.isPoly() && mElem.index[0] < 6) {
        //mSkipPoly++;
        continue;
    }

    projectToTile(mElem);

    if (mElem.isPoly()) {
        if (!mTileClipper.clip(mElem)) {
            continue;
        }
    }

    mElem.setLayer(layer);
    mapDataSink.process(mElem);
}

could we check if mElem is a polygon and if that polygon is a road? And if so, call another clip function that actually returns a list of segments of polylines? We then could iterate through the list, update mElem, call setLayer and process and so on. So we basically could dynamically inject a few mElem's.

You think that would work? Might be cleaner?

What do you think the performance hit would be if we clip to +/- 30 for example?

[stleusc]

some 'pseudo code' added. Used ALLCAPS to highlight new functions needed

for (int wayDataBlock = 0; wayDataBlock < wayDataBlocks; wayDataBlock++) {
    mElem.clear();

    if (!processWayDataBlock(featureWayDoubleDeltaEncoding))
        return false;

    if (mElem.isPoly() && mElem.index[0] < 6) {
        //mSkipPoly++;
        continue;
    }

    projectToTile(mElem);

    List<???> lines = null;     // not sure how the line would be stored, sure there is something in VTM already that should be used

    if (mElem.isPoly()) {
        if (mElem.ISROAD()) { // do multiline clip
            lines = mTileClipper.MULTILINECLIP(mElem);
            if (lines.length == 0) {
                continue;
            }
        } else {
            if (!mTileClipper.clip(mElem)) {
                continue;
            }
        }
    }

    if (lines != null) {
        for (??? line : lines) {
            mElem.clear();
            mElem.UPDATELINEDATA(line);
            mElem.setLayer(layer);
            mapDataSink.process(mElem);
        }
    } else {
        mElem.setLayer(layer);
        mapDataSink.process(mElem);
    }
}

[stleusc]

/**
 * The GeometryBuffer class holds temporary geometry data for processing.
 * Only One geometry type can be set at a time. Use 'clear()' to reset the
 * internal state.
 * <p>
 * 'points[]' holds interleaved x,y coordinates
 * <p>
 * 'index[]' is used to store number of points within a geometry and encode
 * multi-linestrings and (multi-)polygons.
 */

Seems like GeometryBuffer can already handle multi segment lines? If that's the case then clip could simply generate a multi segment line instead of polygon based on a boolean parameter. That way the change is very simple by adding a isRoad() as parameter and have the clipper produce the correct Geometry.

[hjanetzek]

At this point you could set mElem.type = GeometryType.LINE to use line instead of poly clipping: https://github.com/opensciencemap/vtm/blob/master/vtm/src/org/oscim/tiling/source/mapfile/MapDatabase.java#L899

[stleusc]

Ohhhh, I like that idea... So basically we cheat and pretend that the polygon is a polyline before clipping? Does the polygon have an identical start/end point set or is the closing of the ploygon done via magic code? Asking because the polyline needs the identical start/endpoint to look right.

How does the point look like where the 2 ends of a 'closed' polyline meet? What about 'caps' at the end of lines? Would there be round caps or anything visible at the edges?

[hjanetzek]

Here https://github.com/opensciencemap/vtm/blob/master/vtm/src/org/oscim/tiling/source/mapfile/MapDatabase.java#L1012 it should not drop the last node if mGeom.type == LINE

yes, lines will have round end-caps at start and end - not sure where that matters

[hjanetzek]

.. no actually one needs to pass whether one expects sth to be a line into

decodeWayNodesDelta and ignore this heuristic: https://github.com/opensciencemap/vtm/blob/master/vtm/src/org/oscim/tiling/source/mapfile/MapDatabase.java#L622 https://github.com/opensciencemap/vtm/blob/master/vtm/src/org/oscim/tiling/source/mapfile/MapDatabase.java#L674

imo whether something is a polygon or not should be encoded by mapsforge mapfile writer instead of just dumping raw osm tags..

[stleusc]

Could you create a patch for that that I could try out?

[hjanetzek]

on my todo-heap

[stleusc]

how big is your todo-list ;-)

[hjanetzek]

hard to say. size() is unimplemented and the priority function is a close approximation to rand()

[stleusc]

hahaha, only a SW guy would write a comment like that ;-) If you can provide a tiny bit more insight on what should be done where I would give it a shot. I really need this fixed... Too many people complaining to me :-(

[hjanetzek]

you are lucky - just pushed an updated (but untested) MapDatabase

[stleusc]

You're awesome! Thanks.... I'll try it tonight and let you know how it goes. Do you think that this will also impact/fix #122 on high zoom levels or do you also believe that both issues are different?

[stleusc]

I quickly tried the dirty hack...

            mTileClipper.setRect(xmin-20, ymin-20, xmax+20, ymax+20);
        } else {
            mTileClipper.setRect(-20, -20, Tile.SIZE + 20, Tile.SIZE + 20);

which seems to fix it (at least when artifact is < 10px wide). Now I'll try your latest version.

[stleusc]

OK, with my hack I still an odd artifact in one place (different than before). Your latest version fixed that as well! Now I can try to see if it also takes care of #122 when going back to zoom level 18.

I have a not fully painted building though. Screenshot attached. This is with your latest version.

moved to #124, unrelated..

Also just reverted back to my 'hack' version. Same thing. So it's older just didn't notice!

[stleusc]

Summary: Using the 4 commits you made in your repo and the current trunk from here fixes this issue completely and it also fixes #122 with max zoom of 18. So I guess once you make the changes official we can close both issues.

Thanks so much for the awesome and quick help.

[devemux86]

In the fork by Mapsforge I changed TileClipper buffer to be scaled on dpi, for eliminating the artifacts in zooms >17.

See https://github.com/mapsforge/vtm/issues/231