mapcss: partial fill

[Klumbumbus]

partial fill of areas similar to id editor.

[bastiK]

In 9005/josm:

see #12104 - mapcss: add option for partial filling of areas (inspired by iD)

[bastiK]

In 9008/josm:

see #12104 - mapcss: partial filling of areas (fill-extent) for fill-image

[bastiK]

In 9009/josm:

see #12104 - mappaint: activate "fill areas only around their inner edges"

[Klumbumbus]

(I connected your commits to this collective ticket.)

[Klumbumbus]

There is a bug with multipolygons

What steps will reproduce the problem?

1. unclosed multipolygon (missing part or incomplete downloaded)
2. partial fill is drawn between first and last point (for outer and also for inner)

Please provide any additional information below. Attach a screenshot if possible.

I didn't check fill image

Revision: 9015
Repository Root: http://josm.openstreetmap.de/svn
Relative URL: ^/trunk
Last Changed Author: Don-vip
Last Changed Date: 2015-11-16 03:23:57 +0100 (Mon, 16 Nov 2015)
Build-Date: 2015-11-16 02:34:08
URL: http://josm.openstreetmap.de/svn/trunk
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last Changed Rev: 9015

Identification: JOSM/1.5 (9015 de) Windows 7 32-Bit
Memory Usage: 344 MB / 742 MB (150 MB allocated, but free)
Java version: 1.8.0_65, Oracle Corporation, Java HotSpot(TM) Client VM
VM arguments: [-Djava.security.manager, -Djava.security.policy=file:C:\Program Files\Java\jre1.8.0_65\lib\security\javaws.policy, -DtrustProxy=true, -Djnlpx.home=<java.home>\bin, -Djnlpx.origFilenameArg=C:\Program Files\josm-latest-bla.jnlp, -Djnlpx.remove=true, -Djava.util.Arrays.useLegacyMergeSort=true, -Djnlpx.heapsize=256m,768m, -Djnlpx.splashport=51181, -Djnlpx.jvm=<java.home>\bin\javaw.exe, -Djnlpx.vmargs=LURqYXZhLnV0aWwuQXJyYXlzLnVzZUxlZ2FjeU1lcmdlU29ydD10cnVlAA==]
Dataset consistency test: No problems found

[naoliv]

Shouldn't setting resource://styles/standard/elemstyles.mapcss:boolean:partial_fill to false (in JOSM's advanced prefs) get the old behavior back?

[bastiK]

This would fix the problem of closed incomplete multipolygons. But I'm not sure if it would break anything, Don-vip?

src/org/openstreetmap/josm/data/osm/visitor/paint/relations/Multipolygon.java

@@ -229 +229 @@
                     }
                 }
             }
-            if (!initial) { // fix #7593
-                poly.closePath();
-            }
             for (PolyData inner : inners) {
                 appendInner(inner.poly);
             }

[bastiK]

In 9016/josm:

mapcss: minor improvement for drawing of unclosed areas (see #12104)

[Klumbumbus]

Replying to naoliv:

Shouldn't setting resource://styles/standard/elemstyles.mapcss:boolean:partial_fill to false (in JOSM's advanced prefs) get the old behavior back?

Yes, however it somehow only works when you use the style settings gui https://josm.openstreetmap.de/attachment/wiki/Help/Dialog/MapPaint/style-settings.png

[Don-vip]

Replying to bastiK:

This would fix the problem of closed incomplete multipolygons. But I'm not sure if it would break anything, Don-vip?

I don't remember. Looking at #7593 it involved purging MP members, you can give it a try :)

[bastiK]

In 9017/josm:

