Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11496 closed defect (fixed)

[Patch] getVisibleLayersInZOrder() -> Comparator is not transitive

Reported by: michael2402 Owned by: team
Priority: minor Milestone: 15.08
Component: Core mappaint Version:
Keywords: gsoc Cc:

Description (last modified by Don-vip)

if (l1 instanceof OsmDataLayer && l2 instanceof OsmDataLayer) {
     if (l1 == getActiveLayer()) return -1;
     if (l2 == getActiveLayer()) return 1;
     return Integer.compare(layers.indexOf(l1), layers.indexOf(l2));
} else
     return Integer.compare(layers.indexOf(l1), layers.indexOf(l2));

The current comparator implementation is not transitive. Imagine the following layers:

layers = [OSM1, TMS1, OSM2], activeLayer = OSM1

We have OSM1 < TMS1, TMS1 < OSM2, OSM2 < OSM1.

What was the Idea behind the Comparator? My idea would be that it should push the avtive data layer above all adjacent OSM layers. So it would change this:

[OSM1, OSM2, OSM3, TMS1, OSM4], activeLayer = OSM2 to [OSM1, OSM3, OSM2, TMS1, OSM4]

Attachments (2)

0001-Fixed-11496.patch (3.7 KB) - added by michael2402 6 years ago.
0002-Fixed-a-Collections.reverse-that-should-not-be-there.patch (901 bytes) - added by michael2402 6 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 6 years ago by Don-vip

Description: modified (diff)

comment:2 in reply to:  description Changed 6 years ago by Don-vip

Replying to michael2402:

What was the Idea behind the Comparator?

That's very old stuff, it was done in r1943 to address #3228. Probably to paint GPX layers behind OSM data layers.

comment:3 Changed 6 years ago by michael2402

Obsolete with patch #11631, I added a new layer ordering code there.

comment:4 Changed 6 years ago by michael2402

Resolution: fixed
Status: newclosed

comment:5 Changed 6 years ago by bastiK

The code is unchanged, so not fixed.

comment:6 Changed 6 years ago by anonymous

Resolution: fixed
Status: closedreopened

Changed 6 years ago by michael2402

Attachment: 0001-Fixed-11496.patch added

comment:7 Changed 6 years ago by michael2402

This patch should fix the issue. The idea of the comparator - to move the selected data layer to the front but not above e.g. gpx layers - is respected.

comment:8 Changed 6 years ago by michael2402

Summary: getVisibleLayersInZOrder() -> Comparator is not transitive[Patch] getVisibleLayersInZOrder() -> Comparator is not transitive

comment:9 Changed 6 years ago by Don-vip

Keywords: gsoc added
Milestone: 15.07

comment:10 Changed 6 years ago by bastiK

Resolution: fixed
Status: reopenedclosed

In 8613/josm:

fixed #11496 - getVisibleLayersInZOrder() -> Comparator is not transitive (patch by michael2402)

comment:11 Changed 6 years ago by bastiK

Thanks!

comment:12 Changed 6 years ago by wiktorn

Resolution: fixed
Status: closedreopened

Isn't this problem related to this patch, that my fresh custom build show imagery layer (WMS) above data layer, although in the layer list it's the opposite? When I moved down the data layer, it was showed on the top of WMS imagery layer.

comment:13 Changed 6 years ago by michael2402

I missed a Collections.reverse(ret); when preparing the patch... It should not be there.

comment:14 Changed 6 years ago by wiktorn

Ticket #11712 has been marked as a duplicate of this ticket.

comment:15 Changed 6 years ago by bastiK

Resolution: fixed
Status: reopenedclosed

In 8615/josm:

fixed a Collections.reverse that should not be there (fixes #11496, patch by michael2402)

comment:16 Changed 6 years ago by Klumbumbus

Ticket #11717 has been marked as a duplicate of this ticket.

comment:17 Changed 6 years ago by Don-vip

Milestone: 15.0715.08

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.