#11625 closed enhancement (fixed)
Add MapView hooks required by OpenGL plugin.
Reported by: | michael2402 | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 15.09 |
Component: | Core | Version: | |
Keywords: | Cc: |
Description
The following changes are made to JOSM:
- Access to MapView#layers/temporaryLayers is synchronized. Synchronisation is a bit tricky here, because many Listeners do a invoke and wait on the AWT event thread, which might cause dead locks. There should be none in main JSOM, but I don't really know what crazy things plugin developers are doing...
- A hardcoded MapView#redraw(...) is used in many places. I added a listener to intercept those calls and redraw the OpenGL view instead.
- Selection uses a temporary layer to draw the polygon/rectangle. I found no more uses of NavigatableComponent.requestPaintRect/Poly(...) and the related methods, so they could be reoved/marked as deprecated. I don't really think that that is nice coding style, we have temporary layers for that. Those selection changes should also fix some mouse button issues and make e.g. the right mouse button work when lasso-selecting. (#10218, #11094)
- The current cursor also allows multiple listeners.
- The zoom/scrolling got its new class.
- I don't really want to blow up the MapView much more. Together with NavigationComponent, it has almost 3k lines. This is why I moved the cursor management and the zoom/center to new classes.
- Fixed #11496
Some documentation additions and other minor changes are also included.
A full diff is here:
https://github.com/michaelzangl/josm/compare/mirror...gsoc-opengl
Attachments (0)
Change History (7)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
Hi,
Thanks for all those comments. I am currently splitting up the patches and adding separate tickets for the changes.
I had a look at the projection change listeners before. Holding weak references without notifying the user - and not even a javadoc node in the addListener method) leads to bugs you can not trace easily (Why is my listener not firing after xxx seconds?) I know that JOSM often uses the active Object as listener. But many programmers (like me ;-)) like to put all code that is related to the listener into a new class and just register that class. If you do not support unloading (plugins, ...) you don't need to keep a reference to that listener.
comment:3 by , 10 years ago
Summary: | [Patch] Add MapView hooks required by OpenGL plugin. → Add MapView hooks required by OpenGL plugin. |
---|
comment:4 by , 10 years ago
comment:5 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:7 by , 9 years ago
Milestone: | → 15.09 |
---|
Thanks, great patch! I like the idea to refactor the navigation code and move it to another class. But having all in one huge diff file, makes it hard to see, what part of the code is just copied and what has been changed. I would appreciate if you could split the patch in (at least) 2 parts: One which moves code to other classes, but has minimal changes otherwise and one with all the rest. If you are fixing separate issues, it would be better to attach a patch to the corresponding ticket (when appropriate).
Other comments:
GlobalSoM
with theSystemOfMeasurement
class.@deprecated
to the things you just leave for backward compatibility.EastNorth
: I'd rather keep it the way it is now (easier, more efficient). But leave the Javadoc! :)NavigationModel.WeakZoomChangeListener
: Have a look atMain.fireProjectionChanged
. This forces use ofWeakReference
, so is more foolproof.