Changeset 5437 in josm

Timestamp:

Aug 12, 2012 11:43:00 PM (10 months ago)

Author:

bastiK

Message:

fixed #7948 - inverted filter "child type:way" disables untagged nodes of ways

Location:

trunk/src/org/openstreetmap/josm/data

Files:

• Preferences.java (modified) (2 diffs)
• osm/FilterMatcher.java (modified) (5 diffs)
• osm/FilterWorker.java (modified) (2 diffs)
• osm/OsmPrimitive.java (modified) (9 diffs)

trunk/src/org/openstreetmap/josm/data/Preferences.java

@@ -106 +106 @@
     protected final SortedMap<String, List<Map<String,String>>> listOfStructsDefaults = new TreeMap<String, List<Map<String,String>>>();
 
+    /**
+     * Interface for a preference value
+     *
+     * @param <T> the data type for the value
+     */
     public interface Setting<T> {
+        /**
+         * Returns the value of this setting.
+         *
+         * @return the value of this setting
+         */
         T getValue();
+
+        /**
+         * Enable usage of the visitor pattern.
+         *
+         * @param visitor the visitor
+         */
         void visit(SettingVisitor visitor);
+
+        /**
+         * Returns a setting whose value is null.
+         *
+         * Cannot be static, because there is no static inheritance.
+         * @return a Setting object that isn't null itself, but returns null
+         * for {@link #getValue()}
+         */
         Setting<T> getNullInstance();
     }
@@ -448 +472 @@
      * operating systems and hardware, this shouldn't be a performance problem.
      * @param key the unique identifier for the setting
-     * @param value the value of the setting. Can be null or "" wich both removes
+     * @param value the value of the setting. Can be null or "" which both removes
      *  the key-value entry.
      * @return if true, something has changed (i.e. value is different than before)

trunk/src/org/openstreetmap/josm/data/osm/FilterMatcher.java

@@ -44 +44 @@
  */
 public class FilterMatcher {
+
+    /**
+     * Describes quality of the filtering.
+     *
+     * Depending on the context, this can either refer to disabled or
+     * to hidden primitives.
+     *
+     * The distinction is necessary, because untagged nodes should only
+     * "inherit" their filter property from the parent way, when the
+     * parent way is hidden (or disabled) "explicitly" (i.e. by a non-inverted
+     * filter). This way, filters like
+     * <code>["child type:way", inverted, Add]</code> show the
+     * untagged way nodes, as intended.
+     *
+     * This information is only needed for ways and relations, so nodes are
+     * either <code>NOT_FILTERED</code> or <code>PASSIV</code>.
+     */
+    public enum FilterType {
+        /** no filter applies */
+        NOT_FILTERED,
+        /** at least one non-inverted filter applies */
+        EXPLICIT,
+        /** at least one filter applies, but they are all inverted filters */
+        PASSIV
+    }
 
     private static class FilterInfo {
@@ -102 +127 @@
     }
 
-    private boolean getState(OsmPrimitive primitive, boolean hidden) {
-        return hidden?primitive.isDisabledAndHidden():primitive.isDisabled();
-    }
-
+    /**
+     * Check if primitive is filtered.
+     * @param primitive the primitive to check
+     * @param hidden the minimum level required for the primitive to count as filtered
+     * @return when hidden is true, returns whether the primitive is hidden
+     * when hidden is false, returns whether the primitive is disabled or hidden
+     */
+    private boolean isFiltered(OsmPrimitive primitive, boolean hidden) {
+        return hidden ? primitive.isDisabledAndHidden() : primitive.isDisabled();
+    }
+
+    /**
+     * Check if primitive is hidden explicitly.
+     * Only used for ways and relations.
+     * @param primitive the primitive to check
+     * @param hidden the level where the check is performed
+     * @return true, if at least one non-inverted filter applies to the primitive
+     */
+    private boolean isFilterExplicit(OsmPrimitive primitive, boolean hidden) {
+        return hidden ? primitive.getHiddenType() : primitive.getDisabledType();
+    }
+
+    /**
+     * Check if all parent ways are filtered.
+     * @param primitive the primitive to check
+     * @param hidden parameter that indicates the minimum level of filtering:
+     * true when objects need to be hidden to count as filtered and
+     * false when it suffices to be disabled to count as filtered
+     * @return true if (a) there is at least one parent way
+     * (b) all parent ways are filtered at least at the level indicated by the
+     * parameter <code>hidden</code> and
+     * (c) at least one of the parent ways is explicitly filtered
+     */
     private boolean allParentWaysFiltered(OsmPrimitive primitive, boolean hidden) {
         List<OsmPrimitive> refs = primitive.getReferrers();
-        boolean foundWay = false;
-
+        boolean isExplicit = false;
         for (OsmPrimitive p: refs) {
             if (p instanceof Way) {
-                foundWay = true;
-                if (!getState(p, hidden))
+                if (!isFiltered(p, hidden))
                     return false;
-            }
-        }
-
-        return foundWay;
+                isExplicit |= isFilterExplicit(p, hidden);
+            }
+        }
+        return isExplicit;
     }
 
