Changeset 18439 in josm
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModel.java
r18401 r18439 308 308 final ListSelectionModel selectionModel = getSelectionModel(); 309 309 selectionModel.setValueIsAdjusting(true); 310 for (int row : selectedRows) {311 row -= offset;312 if (members.size() > row) {313 members. remove(row);314 selectionModel.removeIndexInterval(row , row);315 offset ++;310 for (int[] row : groupRows(selectedRows)) { 311 if (members.size() > row[0] - offset) { 312 // Remove (inclusive) 313 members.subList(row[0] - offset, row[1] - offset + 1).clear(); 314 selectionModel.removeIndexInterval(row[0] - offset, row[1] - offset); 315 offset += row[1] - row[0] + 1; 316 316 } 317 317 } 318 318 selectionModel.setValueIsAdjusting(false); 319 319 fireTableDataChanged(); 320 } 321 322 /** 323 * Group rows for use in changing selection intervals, to avoid many small calls on large selections 324 * @param rows The rows to group 325 * @return A list of grouped rows, [lower, higher] (inclusive) 326 */ 327 private static List<int[]> groupRows(int... rows) { 328 if (rows.length == 0) { 329 return Collections.emptyList(); 330 } 331 List<int[]> groups = new ArrayList<>(); 332 int[] current = {Integer.MIN_VALUE, Integer.MIN_VALUE}; 333 groups.add(current); 334 for (int row : rows) { 335 if (current[0] == Integer.MIN_VALUE) { 336 current[0] = row; 337 current[1] = row; 338 continue; 339 } 340 if (current[1] == row - 1) { 341 current[1] = row; 342 } else { 343 current = new int[] {row, row}; 344 groups.add(current); 345 } 346 } 347 return Collections.unmodifiableList(groups); 320 348 } 321 349 … … 445 473 void addMembersAtIndexKeepingOldSelection(final Iterable<RelationMember> newMembers, final int index) { 446 474 int idx = index; 475 // Avoid having the inserted rows from being part of the selection. See JOSM #12617, #17906, #21889. 476 int[] originalSelection = null; 477 if (selectedIndices().anyMatch(selectedRow -> selectedRow == index)) { 478 originalSelection = getSelectedIndices(); 479 } 447 480 for (RelationMember member : newMembers) { 448 481 members.add(idx++, member); … … 450 483 invalidateConnectionType(); 451 484 fireTableRowsInserted(index, idx - 1); 485 if (originalSelection != null) { 486 final DefaultListSelectionModel model = this.getSelectionModel(); 487 model.setValueIsAdjusting(true); 488 model.clearSelection(); 489 final int tIdx = idx; 490 // Avoiding many addSelectionInterval calls is critical for performance. 491 for (int[] row : groupRows(IntStream.of(originalSelection).map(i -> i < index ? i : i + tIdx - index).toArray())) { 492 model.addSelectionInterval(row[0], row[1]); 493 } 494 model.setValueIsAdjusting(false); 495 } 452 496 } 453 497 -
trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModelTest.java
r18037 r18439 2 2 package org.openstreetmap.josm.gui.dialogs.relation; 3 3 4 import static org.junit.jupiter.api.Assertions.assertAll; 5 import static org.junit.jupiter.api.Assertions.assertEquals; 4 6 import static org.junit.jupiter.api.Assertions.assertNotNull; 5 7 8 import java.util.Arrays; 6 9 import java.util.Collection; 7 10 import java.util.Collections; 8 11 import java.util.List; 12 import java.util.stream.Stream; 9 13 14 import org.junit.jupiter.api.Test; 15 import org.openstreetmap.josm.TestUtils; 16 import org.openstreetmap.josm.data.coor.LatLon; 17 import org.openstreetmap.josm.data.osm.DataSet; 10 18 import org.openstreetmap.josm.data.osm.Node; 11 19 import org.openstreetmap.josm.data.osm.OsmPrimitive; 20 import org.openstreetmap.josm.data.osm.Relation; 21 import org.openstreetmap.josm.data.osm.RelationMember; 12 22 import org.openstreetmap.josm.data.osm.Tag; 23 import org.openstreetmap.josm.gui.layer.OsmDataLayer; 13 24 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetHandler; 14 25 import org.openstreetmap.josm.testutils.annotations.BasicPreferences; 15 16 import org.junit.jupiter.api.Test;17 26 18 27 /** … … 35 44 @Override 36 45 public Collection<OsmPrimitive> getSelection() { 37 return Collections. <OsmPrimitive>singleton(n);46 return Collections.singleton(n); 38 47 } 39 48 }).getRelationMemberForPrimitive(n)); 40 49 } 50 51 /** 52 * Non-regression test for JOSM #12617, #17906, and #21889. Regrettably, it was not easily possible to test using 53 * drag-n-drop methods. 54 */ 55 @Test 56 void testTicket12617() { 57 final Node[] nodes = new Node[10]; 58 for (int i = 0; i < nodes.length; i++) { 59 nodes[i] = new Node(LatLon.ZERO); 60 // The id has to be > 0 61 nodes[i].setOsmId(i + 1, 1); 62 } 63 final Relation relation = TestUtils.newRelation("", Stream.of(nodes).map(node -> new RelationMember("", node)) 64 .toArray(RelationMember[]::new)); 65 final OsmDataLayer osmDataLayer = new OsmDataLayer(new DataSet(), "testTicket12617", null); 66 osmDataLayer.getDataSet().addPrimitiveRecursive(relation); 67 final MemberTableModel model = new MemberTableModel(relation, osmDataLayer, new TaggingPresetHandler() { 68 @Override 69 public Collection<OsmPrimitive> getSelection() { 70 return Collections.singleton(relation); 71 } 72 73 @Override 74 public void updateTags(List<Tag> tags) { 75 // Do nothing 76 } 77 }); 78 79 model.populate(relation); 80 // Select the members to move 81 model.setSelectedMembersIdx(Arrays.asList(2, 3, 5, 9)); 82 // Move the members (this is similar to what the drag-n-drop code is doing) 83 model.addMembersAtIndexKeepingOldSelection(model.getSelectedMembers(), 2); 84 model.remove(model.getSelectedIndices()); 85 // Apply the changes 86 model.applyToRelation(relation); 87 88 // Perform the tests 89 assertAll(() -> assertEquals(10, relation.getMembersCount(), "There should be no changes to the member count"), 90 () -> assertEquals(nodes[0], relation.getMember(0).getMember()), 91 () -> assertEquals(nodes[1], relation.getMember(1).getMember()), 92 () -> assertEquals(nodes[2], relation.getMember(2).getMember(), "Node 2 should not have moved"), 93 () -> assertEquals(nodes[3], relation.getMember(3).getMember(), "Node 3 should not have moved"), 94 () -> assertEquals(nodes[4], relation.getMember(6).getMember(), "Node 4 should be in position 5"), 95 () -> assertEquals(nodes[5], relation.getMember(4).getMember(), "Node 5 should be in position 4"), 96 () -> assertEquals(nodes[6], relation.getMember(7).getMember(), "Node 6 should not have moved"), 97 () -> assertEquals(nodes[7], relation.getMember(8).getMember()), 98 () -> assertEquals(nodes[8], relation.getMember(9).getMember()), 99 () -> assertEquals(nodes[9], relation.getMember(5).getMember(), "Node 9 should have moved") 100 ); 101 } 41 102 }
Note:
See TracChangeset
for help on using the changeset viewer.