Caching and notifying preferences

[michael2402]

Currently, there are many MapFrame.repaint() calls whenever properties change.

Part of this is because colors and other values are cached by each component seperately.

I added a new infrastructure that allows you to get a property and easily cache that result.

To get a property use:

new IntegerProperty("my.property", "123").get()

To cache the result, use:

prop = new IntegerProperty("my.property", "123").cached(); // registers 123 as default value
prop.get(); // lazy get and convert to int
prop.get(); // get cached integer object

I added a new preference listener that listens to changes of one specific key to make this efficient.

[Don-vip]

Patch does not apply anymore, can you please update it?

[Don-vip]

    public static class ValueChangeEvent<T> {
        private final PreferenceChangeEvent base;

        private final AbstractProperty<T> source;

        ValueChangeEvent(PreferenceChangeEvent base, AbstractProperty<T> source) {
            this.base = base;
            this.source = source;
        }

base is not used?

[Don-vip]

Why is the timeout commented in JOSMTestRules?

           //  statement = new FailOnTimeoutStatement(statement, timeout);

[Don-vip]

In 10824/josm:

see #13309 - Caching and notifying preferences (patch by michael2402) - gsoc-core

[michael2402]

Thanks for finding that timeout. No, it should not be commented. I did this for debugging.

[Don-vip]

ok this is fixed, I didn't apply this part.

[Don-vip]

this is not, this patch cause 74 unit test failures: jenkins/job/JOSM/1725/testReport/

[Don-vip]

In 10839/josm:

see #13309 - remove deprecated color stuff not used outside of JOSM

[Don-vip]

In 10853/josm:

see #13309 - fix most of deprecation warnings

[Don-vip]

it seems the exception causing all these test failures is

java.lang.NullPointerException
	at org.openstreetmap.josm.data.preferences.AbstractToStringProperty.getAsString(AbstractToStringProperty.java:131)
	at org.openstreetmap.josm.data.preferences.AbstractToStringProperty.get(AbstractToStringProperty.java:92)
	at org.openstreetmap.josm.data.preferences.BooleanProperty.get(BooleanProperty.java:21)
	at org.openstreetmap.josm.tools.date.DateUtils.getDateTimeFormat(DateUtils.java:311)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog$NoteRenderer.<init>(NotesDialog.java:209)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog$NoteRenderer.<init>(NotesDialog.java:206)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog.buildDialog(NotesDialog.java:85)
	at org.openstreetmap.josm.gui.dialogs.NotesDialog.<init>(NotesDialog.java:78)
	at org.openstreetmap.josm.gui.MapFrame.<init>(MapFrame.java:264)
	at org.openstreetmap.josm.gui.MainPanel.createNewMapFrame(MainPanel.java:88)
	at org.openstreetmap.josm.gui.MainPanel.updateContent(MainPanel.java:63)
	at org.openstreetmap.josm.gui.MainPanel$1.beforeFirstLayerAdded(MainPanel.java:157)
	at org.openstreetmap.josm.gui.layer.MainLayerManager.realAddLayer(MainLayerManager.java:264)
	at org.openstreetmap.josm.gui.layer.LayerManager.lambda$addLayer$0(LayerManager.java:180)
	at org.openstreetmap.josm.gui.util.GuiHelper.runInEDTAndWaitWithException(GuiHelper.java:138)
	at org.openstreetmap.josm.gui.layer.LayerManager.addLayer(LayerManager.java:180)
	at org.openstreetmap.josm.JOSMFixture.setupGUI(JOSMFixture.java:154)
	at org.openstreetmap.josm.JOSMFixture.access$000(JOSMFixture.java:28)
	at org.openstreetmap.josm.JOSMFixture$1.run(JOSMFixture.java:128)

[michael2402]

The problem is that some class loads DateUtils before Main.pref is instantiated.

I suggest fixing this and adding a dependency on preferences for that test. I'll find the test that causes this and post a patch for it.

An alternative to make our life easier would be to make Main.pref a final field instantiated with an empty instance. We can then simply load it during program startup. This would also make early application startup much easier, since we currently have many classes that may only be loaded after Main.pref is set. We should make those classes listen to preferences updates though, but this is why I created this patch.

[michael2402]

Please ignore patch-latlon-generic.patch​ and patch-fix-13309-2.2.patch​. This happens if you have 4 track windows open because it has a load time of 2 Minutes (if it loads)

I attached patch-fix-13309-2.patch​ to address the issue. It makes the pref field final. That field was only changed in tests and should never be changed while JOSM is running, so it should not be a problem.

[Don-vip]

In 10876/josm:

fix #13309 - fix unit tests (patch by michael2402) - gsoc-core

[Don-vip]

Much better, still 9 tests to fix:

1. org.openstreetmap.josm.gui.dialogs.InspectPrimitiveDialogTest.testBuildMapPaintText
2. org.openstreetmap.josm.data.PreferencesTest.testColorNameAlpha
3. org.openstreetmap.josm.data.PreferencesTest.testColorAlpha
4. org.openstreetmap.josm.gui.JosmUserIdentityManagerTest.test_SingletonAccess
5. org.openstreetmap.josm.gui.JosmUserIdentityManagerTest.userNameChanged
6. org.openstreetmap.josm.gui.JosmUserIdentityManagerTest.apiUrlChanged
7. org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSParserTest.testColorNameTicket9191
8. org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSParserTest.testColorNameTicket9191Alpha
9. org.openstreetmap.josm.tools.bugreport.BugReportExceptionHandlerTest.testHandleException

[Don-vip]

Replying to michael2402:

because it has a load time of 2 Minutes (if it loads)

website was targeted by what seems to be a DDoS, I found a solution, it is now much faster.

[Don-vip]

Deprecation warnings must also be fixed:

PropertiesCellRenderer.java:37, Java Compiler (javac), Priority: Normale
getColor(String,Color) in Preferences has been deprecated

PropertiesCellRenderer.java:38, Java Compiler (javac), Priority: Normale
getColor(String,Color) in Preferences has been deprecated

PropertiesCellRenderer.java:41, Java Compiler (javac), Priority: Normale
getColor(String,Color) in Preferences has been deprecated

PropertiesCellRenderer.java:42, Java Compiler (javac), Priority: Normale
getColor(String,Color) in Preferences has been deprecated

MapCSSParser.java:2025, Java Compiler (javac), Priority: Normale
getColor(String,Color) in Preferences has been deprecated

MapView.java:869, Java Compiler (javac), Priority: Normale
isChanged() in Layer has been deprecated

AbstractTileSourceLayer.java:1853, Java Compiler (javac), Priority: Normale
isChanged() in Layer has been deprecated

GpxLayer.java:259, Java Compiler (javac), Priority: Normale
isChanged() in Layer has been deprecated

OsmDataLayer.java:896, Java Compiler (javac), Priority: Normale
isChanged() in Layer has been deprecated

ValidatorLayer.java:126, Java Compiler (javac), Priority: Normale
isChanged() in Layer has been deprecated

[michael2402]

In 10920/josm:

See #13309: Do not add 'color.' prefix twice in getColor()

[michael2402]

In 10922/josm:

See #13309: Make PreferencesTest compile again.

[Don-vip]

one remaining deprecation warning to fix in MapCSSParser.jj:

        { return Main.pref.getColor("mappaint." + (sheet == null ? "MapCSS" : sheet.title) + "." + pref, ColorHelper.html2color(t.image)); }

[michael2402]

I was working on that one already.

I have two options:

1. Simply replace that call
2. Use a color property as return value - that way, we can reload preferences easier

I'd like to apply Option 2 after the release

[simon04]

Milestone renamed

[simon04]

Since recently, opening the preferences outputs the following warnings:

Last errors/warnings:
- W: Unable to get color from '' for color preference 'extrude.main.line'
- W: Unable to get color from '' for color preference 'improve.way.accuracy.helper.line'
- W: Unable to get color from '' for color preference 'make.parallel.helper.line'

[michael2402]

In 11270/josm:

See #13309: Make download dialog use preferences interface.

[Don-vip]

Milestone renamed

[Don-vip]

#14343#comment:10 must be addressed as well.

[Klumbumbus]

Replying to simon04:

Since recently, opening the preferences outputs the following warnings:

Last errors/warnings:
- W: Unable to get color from '' for color preference 'extrude.main.line'
- W: Unable to get color from '' for color preference 'improve.way.accuracy.helper.line'
- W: Unable to get color from '' for color preference 'make.parallel.helper.line'

I think this was fixed in r11542

[michael2402]

In 11758/josm:

See #13309: Use cached preferences for map drawing; align box text to full pixels if not rotated.

[Don-vip]

The warnings have been solved in r11623 and r11713.