Modify

Opened 19 months ago

Closed 4 weeks ago

#14120 closed defect (fixed)

Remove methods deprecated during gsoc-core

Reported by: Don-vip Owned by: michael2402
Priority: normal Milestone: 18.06
Component: Core Version:
Keywords: gsoc-core Cc: bastiK

Description (last modified by Don-vip)

Deprecated features from gsoc-core to remove, which are still used in core or plugins:

  • LatLon.heading(): used in 5 plugins: improveway, iodb, nanolog, photoadjust, rex, ​SimplifyArea
  • Preferences.getColor(): used in MapCSSParser + plugins: utilsplugin2, commandLine, routing, graphview
  • Layer.isChanged(): used in MapView, GpxLayer, AbstractTileSourceLayer + plugin osminspector
  • ImageryLayer.getDx()/getDy()/setOffset()/displace(): used in ApplyOffsetAction, ImageryLayer + 4 plugins: commandLine, imageryadjust, iodb, irsrectify

Attachments (0)

Change History (70)

comment:1 Changed 19 months ago by Don-vip

Milestone: 16.1217.01

comment:2 Changed 18 months ago by michael2402

Milestone: 17.0117.03

I don't have much time right now and won't have until March 31. I would like to re-schedule this one for milestone 17.4 (I just put it to 17.3, since there is no 17.4 yet).

comment:3 Changed 18 months ago by Don-vip

Milestone: 17.0317.04

ok thanks for the heads up :)

comment:4 Changed 17 months ago by stoecker

In 11623/josm:

see #14120 - fix deprecation in core

comment:5 Changed 17 months ago by Don-vip

In 11631/josm:

see #14120 - update to JavaCC 7.0.2 - patched to avoid error-prone compilation warnings by removing everything related to ReInit methods in ParseGen.java and SimpleCharStream.template

comment:6 Changed 17 months ago by Don-vip

The only warnings in core now concern Layer.isChanged().

comment:7 Changed 16 months ago by stoecker

@michael2402:

Can you please give fixing the deprecations highest priority? We need to get a clean state, before we start other deprecations.

comment:8 Changed 16 months ago by michael2402

Status: newassigned

comment:9 Changed 15 months ago by Don-vip

Type: enhancementdefect

comment:10 Changed 15 months ago by Don-vip

14.04 is due in two weeks, can you please fix the core deprecation warnings before that?

comment:11 Changed 15 months ago by Don-vip

I think I have updated all the plugins now. But it seems we still have a bug in core:

  • #13503: IAE: Cannot paint layer, it is not registered (TMSLayer)
  • #13604: IAE: Cannot paint layer, it is not registered (OsmDataLayer)
  • #13886: IAE: Cannot paint layer, it is not registered (ValidatorLayer)
  • #14234: IAE: Cannot paint layer, it is not registered (GeoImageLayer)
  • #13589: IAE: Cannot paint layer, it is not registered (ElevationProfileLayer)

comment:12 Changed 15 months ago by Don-vip

One week left before 14.04. Because of #14604 impacting lots of users, and with #7427 being fixed, the release date this month will not shift.

comment:13 Changed 15 months ago by michael2402

I've been busy the last week, but I have lots of time this one.

comment:14 Changed 15 months ago by michael2402

In 12014/josm:

See #14120: Use a listener to get notified of way segment / virtual node highlight changes.

comment:15 Changed 15 months ago by Don-vip

Two unit tests are failing.

comment:16 Changed 15 months ago by michael2402

I already saw that. Some problem with the test framework removing a layer twice, i'm debugging it but tests always take some time to run ;-)

comment:17 Changed 15 months ago by Don-vip

ok thanks :) I tried to fix them but didn't understand the error.

comment:18 Changed 15 months ago by Don-vip

In 12020/josm:

see #14120 - checkstyle

comment:19 Changed 15 months ago by michael2402

In 12023/josm:

See #14120: Do not overwrite the mapView field for unit tests. Fix map view scale/center.

comment:20 Changed 15 months ago by michael2402

In 12024/josm:

See #14120: Fix unit tests by using a separate layer manager when mocking the map view.

comment:21 Changed 15 months ago by michael2402

In 12025/josm:

See #14120: Throw a nicer exception if a layer is destroyed twice.

comment:22 Changed 15 months ago by michael2402

I found the problem:

The map view handles the layer lifecycle. If tests create multiple map views for the same layer, each map view destroys the layers during test exit - so we have several destroy() calls. I removed one of the places where MapViewMock is used. We could completely remove that Mock, I think.

comment:23 Changed 15 months ago by Don-vip

In 12028/josm:

see #14120 - fix java warnings

comment:24 Changed 15 months ago by michael2402

Description: modified (diff)
Milestone: 17.0417.05

comment:25 Changed 15 months ago by Don-vip

there are still 10 warnings to fix.

comment:26 Changed 15 months ago by Don-vip

Also 6 unit tests are failing for 23/26 builds.

comment:27 Changed 15 months ago by michael2402

I've seen them. The most important thing for me is getting the warnings safely away until next release. They are caused by a selection change event that was used to trigger a map view repaint in most cases. There is a reason why someone added those calls back then, so I'm going trough the UI code and fixing them one by one (or check if it is already fixed by some other mechanism that triggers a repaint). And I'm fixing some other repaint/layout related issues on the way.

The AlignInLineActionTest(s) are caused by the selection not being sorted for a single addSelection(). I tried around a bit and it seems the LinkedHashSet order gets lost in the streams, even when using a LinkedHashSet as collector. Have to look into it more. I was not aware that the selection order is used that much.
If users select the points, multiple addSelection calls will be issued. So for now, this is only a problem if a plugin uses this functionality.