mapcss: fix partial fill for incomplete multipolygons (see #12104)

[Klumbumbus]

The linecap is a bit strange in some cases. I think atleast the linecap for closed inners should be fixed.

[bastiK]

In 9032/josm:

mapcss / partial fill: multipolygon inners now closed loop without first & last node visible (see #12104)

[bastiK]

Replying to Klumbumbus:

I think atleast the linecap for closed inners should be fixed.

Done.

The linecap is a bit strange in some cases.

Yes, but at the moment, I don't know how to fix this.

For large incomplete multipolygons (riverbank, forest), the area-margin is sometimes on the wrong side. Without any additional information, there is no way to tell for sure, what is inner and what is outer. One way to deal with this, would be to draw a margin on both sides, as long as the area is not closed:

It would make a clear distinction between closed and "almost closed" ways.

I'm not really sure about this and would like to hear your opinions.

[Klumbumbus]

Replying to bastiK:

For large incomplete multipolygons (riverbank, forest), the area-margin is sometimes on the wrong side. Without any additional information, there is no way to tell for sure, what is inner and what is outer. One way to deal with this, would be to draw a margin on both sides, as long as the area is not closed:

I think this would be good, becasue it would avoid other strange renderings like in the following picture.
And since the width is half on each side it should not be mixed up with a line where two closed areas touch each other.

[RicoElectrico]

My humble opinion is that this fill style should not be turned on by default for users. While it's nice to have, it should be opt-in. Style options are quite deep buried anyway.
Also, there is no logic to discriminate areas for which it makes no sense, like buildings.

[bastiK]

Replying to RicoElectrico:

My humble opinion is that this fill style should not be turned on by default for users. While it's nice to have, it should be opt-in. Style options are quite deep buried anyway.

Thanks for your input! Could you give practical reasons, why you prefer the complete fill to the new style?

Also, there is no logic to discriminate areas for which it makes no sense, like buildings.

We could turn partial fill off for buildings and other selected features. Technically, this would be not a problem. So far, I like that everything is consistent, but there is certainly room for optimization and fine tuning.

[Klumbumbus]

Replying to bastiK:

Replying to RicoElectrico:

My humble opinion is that this fill style should not be turned on by default for users. While it's nice to have, it should be opt-in. Style options are quite deep buried anyway.

Thanks for your input! Could you give practical reasons, why you prefer the complete fill to the new style?

IMO the mapview is more confusing, because there are more lines.

[Don-vip]

+1 to exclude buildings, it is confusing in urban areas:

[bastiK]

Filled version:

This is an interesting example:

First, there are multiple nested areas stacked on top of each other ("ADM" building inside medical faculty building inside university). The partial fill version sure looks more complicated, but the question should be, which view makes it easier to see the structure of areas and understand the data without clicking through each object. For example take the area with the red highlight in the partial-fill image. In the filled version, I can't really tell for sure, if this is a single connected area or not. In the other image, this is quite clear, even without red highlight.

Second, the outline of the "ADM" building is not properly lined up with the other building, the ways are intersecting. This error can be seen in both versions, but in the partial fill image, this is more eye-catching because the overlapping lines are duplicated on the inside of the margin.

That said, I agree, that the partial fill is much less useful for smaller objects and I wouldn't mind turning it off for buildings, etc.

[Don-vip]

Tomorrow I must release JOSM as new tested because of certificate issue. Can we set this new behaviour disabled by default until we are all happy with it? :)

[Klumbumbus]

Replying to Don-vip:

Tomorrow I must release JOSM as new tested because of certificate issue. Can we set this new behaviour disabled by default until we are all happy with it? :)

+1

[bastiK]

In 9055/josm:

see #12104 - turn off partial fill for next release

[bastiK]

Replying to Don-vip:

Tomorrow I must release JOSM as new tested because of certificate issue. Can we set this new behaviour disabled by default until we are all happy with it? :)

Alright.. :)

[Don-vip]

thanks :)

[bastiK]

In 9061/josm:

mappaint partial fill: render unclosed areas differently (margin on both sites) (see #12104)

[bastiK]

Replying to Klumbumbus:

I think this would be good, becasue it would avoid other strange renderings like in the following picture.
And since the width is half on each side it should not be mixed up with a line where two closed areas touch each other.

Done. There is an Einstein pref to turn it on/off and set the relative width (currently 80%).

Replying to Don-vip:

+1 to exclude buildings, it is confusing in urban areas.

I have an alternative idea, that handles all kinds of small areas, not just buildings or what we explicitly set in the style:

Calculate the area of the unfilled black part in the center. If it is less than 50% of the total area, turn off partial fill for this object and fill it completely.

In my first test, this worked quite well and removed the strange looking artifacts in buildings, etc.

[Klumbumbus]

Replying to bastiK:

Calculate the area of the unfilled black part in the center. If it is less than 50% of the total area, turn off partial fill for this object and fill it completely.

That sounds good!

[Don-vip]

Generic solution are often better :)

