Modify

Opened 9 years ago

Last modified 3 months ago

#11487 reopened enhancement

[WIP PATCH] Have josm render data to tiles

Reported by: kimmybjonsson Owned by: team
Priority: normal Milestone:
Component: Core mappaint Version:
Keywords: image render paint tile tiles raster rasterize Cc:

Description

Dear Developers,
Doing work on large sets of data is slow. Why don't we let josm rasterize vector data into net little tiles? That way we conserve computing power AND make the application more able to respond to user input.

Attachments (4)

11487.patch (7.2 KB ) - added by taylor.smock 12 months ago.
Proof of Concept -- DO NOT APPLY -- see comment:2 for details
11487.2.patch (16.4 KB ) - added by taylor.smock 12 months ago.
WIP -- DO NOT APPLY -- Allows for arbitrary zoom levels, but do note that pixelation may occur; render code checks that the mapview hasn't changed between paint request, paint, and store; selection/highlight now only rerenders tiles with the selection/highlight change (once); use old render path at high zoom (configurable)
11487.3.patch (35.9 KB ) - added by taylor.smock 12 months ago.
Keep invalidated tiles until they can be painted again, refactor so that there is a new renderer class (StyledTiledMapRenderer, set mappaint.renderer-class-name to org.openstreetmap.josm.data.osm.visitor.paint.StyledTiledMapRenderer to use), various render optimizations (use "native" BufferedImage for images), add Rendering status text, avoid invalidating mapview everytime a tile finishes rendering, render around mouse preferentially
11487.4.patch (45.3 KB ) - added by taylor.smock 2 months ago.
Add UI element to enable turning tiled rendering off/on, initial work on batching tile rendering

Download all attachments as: .zip

Change History (18)

comment:1 by stoecker, 9 years ago

Resolution: wontfix
Status: newclosed

JOSM is an editor. It's main purpose is to modify data. Thus rendering to tiles does not really work.

comment:2 by taylor.smock, 12 months ago

As a stupid question, why doesn't it work?
We could render to tiles, and then invalidate and redraw specific tiles instead of the entire mapview.
Advantage:

  • We don't have to redraw the world when someone is panning around
  • We can just redraw the changed tiles instead of the world
  • We don't have to have special code paths to not draw expensive paths during map pan

Cons:

  • A bit more processing up front (I've got a proof-of-concept patch which is broken, but shows that an area will be fully drawn, which means that an area will be drawn on every tile it is in)

I'll upload the proof-of-concept patch. A couple of notes on it:

  • You have to load an imagery layer at least once. No clue why this is happening (yet).
  • Current render code uses the mapview to stop painting outside of the mapview. These tiles are cached, so there will be some rendering weirdness when panning (on the outer tiles). This can probably be fixed by marking tiles as fully inside the mapview.
  • Not as efficient as it could be -- all tiles are invalidated when something is selected/highlighted

by taylor.smock, 12 months ago

Attachment: 11487.patch added

Proof of Concept -- DO NOT APPLY -- see comment:2 for details

comment:3 by stoecker, 12 months ago

It depends on what you want. If you want to have a display tool which shows data at certain zoom levels, then caching will work. But there probably is better software for this purpose out there (is there?).

When you want an editor with constantly zooming and panning and selecting and modifying, then the speed advantage will be low and you'll have a lot of code to keep data in sync. It was already a long process to get the caching in MapView work reliable. Another software layer on top of this will have similar trouble.

comment:4 by taylor.smock, 12 months ago

Most of the "wins" for rendering tiles will be at low zoom levels. At levels where editors are actually changing things (z14+, although I usually work at z18+), the wins be minimal to none, depending upon data density. With that said, there are very good performance wins at the lower zoom levels (like measuring UI responsiveness in seconds per frame to frames per second).

It probably wouldn't matter as much if the entire UI was not blocked during the map paint (so if something takes 30s to render, you have to wait 30s before input is recognized, then wait another 30s for the next input to be recognized, and so on).

Realistically this will help people who decide to load large amounts of data (city, county, or country scale) the most.

comment:5 by stoecker, 12 months ago

Fixing the low zoom issue with lots of data would be a fine goal. Thought I'm not sure if that's the right approach. Usually it's not necessary to render the low zoom fully to work. You only need to see something.

I think it would be much more helpful when
a) Rendering doesn't waste effort on anything no longer displayed due to in-between zoom operation (don't know if this was fixed already)
b) Do something like PNG progressive: first draw a raw sketch and step by step improve that

  • as soon as you seem something you can move around or zoom
  • if you wait the image gets more detailed

I.e. JOSM could first draw wireframe style data or simple line drawing or whatever and then full mapcss based drawing.

in reply to:  5 comment:6 by stoecker, 12 months ago

a) Rendering doesn't waste effort on anything no longer displayed due to in-between zoom operation (don't know if this was fixed already)

Ah, reading you text again I see this was not fixed yet :-) That's your "It probably wouldn't matter as much if the entire UI was not blocked during the map paint".

comment:7 by taylor.smock, 12 months ago

For (b), I was originally going to try to use the ordering from the mapcss styles, but that didn't work well -- I was trying to get map paint to take <16ms, and when I shortcircuited the paint once that 16ms was taken and then painted the 100 most important objects, I came out with a funny looking render.

The other option I had was adding a selector of some kind to the mapcss spec, so something like importance(10), and then back stepping through that. I'd have to render to different images and then concat them, but it would be doable.

