#17119 closed enhancement (fixed)
[Patch] Improve rendering time of partly visible complex shapes
Reported by: | GerdP | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 18.12 |
Component: | Core mappaint | Version: | |
Keywords: | template_report performance | Cc: |
Description
What steps will reproduce the problem?
- Load relation 7379046 (6471 Members)
- Zoom to node 2576979853
- Use Download in current view
- Try to edit
What is the expected result?
Nothing special, editing should be possible
What happens instead?
Extreme slow reaction, CPU is near 100% when moving the mouse.
Please provide any additional information below. Attach a screenshot if possible.
Problem is that the complete outer ring of the multipolygon is passed to java.awt.Graphics2D.clip(Shape s)
each time you move the mouse. I've noticed this while working on #17095.
The attached patch clips the polygon with the rather simple Sutherland-Hodgman algorithm before using Graphics2D.clip()
. As a result, the reaction is much faster, though still not without any delay.
In the new ShapeClipper class I've copied source that I wrote for the mkgmap project years ago.
The code might as well go into class Geometry.
Build-Date:2018-12-17 19:37:47 Revision:14573 Is-Local-Build:true Identification: JOSM/1.5 (14573 SVN en) Windows 10 64-Bit OS Build number: Windows 10 Home 1803 (17134) Memory Usage: 1639 MB / 1753 MB (523 MB allocated, but free) Java version: 1.8.0_191-b12, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM Screen: \Display0 1920x1080 Maximum Screen Size: 1920x1080 VM arguments: [-agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:50636, -ea, -Dfile.encoding=UTF-8] Program arguments: [--debug] Dataset consistency test: No problems found Plugins: + OpeningHoursEditor (34535) + apache-commons (34506) + buildings_tools (34724) + download_along (34503) + ejml (34389) + geotools (34513) + jaxb (34506) + jts (34524) + o5m (34405) + opendata (34698) + pbf (34576) + poly (34546) + reverter (34552) + utilsplugin2 (34780) Last errors/warnings: - W: Update plugins - org.openstreetmap.josm.plugins.PluginHandler$UpdatePluginsMessagePanel[,0,0,0x0,invalid,layout=java.awt.GridBagLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=] - W: No configuration settings found. Using hardcoded default values for all pools. - W: Cannot start IPv4 remotecontrol https server on port 8112: Keystore was tampered with, or password was incorrect - W: Cannot start IPv6 remotecontrol https server on port 8112: Keystore was tampered with, or password was incorrect - E: Invalid Bounds: Bounds[0.0,0.0,0.0,0.0]
Attachments (7)
Change History (28)
by , 6 years ago
Attachment: | clip.patch added |
---|
comment:1 by , 6 years ago
comment:3 by , 6 years ago
I see no improvements in Java 11 regarding this problem. VisualVM reports the same bottleneck in the clip() method. So, the patch still improves performance a lot. I'll try to improve it further, I'd like to avoid the extra clipping for objects inside the view rectangle and maybe use a cache that would store the results as long as the view rectangle and the relation don't change.
by , 6 years ago
Attachment: | clip-v2.patch added |
---|
clip only when shape is partially outside view, some code improvements
comment:4 by , 6 years ago
The patch already helps a lot but there is a another problem:
When you edit something, e.g. draw a new way within the complex shape without changing anything near any of the rings of the mp, all elements of the mp visible on the screen are rendered again and again. The outer shape is selected because its bbox intersects with the
bbox of the visible area. Without the patch it is almost impossible to edit, with the patch it is still slow for very complex mp like in my example. This is also true when nothing of the mp is visibile on the screen.
Even more confusing: When you open a new Data layer the underlying layer is still rendered from time to time although I don't change the viewport or the order of layers. I did not yet find out by which event this is triggered...
I wonder why we render the full screen for every small change. I am a newbe here, but I think the graphic routines allow to redraw only a part of the screen. For example, when I hover over a line it is rendered with a different colour, but I see no need to render all visible elements. Still learning ...
Please let me know if I should commit this patch (after adding the @since lines) as is or if I should move the code in ShapeClipper somewhere else.
I'd also like to hear from Michael what he thinks about my changes in MapViewState
. I hope I got the calculations right.
comment:5 by , 6 years ago
The problem with a partial update is that it is extremely complicated to get it right all the time.
comment:6 by , 6 years ago
Yes, nobody says it's easy ;-) I'll probably need some more weeks to understand enough to improve this.
Besides the problems with complex MP this is also causing trouble when you download a larger area. Unless you zoom in or disable rendering for the layer JOSM is nearly unusable.
Not talking about power consumption and CO2 footprint ;-)
follow-up: 8 comment:7 by , 6 years ago
I doubt you get partial updates right. Some tried this...
At the end all tries went into getting the main rendering fast, so that partial rendering is no issue anymore.
An idea based on your text: What if we monitor the time it takes to render and reduce re-rendering if it takes too long. That could help to fix that "awfully slow when zoomed out" issue. Simply display an notice about the "fast render mode"?
Would that help?
P.S. JOSM is actually very fast usually, even expensive commercial software doesn't do much better.
follow-up: 10 comment:8 by , 6 years ago
Replying to stoecker:
I doubt you get partial updates right. Some tried this...
At the end all tries went into getting the main rendering fast, so that partial rendering is no issue anymore.
OK
An idea based on your text: What if we monitor the time it takes to render and reduce re-rendering if it takes too long. That could help to fix that "awfully slow when zoomed out" issue. Simply display an notice about the "fast render mode"?
Would that help?
What is the fast render mode? The wireframe style?
P.S. JOSM is actually very fast usually, even expensive commercial software doesn't do much better.
Yes, in most use cases it works very well. If not I would not work on these special cases.
comment:9 by , 6 years ago
One of my ideas: When rendering takes long, disable the highlighting of elements below cursor while cursor is moving.
I think it makes only sense when the cursor rests on an area.
comment:10 by , 6 years ago
An idea based on your text: What if we monitor the time it takes to render and reduce re-rendering if it takes too long. That could help to fix that "awfully slow when zoomed out" issue. Simply display an notice about the "fast render mode"?
Would that help?
What is the fast render mode? The wireframe style?
Nah. That would be too harsh. It is what ever we decide to keep the speed in a reasonable range.
comment:11 by , 6 years ago
Attached 17119-simple.patch simply disables the partial fill for complex polygons. That also makes JOSM usable again. I think I have to look at the code that computes the partial fill. Maybe we simply use the wrong draw method here.
by , 6 years ago
Attachment: | faster-complex-mp.patch added |
---|
comment:12 by , 6 years ago
Summary: | [Patch] Improve reaction time near complete complex multipolygons → [Patch] Improve rendering time of partly visible complex shapes |
---|
I think faster-complex-mp.patch is good for commit now. It improves performance for closed and unclosed shapes when they are only partially visible and has no notable impact on performance when the shape is completely visible.
Some numbers: The unpatched version requires between 4 and 5 seconds (!) to render a small part of the complex polygon, the patched version ~0.15 secs.
Besides better performance I see no differences, so if I hear no complains I'll commit this patch tomorrow.
comment:13 by , 6 years ago
No idea why I didn't hit that problem earlier, ShapeClipper.clipShape was not thread safe.
by , 6 years ago
Attachment: | faster-complex-mp-v3.patch added |
---|
reworked loop control, something is special with the pathIterator
follow-up: 20 comment:18 by , 6 years ago
Replying to stoecker:
Code changing hooks aren't really possible.
OK, no problem. I just stumbled over this script and hoped for the best ;)
comment:19 by , 6 years ago
BTW: since when does Overpass return all the data for the relations? Is that something that we can control?
comment:20 by , 6 years ago
comment:21 by , 6 years ago
Milestone: | → 18.12 |
---|
Did you compare with Java 11? The 2D renderer has been vastly improved, including for clipping algorithms. Maybe we don't need our custom implementation.