[bastiK]

In 9063/josm:

mapcss: partial fill - if partial fill would only leave a small gap in the center, fill area completely (see #12104)

[bastiK]

I've implemented this idea, please have a look!

Again, there is an advanced parameter (draw.area.partial_fill_threshold) to tweak the threshold (currently at 50%). This feature is turned off by setting the parameter to something large, e.g. 1000.

Feel free to experiment with all the parameters and suggest alternative values!

[Klumbumbus]

I tested a bit and I think the margin on both sides does not work so good as expected. There are problems especially when MPs are used. With margin on both sides:

• you do not see which side is the inner side which makes the mapview more confusing (The case from comment:16 is in reality rare and mostly the inner side of unclosed areas is shown correct.)
• you do not see different colors if they are not the same on both sides, see the following gif

[Klumbumbus]

Low zoom is also problematic when partial fill is activated and the dataset contains some uncomplete downloaded MPs, see attachment:zoom.gif​

[bastiK]

In 9071/josm:

mappaint partial fill: turn off margin on both sites for unclosed area (see #12104)

[bastiK]

Replying to Klumbumbus:

I tested a bit and I think the margin on both sides does not work so good as expected. There are problems especially when MPs are used. With margin on both sides:

• you do not see which side is the inner side which makes the mapview more confusing (The case from comment:16 is in reality rare and mostly the inner side of unclosed areas is shown correct.)
• you do not see different colors if they are not the same on both sides, see the following gif

Okay, this is pretty convincing. I've turned off the margin on both sides.

[bastiK]

In [9076]:

mapcss: partial fill - fix for [9063] - use scale instead of circum (otherwise it would depend on latitude)

[bastiK]

In 9077/josm:

mapcss: partial fill - fix linecap (see #12104)

[bastiK]

Replying to Klumbumbus:

The linecap is a bit strange in some cases.

Could you try again with [9077]?

[bastiK]

In 9082/josm:

mapcss partial fill: remove [9061] code & turn it on again (see #12104)

[Klumbumbus]

Replying to bastiK:

Replying to Klumbumbus:

The linecap is a bit strange in some cases.

Could you try again with [9077]?

There are some minor artefacts on lower zoom:

attachment:"partial fill test.osm"​ for testing

[Klumbumbus]

I tried to workaroud this issue on low zoom with open multipolygons (see also comment:34) by lowering the fill extend of open multipolygons with lower zoom. But it seems that it is not possible within mapcss, because the pseudoclasses :closed and :unclosed_multipolygon handle uncomplete and just not fully downloaded multipolygons different (which is good in other cases). Atleast I could not find a way that reduces the fill extend only for unclosed multipolygons, but not for closed multipolygons or for ways.

[bastiK]

Replying to Klumbumbus:

I tried to workaroud this issue on low zoom with open multipolygons (see also comment:34) by lowering the fill extend of open multipolygons with lower zoom. But it seems that it is not possible within mapcss, because the pseudoclasses :closed and :unclosed_multipolygon handle uncomplete and just not fully downloaded multipolygons different (which is good in other cases).

Is it?

Atleast I could not find a way that reduces the fill extend only for unclosed multipolygons, but not for closed multipolygons or for ways.

Another option would be to also turn to ordinary fill for small unclosed areas. This would be like in [9063], but maybe with a different, very conservative threshold. When the user draws a building or parking lot, it shouldn't alternate between partial fill and complete fill depending on the current size of the area, this would be confusing.

Replying to Klumbumbus:

There are some minor artefacts on lower zoom

Yes, it turned out to be a rather difficult problem. So far, I could not find an algorithm, that works perfectly for all cases. The current code works well, when the extent is smaller than the distances of the end-nodes to all the other polygon edges (like in comment:13). This should be the case for downloaded incomplete multipolygons, except at low zoom.

When the user is drawing an area, there can be somewhat strange overlaps, as you show in the screenshots. At the moment, I think this is acceptable: It shouldn't happen too often and the area will be closed in the end - then it looks fine.

[Klumbumbus]

Replying to bastiK:

Replying to Klumbumbus:

I tried to workaroud this issue on low zoom with open multipolygons (see also comment:34) by lowering the fill extend of open multipolygons with lower zoom. But it seems that it is not possible within mapcss, because the pseudoclasses :closed and :unclosed_multipolygon handle uncomplete and just not fully downloaded multipolygons different (which is good in other cases).

Is it?

Well, the pseudoclass :unclosed_multipolygon was once developed for validation purposes. There the difference between open multipolygons and actually complete but just uncomplete downloaded multipolygons was important. See ticket:10529#comment:68. However we could rethink the definition of these pseudoclasses. (Maybe split them up and make an own pseudoclass for :complete_downloaded, to make them independent!?)

[bastiK]

In 9099/josm:

mapcss partial fill: move threshold parameter ([9063]) into the mapcss style
(new property fill-extent-threshold)

• add support for this parameter when area is unclosed
• smaller extent for unclosed areas

(see #12104)

[bastiK]

Replying to Klumbumbus:

Well, the pseudoclass :unclosed_multipolygon was once developed for validation purposes. There the difference between open multipolygons and actually complete but just uncomplete downloaded multipolygons was important. See ticket:10529#comment:68. However we could rethink the definition of these pseudoclasses. (Maybe split them up and make an own pseudoclass for :complete_downloaded, to make them independent!?)

Thanks, I wasn't aware of this. I've added a pseudoclass :closed2, which ignores the fact that the multipolygon is downloaded completely and just checks for open ends. This is meant as an (internal) temporary solution, until we figure this out. In addition, there is a new pseudoclass :completely_downloaded in [9099], like you suggested.

As you can see, I've moved much of the code into the style sheet, so it should be easier to adapt. The extent for unclosed areas is now smaller - to visually distinguish from closed areas and (as a side effect) reduce the artifacts further.

[Klumbumbus]

This is working very nice. Great job!
However I think we should use fill-extent-threshold: 50%; instead of 70%. The reason is that with partial fill it is much harder to see if a building way is properly aligned to the aerial imagery and with 70% a lot of "normal sized" buildings use the partial fill to early.

I think we should also make this parameter a josm_pref, so advanced users can adjust it (without editing the mapcss style and then forever use their outdated copy).

[bastiK]

In 9103/josm:

mapcss partial fill: adjust threshold parameter for closed areas (see #12104)

[bastiK]

Replying to Klumbumbus:

This is working very nice. Great job!
However I think we should use fill-extent-threshold: 50%; instead of 70%. The reason is that with partial fill it is much harder to see if a building way is properly aligned to the aerial imagery and with 70% a lot of "normal sized" buildings use the partial fill to early.

Okay, sure.

I think we should also make this parameter a josm_pref, so advanced users can adjust it (without editing the mapcss style and then forever use their outdated copy).

Yes, good idea.

[Klumbumbus]

area[setting("partial_fill")]:closed2 {
    fill-extent: 25;
    fill-extent-threshold: JOSM_pref(partial_fill_extent_threshold, 50%);
}

Does not work. Do we need an additional prop() here?

[bastiK]

Works for me, but we have to keep an eye on the performance. Preferences lookup was a reason for slowness before, it is probably better to add an intermediate cache for this.

[bastiK]

In 9114/josm:

mapcss: make partial fill threshold an advanced preference option (see #12104)

[bastiK]

Replying to Klumbumbus:

Does not work.

Okay, I had to make sure the style is actually updated, when the preference value is changed.

[Klumbumbus]

Replying to bastiK:

Okay, I had to make sure the style is actually updated, when the preference value is changed.

So #10185 is fixed now, right?

[bastiK]

Yes, should be. :)