Now that I think about it, I could probably do something like (b). I'd just have to pick an arbitrary number, render that number of objects to an image, then merge the previous on top of that. Only caveat is that users will see some data popping in and out.

For (a), we rerender everything right now, not just the stuff that changed. I (personally) think the easiest way to rerender just the stuff that changed is to try and cache everything that didn't change, which is effectively what I am doing with the tiled render approach. As a bonus, I've been able to offload the rendering to a separate thread (caveat: moving the mapview creates interesting results), so as a tile gets rendered, I indicate that the layer needs to be repainted.

We do cache the mapcss style that the object uses to get painted, but that doesn't help with a lot of data. Most of the time in the render loop is spent rendering ways/areas (when I've been profiling rendering areas like Washington D.C.).

in reply to:  7 comment:8 by stoecker, 12 months ago

Only caveat is that users will see some data popping in and out.

Well, I consider that better than "display nothing and block". But I'm 100% sure that when we do that we'll get a lot of tickets...

by taylor.smock, 12 months ago

Attachment: 11487.2.patch added

WIP -- DO NOT APPLY -- Allows for arbitrary zoom levels, but do note that pixelation may occur; render code checks that the mapview hasn't changed between paint request, paint, and store; selection/highlight now only rerenders tiles with the selection/highlight change (once); use old render path at high zoom (configurable)

comment:9 by taylor.smock, 12 months ago

Resolution: wontfix
Status: closedreopened

Honestly, just about anything is better than either "display nothing" or "block". But having data appear and disappear will definitely cause most people to file tickets.

Anyway, notes for attachment:11487.2.patch:

  • 90% complete (still should not be applied)
  • It throws away rendered data when a tile is invalidated due to highlighting/selection. This tile is still valid, just not in that state. Worst case: keep that tile until a new tile is painted, but that will require a fix for tearing on mapview move, I think.
  • mappaint.fast_render and mappaint.fast_render.zlevel control the use of the fast path -- the former currently defaults to false and the latter defaults to 16.
  • There is a noticeable performance decrease in UI responsiveness when going from the "fast" render path to the current render path.
  • Tiles are currently painted on the worker thread. The tile painting is not thread safe, so each tile must be painted by without other tiles being painted. We could probably have a separate thread (or thread pool) for when there are multiple data layers.
  • Cached tiles don't update for the current inactive state. Maybe paint the cached tiles in grayscale?
  • It might be better to do the tiling in the renderer, e.g. add a setRenderedTileCache method to the interface.
  • Not the most efficient -- each rendered tile actually renders a total of 25 tiles (an extra two rows/columns on each side) to ensure that the placement doesn't change across tile boundaries. This is still much more efficient than current code, since it does that once, instead of rendering everything in view once per frame.

in reply to:  9 comment:10 by stoecker, 12 months ago

Replying to taylor.smock:

Honestly, just about anything is better than either "display nothing" or "block". But having data appear and disappear will definitely cause most people to file tickets.

A "Rendering in progress" note in the notes function of the map?

by taylor.smock, 12 months ago

Attachment: 11487.3.patch added

Keep invalidated tiles until they can be painted again, refactor so that there is a new renderer class (StyledTiledMapRenderer, set mappaint.renderer-class-name to org.openstreetmap.josm.data.osm.visitor.paint.StyledTiledMapRenderer to use), various render optimizations (use "native" BufferedImage for images), add Rendering status text, avoid invalidating mapview everytime a tile finishes rendering, render around mouse preferentially

comment:11 by taylor.smock, 11 months ago

Summary: Have josm render data to tiles[WIP PATCH] Have josm render data to tiles

comment:12 by zelonewolf@…, 3 months ago

Taylor provided me a jar file with this change included. It is definitely much faster than the current bleeding-edge JOSM.

I'm using it for the edits discussed here, so this is a real use case:
https://community.openstreetmap.org/t/proposal-undo-4-year-old-parcel-import-in-dallas-tx

comment:13 by stoecker, 3 months ago

What's you overall experience? Much better, better but has some issues or usable for special cases but we'll get many tickets.

If it's one of the first two I'd vote to integrate it into core and see what happens. This is important enough to not let it bit-rot.

comment:14 by taylor.smock, 3 months ago

I think he hit one of the other changes I made for performance reasons; my working branch code requires the user to change mappaint.renderer-class-name to org.openstreetmap.josm.data.osm.visitor.paint.StyledTiledMapRenderer (and restart, if data has already been painted). I didn't tell him that he needed to do that, since I forgot that I needed to do that.

Much better, better but has some issues or usable for special cases but we'll get many tickets.

With my current code (on my machine), the default renderer hasn't been changed. I never committed since there was a pretty nasty performance regression that I think I just fixed w.r.t. to selecting large amounts of data. We'll see what zelonewolf says after he tries out the new jar I shared.

Anyway, for other regression issues, the non-tiled renderer is used at z levels where people might actually want to edit. That is configurable so I could test how things rendered at z levels where I want to edit. But tiled rendering isn't usually necessary at those levels.

I'd guess that some of the performance improvements he saw were due to me changing new BufferedImage in MapView to GraphicsConfiguration#createCompatibleImage.

by taylor.smock, 2 months ago

Attachment: 11487.4.patch added

Add UI element to enable turning tiled rendering off/on, initial work on batching tile rendering

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user. Next status will be 'new'.
Next status will be 'needinfo'. The owner will be changed from team to kimmybjonsson.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.