From 747b77d34b304c193477e3609edf700255bfe8de Mon Sep 17 00:00:00 2001 From: Simon Poole Date: Sat, 30 Sep 2023 10:41:49 +0200 Subject: [PATCH 1/2] Include relations with appropriate level tags Fixes https://github.com/MarcusWolschon/osmeditor4android/issues/738 --- .../de/blau/android/filter/IndoorFilter.java | 84 +++++++++++++------ 1 file changed, 58 insertions(+), 26 deletions(-) diff --git a/src/main/java/de/blau/android/filter/IndoorFilter.java b/src/main/java/de/blau/android/filter/IndoorFilter.java index 249ee3745a..dfbbc2b1fa 100644 --- a/src/main/java/de/blau/android/filter/IndoorFilter.java +++ b/src/main/java/de/blau/android/filter/IndoorFilter.java @@ -14,6 +14,7 @@ import android.widget.RelativeLayout; import android.widget.RelativeLayout.LayoutParams; import android.widget.TextView; +import androidx.annotation.NonNull; import de.blau.android.App; import de.blau.android.Main; import de.blau.android.R; @@ -44,7 +45,7 @@ public class IndoorFilter extends InvertableFilter { /** * Current level */ - private int level = 0; + private int level = 0; /** * Construct a new instance of IndoorFilter @@ -60,12 +61,9 @@ public boolean include(Node node, boolean selected) { return include != Include.DONT; } if (!inverted) { - include = (selected - || (node.hasTags() && (contains(node.getTagWithKey(Tags.KEY_LEVEL), level) || contains(node.getTagWithKey(Tags.KEY_REPEAT_ON), level)))) - ? Include.INCLUDE - : Include.DONT; + include = (selected || (node.hasTags() && onLevel(node, level))) ? Include.INCLUDE : Include.DONT; } else { - include = (selected || (node.hasTags() && !node.hasTagKey(Tags.KEY_LEVEL) && !node.hasTagKey(Tags.KEY_REPEAT_ON))) ? Include.INCLUDE : Include.DONT; + include = selected || (node.hasTags() && notIndoor(node)) ? Include.INCLUDE : Include.DONT; } if (include == Include.DONT) { @@ -85,6 +83,30 @@ public boolean include(Node node, boolean selected) { return include != Include.DONT; } + /** + * Check if an OsmElement is on a specific level + * + * @param e + * the OsmElement + * @param level + * the level + * @return true if the object is on the level + */ + private boolean onLevel(@NonNull OsmElement e, int level) { + return contains(e.getTagWithKey(Tags.KEY_LEVEL), level) || contains(e.getTagWithKey(Tags.KEY_REPEAT_ON), level); + } + + /** + * Check if OsmElement has any of the conventional indoor level tags + * + * @param e + * the OsmElement + * @return true if no level tags + */ + private boolean notIndoor(@NonNull OsmElement e) { + return !e.hasTagKey(Tags.KEY_LEVEL) && !e.hasTagKey(Tags.KEY_REPEAT_ON); + } + @Override public boolean include(Way way, boolean selected) { Include include = cachedWays.get(way); @@ -92,11 +114,13 @@ public boolean include(Way way, boolean selected) { return include != Include.DONT; } if (!inverted) { - include = (selected || (way.hasTags() && (contains(way.getTagWithKey(Tags.KEY_LEVEL), level) - || contains(way.getTagWithKey(Tags.KEY_REPEAT_ON), level) || buildingHasLevel(way, level)))) ? Include.INCLUDE : Include.DONT; + include = (selected || (way.hasTags() && (onLevel(way, level) || buildingHasLevel(way, level)))) + ? Include.INCLUDE + : Include.DONT; } else { - include = (selected || (way.hasTags() && !way.hasTagKey(Tags.KEY_LEVEL) && !way.hasTagKey(Tags.KEY_REPEAT_ON) - && !(way.hasTagKey(Tags.KEY_MIN_LEVEL) || way.hasTagKey(Tags.KEY_MAX_LEVEL)))) ? Include.INCLUDE : Include.DONT; + include = (selected || (way.hasTags() && notIndoor(way) + && !(way.hasTagKey(Tags.KEY_MIN_LEVEL) || way.hasTagKey(Tags.KEY_MAX_LEVEL)))) ? Include.INCLUDE + : Include.DONT; } if (include == Include.DONT) { @@ -136,11 +160,10 @@ public boolean include(Relation relation, boolean selected) { return include != Include.DONT; } if (!inverted) { - include = (selected || buildingHasLevel(relation, level)) ? Include.INCLUDE : Include.DONT; - } else { - include = (selected || (relation.hasTags() && !(relation.hasTagKey(Tags.KEY_MIN_LEVEL) || relation.hasTagKey(Tags.KEY_MAX_LEVEL)))) - ? Include.INCLUDE + include = (selected || onLevel(relation, level) || buildingHasLevel(relation, level)) ? Include.INCLUDE : Include.DONT; + } else { + include = (selected || (relation.hasTags() && notIndoor(relation))) ? Include.INCLUDE : Include.DONT; } cachedRelations.put(relation, include); @@ -187,8 +210,10 @@ public boolean include(Relation relation, boolean selected) { /** * Check if a specific level is included in a level spec * - * @param levelSpec either a single integer, a semi-colon separated list, or a range - * @param level level we are interested in + * @param levelSpec + * either a single integer, a semi-colon separated list, or a range + * @param level + * level we are interested in * @return true if the level is contained in levelSpec */ private boolean contains(String levelSpec, int level) { @@ -235,11 +260,12 @@ private boolean contains(String levelSpec, int level) { } /** - * Check if a object is a building or building:part, has min_level and max_level keys and level is between the min - * and max + * Check if a object is a building or building:part, has min_level and max_level keys and level is between the min and max * - * @param b the OsmElement - * @param level our current level + * @param b + * the OsmElement + * @param level + * our current level * @return true if the building/building:part has a level between (inclusive) min/max */ private static boolean buildingHasLevel(OsmElement b, int level) { @@ -267,7 +293,8 @@ public int getLevel() { /** * Set level used in indoor mode * - * @param level the level to set + * @param level + * the level to set */ public void setLevel(int level) { if (level != this.level) { @@ -307,7 +334,8 @@ public void addControls(ViewGroup layout, final Update update) { Preferences prefs = App.getPreferences(context); LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE); controls = (RelativeLayout) inflater - .inflate("LEFT".equals(prefs.followGPSbuttonPosition()) ? R.layout.indoor_controls_right : R.layout.indoor_controls_left, layout); + .inflate("LEFT".equals(prefs.followGPSbuttonPosition()) ? R.layout.indoor_controls_right + : R.layout.indoor_controls_left, layout); levelUp = (FloatingActionButton) controls.findViewById(R.id.levelUp); levelDisplay = (FrameLayout) controls.findViewById(R.id.level); levelText = (TextView) controls.findViewById(R.id.levelText); @@ -355,7 +383,8 @@ public void addControls(ViewGroup layout, final Update update) { /** * Setup the up and down buttons and the level display * - * @param toggle if true toggle between inverted and normal filter mode + * @param toggle + * if true toggle between inverted and normal filter mode */ private void setupControls(boolean toggle) { if (toggle) { @@ -366,7 +395,8 @@ private void setupControls(boolean toggle) { levelText.setText("--"); levelUp.setEnabled(false); levelDown.setEnabled(false); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && disabledLayoutParamsUp != null && disabledLayoutParamsDown != null) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && disabledLayoutParamsUp != null + && disabledLayoutParamsDown != null) { levelUp.setLayoutParams(disabledLayoutParamsUp); levelDown.setLayoutParams(disabledLayoutParamsDown); } @@ -374,7 +404,8 @@ private void setupControls(boolean toggle) { updateLevel(level); levelUp.setEnabled(true); levelDown.setEnabled(true); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && originalLayoutParamsUp != null && originalLayoutParamsDown != null) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && originalLayoutParamsUp != null + && originalLayoutParamsDown != null) { levelUp.setLayoutParams(originalLayoutParamsUp); levelDown.setLayoutParams(originalLayoutParamsDown); } @@ -420,7 +451,8 @@ public void showControls() { /** * Update the displayed level * - * @param level the level to show on the button + * @param level + * the level to show on the button */ private void updateLevel(int level) { Log.d(DEBUG_TAG, "setting level to " + level); From be8959b7593ba595acc15e0eb7a219db64e76c69 Mon Sep 17 00:00:00 2001 From: Simon Poole Date: Sat, 30 Sep 2023 11:23:05 +0200 Subject: [PATCH 2/2] Add test for normal indoor filtering for Relations --- .../blau/android/filter/IndoorFilterTest.java | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/test/java/de/blau/android/filter/IndoorFilterTest.java b/src/test/java/de/blau/android/filter/IndoorFilterTest.java index 3f22cf352d..85a7a9675b 100644 --- a/src/test/java/de/blau/android/filter/IndoorFilterTest.java +++ b/src/test/java/de/blau/android/filter/IndoorFilterTest.java @@ -181,10 +181,10 @@ public void indoorFilterWayInverted() { } /** - * Test if a relation is filtered correctly (without UI) + * Test if a building relation is filtered correctly (without UI) */ @Test - public void indoorFilterRelation() { + public void indoorFilterBuildingRelation() { try { TreeMap tags = new TreeMap<>(); Logic logic = App.getLogic(); @@ -209,7 +209,7 @@ public void indoorFilterRelation() { Relation r = logic.createRelation(null, "", members); f.clear(); f.setLevel(1); - // check that relation without level doesn't change member status + // check that relation without level does change member status Assert.assertFalse(f.include(r, false)); Assert.assertFalse(f.include(w, false)); Assert.assertTrue(f.include(w2, false)); @@ -227,4 +227,48 @@ public void indoorFilterRelation() { Assert.fail(e.getMessage()); } } + + /** + * Test if a relation is filtered correctly (without UI) + */ + @Test + public void indoorFilterRelation() { + try { + TreeMap tags = new TreeMap<>(); + Logic logic = App.getLogic(); + + logic.performAdd(null, 100.0f, 100.0f); + logic.performAdd(null, 1000.0f, 1000.0f); + Way w = logic.getSelectedWay(); + logic.setSelectedNode(null); + logic.setSelectedWay(null); + logic.performAdd(null, 100.0f, 400.0f); + logic.performAdd(null, 1000.0f, 1000.0f); + Way w2 = logic.getSelectedWay(); + logic.setSelectedNode(null); + logic.setSelectedWay(null); + tags.put(Tags.KEY_LEVEL, "" + 1); + logic.setTags(null, w2, tags); + + IndoorFilter f = new IndoorFilter(); + List members = new ArrayList<>(); + members.add(w); + members.add(w2); + Relation r = logic.createRelation(null, "", members); + f.clear(); + f.setLevel(1); + + // now check relation and inheritance from relation + tags.clear(); + tags.put(Tags.KEY_LEVEL, "" + 1); + logic.setTags(null, r, tags); + Assert.assertTrue(f.include(r, true)); + Assert.assertTrue(f.include(w, true)); + f.setInverted(true); + Assert.assertTrue(f.include(r, false)); + Assert.assertTrue(f.include(w, false)); + } catch (OsmIllegalOperationException e) { + Assert.fail(e.getMessage()); + } + } }