@@ -124 +176 @@
         List<OsmPrimitive> refs = primitive.getReferrers();
         for (OsmPrimitive p: refs) {
-            if (p instanceof Way && !getState(p, hidden))
+            if (p instanceof Way && !isFiltered(p, hidden))
                 return true;
         }
@@ -131 +183 @@
     }
 
-    private boolean test(List<FilterInfo> filters, OsmPrimitive primitive, boolean hidden) {
+    private FilterType test(List<FilterInfo> filters, OsmPrimitive primitive, boolean hidden) {
 
         if (primitive.isIncomplete())
-            return false;
-
-        boolean selected = false;
+            return FilterType.NOT_FILTERED;
+
+        boolean filtered = false;
         // If the primitive is "explicitly" hidden by a non-inverted filter.
         // Only interesting for nodes.
-        boolean explicitlyHidden = false;
+        boolean explicitlyFiltered = false;
 
         for (FilterInfo fi: filters) {
             if (fi.isDelete) {
-                if (selected && fi.match.match(primitive)) {
-                    selected = false;
+                if (filtered && fi.match.match(primitive)) {
+                    filtered = false;
                 }
             } else {
-                if ((!selected || (!explicitlyHidden && !fi.isInverted)) && fi.match.match(primitive)) {
-                    selected = true;
+                if ((!filtered || (!explicitlyFiltered && !fi.isInverted)) && fi.match.match(primitive)) {
+                    filtered = true;
                     if (!fi.isInverted) {
-                        explicitlyHidden = true;
+                        explicitlyFiltered = true;
                     }
                 }
@@ -159 +211 @@
             // Technically not hidden by any filter, but we hide it anyway, if
             // it is untagged and all parent ways are hidden.
-            if (!selected)
-                return !primitive.isTagged() && allParentWaysFiltered(primitive, hidden);
+            if (!filtered) {
+                if (!primitive.isTagged() && allParentWaysFiltered(primitive, hidden))
+                    return FilterType.PASSIV;
+                else
+                    return FilterType.NOT_FILTERED;
+            }
             // At this point, selected == true, so the node is hidden.
             // However, if there is a parent way, that is not hidden, we ignore
             // this and show the node anyway, unless there is no non-inverted
             // filter that applies to the node directly.
-            if (!explicitlyHidden)
-                return !oneParentWayNotFiltered(primitive, hidden);
-            return true;
-        } else
-            return selected;
-
-    }
-
-    public boolean isHidden(OsmPrimitive primitive) {
+            if (!explicitlyFiltered) {
+                if (!oneParentWayNotFiltered(primitive, hidden))
+                    return FilterType.PASSIV;
+                else
+                    return FilterType.NOT_FILTERED;
+            }
+            return FilterType.PASSIV;
+        } else {
+            if (filtered)
+                return explicitlyFiltered ? FilterType.EXPLICIT : FilterType.PASSIV;
+            else
+                return FilterType.NOT_FILTERED;
+        }
+
+    }
+
+    /**
+     * Check if primitive is hidden.
+     * The filter flags for all parent objects must be set correctly, when
+     * calling this method.
+     * @param primitive the primitive
+     * @return FilterType.NOT_FILTERED when primitive is not hidden;
+     * FilterType.EXPLICIT when primitive is hidden and there is a non-inverted
+     * filter that applies;
+     * FilterType.PASSIV when primitive is hidden and all filters that apply
+     * are inverted
+     */
+    public FilterType isHidden(OsmPrimitive primitive) {
         return test(hiddenFilters, primitive, true);
     }
 
-    public boolean isDisabled(OsmPrimitive primitive) {
+    /**
+     * Check if primitive is disabled.
+     * The filter flags for all parent objects must be set correctly, when
+     * calling this method.
+     * @param primitive the primitive
+     * @return FilterType.NOT_FILTERED when primitive is not disabled;
+     * FilterType.EXPLICIT when primitive is disabled and there is a non-inverted
+     * filter that applies;
+     * FilterType.PASSIV when primitive is disabled and all filters that apply
+     * are inverted
+     */
+    public FilterType isDisabled(OsmPrimitive primitive) {
         return test(disabledFilters, primitive, false);
     }

trunk/src/org/openstreetmap/josm/data/osm/FilterWorker.java

@@ -3 +3 @@
 
 import java.util.Collection;
+import java.util.Collections;
+
+import org.openstreetmap.josm.data.osm.FilterMatcher.FilterType;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -15 +19 @@
      * be updated
      * @param filterMatcher the FilterMatcher
-     * @return true, if the filter state of any primitive has changed in the process
+     * @return true, if the filter state (normal / disabled / hidden)
+     * of any primitive has changed in the process
      */
     public static boolean executeFilters(Collection<OsmPrimitive> all, FilterMatcher filterMatcher) {
+        boolean changed = false;
+        // first relations, then ways and nodes last; this is required to resolve dependencies
+        changed = doExecuteFilters(Utils.filter(all, OsmPrimitive.relationPredicate), filterMatcher);
+        changed |= doExecuteFilters(Utils.filter(all, OsmPrimitive.wayPredicate), filterMatcher);
+        changed |= doExecuteFilters(Utils.filter(all, OsmPrimitive.nodePredicate), filterMatcher);
+        return changed;
+    }
+
+    private static boolean doExecuteFilters(Collection<OsmPrimitive> all, FilterMatcher filterMatcher) {
 
         boolean changed = false;
 
-        // First relation and ways
         for (OsmPrimitive primitive: all) {
-            if (!(primitive instanceof Node)) {
-                if (filterMatcher.isHidden(primitive)) {
-                    changed = changed | primitive.setDisabledState(true);
-                } else if (filterMatcher.isDisabled(primitive)) {
-                    changed = changed | primitive.setDisabledState(false);
+            FilterType hiddenType = filterMatcher.isHidden(primitive);
+            if (hiddenType != FilterType.NOT_FILTERED) {
+                changed |= primitive.setDisabledState(true);
+                primitive.setHiddenType(hiddenType == FilterType.EXPLICIT);
+            } else {
+                FilterType disabledType = filterMatcher.isDisabled(primitive);
+                if (disabledType != FilterType.NOT_FILTERED) {
+                    changed |= primitive.setDisabledState(false);
+                    primitive.setDisabledType(hiddenType == FilterType.EXPLICIT);
                 } else {
-                    changed = changed | primitive.unsetDisabledState();
+                    changed |= primitive.unsetDisabledState();
                 }
             }
         }
-
-        // Then nodes (because they state may depend on parent ways)
-        for (OsmPrimitive primitive: all) {
-            if (primitive instanceof Node) {
-                if (filterMatcher.isHidden(primitive)) {
-                    changed = changed | primitive.setDisabledState(true);
-                } else if (filterMatcher.isDisabled(primitive)) {
-                    changed = changed | primitive.setDisabledState(false);
-                } else {
-                    changed = changed | primitive.unsetDisabledState();
-                }
-            }
-        }
-
         return changed;
     }
 
     public static boolean executeFilters(OsmPrimitive primitive, FilterMatcher filterMatcher) {
-        boolean changed = false;
-        if (filterMatcher.isHidden(primitive)) {
-            changed = changed | primitive.setDisabledState(true);
-        } else if (filterMatcher.isDisabled(primitive)) {
-            changed = changed | primitive.setDisabledState(false);
-        } else {
-            changed = changed | primitive.unsetDisabledState();
-        }
-        return changed;
+        return doExecuteFilters(Collections.singleton(primitive), filterMatcher);
     }
 

trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -53 +53 @@
     /**
      * This flag is only relevant if an object is disabled by the
-     * filter mechanism (i.e. FLAG_DISABLED is set).
+     * filter mechanism (i.e. FLAG_DISABLED is set).
      * Then it indicates, whether it is completely hidden or
      * just shown in gray color.
@@ -61 +61 @@
      */
     protected static final int FLAG_HIDE_IF_DISABLED = 1 << 5;
+
+    /**
+     * Flag used internally by the filter mechanism.
+     */
+    protected static final int FLAG_DISABLED_TYPE = 1 << 6;
+
+    /**
+     * Flag used internally by the filter mechanism.
+     */
+    protected static final int FLAG_HIDDEN_TYPE = 1 << 7;
 
     /**
@@ -67 +77 @@
      * (e.g. one way street.)
      */
-    protected static final int FLAG_HAS_DIRECTIONS = 1 << 6;
+    protected static final int FLAG_HAS_DIRECTIONS = 1 << 8;
 
     /**
@@ -73 +83 @@
      * Some trivial tags like source=* are ignored here.
      */
-    protected static final int FLAG_TAGGED = 1 << 7;
+    protected static final int FLAG_TAGGED = 1 << 9;
 
     /**
@@ -80 +90 @@
      * (E.g. oneway=-1.)
      */
-    protected static final int FLAG_DIRECTION_REVERSED = 1 << 8;
+    protected static final int FLAG_DIRECTION_REVERSED = 1 << 10;
 
     /**
@@ -87 +97 @@
      * that the primitive is currently highlighted.
      */
-    protected static final int FLAG_HIGHLIGHTED = 1 << 9;
+    protected static final int FLAG_HIGHLIGHTED = 1 << 11;
 
     /**
@@ -431 +441 @@
      *
      * To enable the primitive again, use unsetDisabledState.
-     * @param hide if the primitive should be completely hidden from view or
+     * @param hidden if the primitive should be completely hidden from view or
      *             just shown in gray color.
      * @return true, any flag has changed; false if you try to set the disabled
      * state to the value that is already preset
      */
-    public boolean setDisabledState(boolean hide) {
+    public boolean setDisabledState(boolean hidden) {
         boolean locked = writeLock();
         try {
             int oldFlags = flags;
             updateFlagsNoLock(FLAG_DISABLED, true);
-            updateFlagsNoLock(FLAG_HIDE_IF_DISABLED, hide);
+            updateFlagsNoLock(FLAG_HIDE_IF_DISABLED, hidden);
             return oldFlags != flags;
         } finally {
@@ -465 +475 @@
 
     /**
+     * Set binary property used internally by the filter mechanism.
+     */
+    public void setDisabledType(boolean isExplicit) {
+        updateFlags(FLAG_DISABLED_TYPE, isExplicit);
+    }
+
+    /**
+     * Set binary property used internally by the filter mechanism.
+     */
+    public void setHiddenType(boolean isExplicit) {
+        updateFlags(FLAG_HIDDEN_TYPE, isExplicit);
+    }
+
+    /**
      * Replies true, if this primitive is disabled. (E.g. a filter
      * applies)
@@ -478 +502 @@
     public boolean isDisabledAndHidden() {
         return (((flags & FLAG_DISABLED) != 0) && ((flags & FLAG_HIDE_IF_DISABLED) != 0));
+    }
+
+    /**
+     * Get binary property used internally by the filter mechanism.
+     */
+    public boolean getHiddenType() {
+        return (flags & FLAG_HIDDEN_TYPE) != 0;
+    }
+
+    /**
+     * Get binary property used internally by the filter mechanism.
+     */
+    public boolean getDisabledType() {
+        return (flags & FLAG_DISABLED_TYPE) != 0;
     }