I'll probably set up travis/git again next week, so that I will be able to run the tests in background more easily.

After the selection stuff is done, I already have a patch for the GPX layer that removes the update counting code. That way, we can get rid of the isChanged() methods and rely on layer invalidation events in the future. This should allow us to remove most of the MapView.repaint() calls in code.

comment:28 Changed 15 months ago by Don-vip

OK thanks :)

comment:29 in reply to:  27 Changed 15 months ago by Klumbumbus

Replying to michael2402:

I was not aware that the selection order is used that much.
...
So for now, this is only a problem if a plugin uses this functionality.

(Just in case you didn't know: When creating turn restrictions with the turnrestrictions plugin, the selection order is required as the roles are set automatically depending on the selction order.)

comment:30 Changed 15 months ago by Don-vip

Cc: bastiK added

comment:31 Changed 15 months ago by michael2402

In 12103/josm:

See #14120: MapView should repaint on temporary layer changes automatically.

comment:32 Changed 14 months ago by bastiK

Only one failed unit test remaining...

comment:33 Changed 14 months ago by michael2402

In 12156/josm:

See #14120: Don't make gpx tracks depend on the isChanged method, use a listener based approach instead.

comment:34 Changed 14 months ago by michael2402

In 12170/josm:

"See #13175, see #14120: Remove isChanged() method call in map view - all layers should invalidate correctly now."

comment:35 Changed 14 months ago by Don-vip

Only one test to fix and we won't get spammed by Jenkins at each build anymore: MapCSSRendererTest.testRender[order]. After that I expect the build to be stable a longer time. A test failing for 91 builds is a lot of noise :(

comment:36 Changed 14 months ago by michael2402

I did not know you get a mail for every test run...

But now they are all fixed. There a still a a few deprecations to track down, but then everything should be stabilized for the next release.

comment:37 Changed 14 months ago by michael2402

Milestone: 17.0517.06

I don't want to remove the remaining selection changed calls now since I did not fully test if/where they are required.

comment:38 Changed 13 months ago by Don-vip

only one java warning left.

comment:39 Changed 13 months ago by Don-vip

Milestone: 17.0617.07

comment:40 Changed 12 months ago by Don-vip

Milestone: 17.0717.08

comment:41 Changed 11 months ago by bastiK

Milestone: 17.0817.09

comment:42 Changed 10 months ago by Don-vip

Michael can you please fix the last deprecation warning?

comment:43 Changed 10 months ago by michael2402

I'll have a look at it.

The line can simply be removed in theory, the hard part is to check if all UI components are actually redrawn correctly if commands are executed.

comment:44 Changed 10 months ago by Don-vip

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

comment:45 Changed 10 months ago by Don-vip

Milestone: 17.0917.10

comment:46 Changed 9 months ago by bastiK

In 13010/josm:

see #14120 - fix use of deprecated API (selection handling)

comment:47 in reply to:  43 ; Changed 9 months ago by bastiK

Replying to michael2402:

The line can simply be removed in theory, the hard part is to check if all UI components are actually redrawn correctly if commands are executed.

Looks like it is working. Why is the private method in DataSet named fireDeprecatedSelectionChange?

comment:48 Changed 9 months ago by bastiK

Description: modified (diff)

comment:49 in reply to:  47 Changed 9 months ago by michael2402

Replying to bastiK:

Why is the private method in DataSet named fireDeprecatedSelectionChange?

Because I wanted to deprecated the old selection listener mechanism first - but it was used in too many places so that I could not do this on the inital patch. And then I forgot about the method name...

See #13467

Last edited 9 months ago by michael2402 (previous) (diff)

comment:50 Changed 9 months ago by Don-vip

Milestone: 17.1017.11

comment:51 Changed 8 months ago by Don-vip

Milestone: 17.1117.12

comment:52 Changed 8 months ago by Don-vip

@team: I'm not sure how to update last plugins regarding deprecation of heading()/getDx()/getDy() I'd like to get some help.

comment:53 Changed 7 months ago by Don-vip

Milestone: 17.1218.01

comment:54 Changed 6 months ago by Don-vip

Milestone: 18.0118.02

Help is still welcome.

comment:55 Changed 5 months ago by Don-vip

Milestone: 18.0218.03

comment:56 Changed 4 months ago by Don-vip

Milestone: 18.0318.04

comment:57 Changed 3 months ago by Don-vip

Milestone: 18.0418.05

comment:58 Changed 8 weeks ago by Don-vip

Milestone: 18.0518.06

comment:59 Changed 4 weeks ago by Don-vip

Description: modified (diff)

comment:60 Changed 4 weeks ago by Don-vip

iodb updated in [o34337:34338]

comment:61 Changed 4 weeks ago by Don-vip

nanolog updated in [o34339:34340].

comment:62 Changed 4 weeks ago by Don-vip

Description: modified (diff)

comment:64 Changed 4 weeks ago by Don-vip

In 13966/josm:

see #14120, see #12524 - remove deprecated method LatLon.heading()

comment:65 Changed 4 weeks ago by Don-vip

Description: modified (diff)

comment:66 Changed 4 weeks ago by Don-vip

Description: modified (diff)

comment:67 Changed 4 weeks ago by Don-vip

Description: modified (diff)

comment:68 Changed 4 weeks ago by Don-vip

irsrectify plugin deleted in [o34341].

comment:69 Changed 4 weeks ago by Don-vip

commandline updated in [o34342:34343].

comment:70 Changed 4 weeks ago by Don-vip

Resolution: fixed
Status: assignedclosed

In 13967/josm:

fix #14120 - remove deprecated methods ImageryLayer.getDx()/getDy()/setOffset()

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain michael2402.
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.