Modify

Opened 9 years ago

Closed 3 months ago

Last modified 2 months ago

#11487 closed enhancement (fixed)

[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 19 months ago.
Proof of Concept -- DO NOT APPLY -- see comment:2 for details
11487.2.patch (16.4 KB ) - added by taylor.smock 19 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 19 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 9 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 (25)

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, 19 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, 19 months ago

Attachment: 11487.patch added

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

comment:3 by stoecker, 19 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, 19 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, 19 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, 19 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, 19 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, 19 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, 19 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, 19 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, 19 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, 19 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, 18 months ago

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

comment:12 by zelonewolf@…, 10 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, 10 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, 10 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, 9 months ago

Attachment: 11487.4.patch added

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

comment:15 by taylor.smock, 3 months ago

Resolution: fixed
Status: reopenedclosed

In 19176/josm:

Fix #11487: Have josm render data to tiles

This adds a new rendering method that renders async. This avoids blocking the UI.

Where this is useful:

  • Large datasets (think county or country level)

Where this is not useful:

  • Micromapping -- the tiles aren't being rendered exactly where they should be and there are some minor rendering artifacts.

Known issues:

  • Some tiles aren't exactly where they should be (off by a pixel or two -- by default, we use the old render method at z16+)
  • Rendering of tiles is slow -- there is some prework done to render tiles in batches. The primary reason rendering is slow is we are effectively rendering 25 total tiles (to avoid movement of text, we render 2 tiles in each directory and only keep the middle one)
  • Due to the above speed issue, hovering over an object will cause the highlight to render in slowly.

New advanced preferences:

  • mappaint.fast_render.tile_size -- controls the number of pixels in a tile
  • mappaint.fast_render.zlevel -- controls the maximum z level at which tiles are generated

comment:16 by taylor.smock, 3 months ago

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

comment:17 by skyper, 3 months ago

Starting the documentation, Help/Menu/View, I think this new action still needs an icon.

comment:18 by taylor.smock, 3 months ago

You probably aren't wrong. My long term goal with the rendering method is to make it the default and get rid of the action though.

Last edited 3 months ago by taylor.smock (previous) (diff)

comment:19 by skyper, 3 months ago

Mmh, then I do not know if it makes sense to document it at all in its current state. I wonder if a menu item is needed or if an advanced preferences would be enough. I guess it gets more attention with the new menu item which hopefully results in more user testing it plus the menu item makes it more handy to switch between the two renderings. So I understand why you chose a menu item instead of just an advanced preference.

comment:20 by taylor.smock, 3 months ago

I mostly added the menu item to make it easier for me to add a shortcut to flip between tiled rendering and non-tiled rendering. Locally, I just reused the shortcut for wireframe mode.

I committed it since I figured it would be helpful for other people while I fix bugs in the tiled rendering pipeline.

comment:21 by taylor.smock, 2 months ago

In 19220/josm:

See #11487: Have josm render data to tiles

Start adding basic rendering tests for tiled rendering.
Right now, the test only looks at a point in the center of the tile; there seems
to be some positioning/stretching issues at the edges that I need to debug and
fix.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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