Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When a Draw interaction of vector layer is in use, all tile and image layers are also being rerender #15722

Open
Feofilakt opened this issue Apr 10, 2024 · 25 comments
Labels

Comments

@Feofilakt
Copy link

Describe the bug
Don't know if it is a bug or intended behavior. It seems that when a Draw interaction of vector layer is in use, all tile and image layers are also being rerender. We have faced performance problems because of it (high GPU usage during simple interaction). Maybe I'm missing something here - unfortunatly, am not an expert in OL rendering system.

To Reproduce

  1. Go to https://codepen.io/Feofilakt/pen/mdgLzPr
    You can see captions "Hello world 0" and "Hello world 1". Each on one of canvases - I checked it by removing single canvas:

image

image

  1. Try to draw line (click and move cursor)
    Both captions disappear.

Expected behavior
Only one caption disappears, other (on the canvas of tile layer) still remains.

@Feofilakt Feofilakt added the bug label Apr 10, 2024
@ahocevar
Copy link
Member

This is intended behavior - for any change anywhere on the map, the whole map is re-rendered. Usually what's being done is just the composition of the final map canvas, so that should not cause any performance problems.

It would be interesting to hear in which scenario you're facing performance problems. I guess not a simple one like in the code pen you have prepared?

@Feofilakt
Copy link
Author

Of cause :) Multiple image and tile layers on 4k monitor resolution with not so powerful internal grahpic card. We create one vector layer with draw interaction and it is quite slow during line drawing (sketch is not on time with mouse cursor). If turn off all the image and tiles layers, the problem completely disappears.

Could you advice some optimization ways for this case (other than turning off other layers)?

@ahocevar
Copy link
Member

ahocevar commented Apr 11, 2024

Make sure all image and tile layers have the same resolution and size (i.e. ratio: 1 on image layers), and avoid setting className on layers, so they can be combined onto a single canvas. But hard to give more detail without seeing actual code.

@Feofilakt
Copy link
Author

Its just too much rendering, I think: all the layers on big map. Not a problem by itself, but it happens in real time (on each mouse move event?) and so interfere with interaction sketch rendering (which, I guess, has same priority).

@ahocevar
Copy link
Member

ahocevar commented Apr 12, 2024

The rendering engine is built for that - when you pan the map, it also re-renders on every pointer move. Check your code for the pitfalls I mentioned above. In the case of image layers, it might also be the case that an image cannot be converted to an ImageBitmap, which is required for fast composition. You can check for that with a breakpoint in ol/Image.js's decode() function and see if it runs into one of the fallbacks.

@Feofilakt
Copy link
Author

You were right about ImageBitmap. We use custom loader for POST requests and function actually runs into fallback. After changing that performance have imroved a little, but unfortunatly lags are still noticable.

@ahocevar
Copy link
Member

Are all your images now being converted to ImageBitmaps? If not, make them smaller (e.g. by setting ratio: 1 in the source or loader).

@Feofilakt
Copy link
Author

Yes, all images are now being converted to ImageBitmap and ratio is 1. Probably we will be forced to implement draw without sketch.

@ahocevar
Copy link
Member

Did you also remove all className configs from the layers?

@Feofilakt
Copy link
Author

Yes, we did that.

@ahocevar
Copy link
Member

@Feofilakt If you can provide a live example that shows the problem, we could profile it and see what the culprit is.

@alexerisov
Copy link

Hello. I faced with similar issue and had an idea, maybe make a fork and make a correction so that skip render of other layers on each "pointermove” event while drawing a sketch ? Could these fixes break something?

@ahocevar
Copy link
Member

This would be quite a hack. And you'd have to put the draw layer on a separate canvas. It's better/easier to find out why you're spending so much time in the composition phase. This is why I'd like to see this issue in a live example I can profile.

@alexerisov
Copy link

alexerisov commented Apr 21, 2024

The issue here is not with the composition time, but with the GPU load. In other words, it’s not a library bug; it’s simply that the GPU is struggling. I’ve faced a similar issue even in online paint. Providing a demo is challenging, but I guess you could achieve this effect by heavily loading the GPU (e.g., using a benchmark) and then using the Draw interraction.

Regarding the separate canvas, yes, that was my initial attempt. I added className: "draw-layer" here. However, the main canvas still updates during events. Currently, the idea is to add here a simple check (smth like event.originalEvent.target.className === "draw-layer" && layer !== sketchLayer) and and ignore move events for all layers except sketchlayer.

@ahocevar
Copy link
Member

@alexerisov It's not the pointermove event that triggers map rendering, so your idea won't work. What would work is in the map's Composite renderer's renderFrame() method if all layers that can be grouped in the same container (useContainer()) did not change since the previous render frame. If they did not, the previous render frame's canvas can be used again.

@alexerisov
Copy link

Thanks for the help, I'll try your suggestion :)

@alexerisov
Copy link

alexerisov commented Apr 22, 2024

I tried to make the edit but couldn't find how to access previous frameState in renderFrame method (there's just current frameState as argument). Could you advise on the best way to make this edit?

ps. Wouldn't such behavior be useful for everyone? I mean, if the layer hasn't changed. why rerender it? Maybe it could be integrated into openlayers as PR?

@ahocevar
Copy link
Member

ahocevar commented Apr 22, 2024

@alexerisov It's a corner case, and would require quite a lot of code for this corner case.

By the way, your suggestion to not handle pointermove can be handled easily by adding a custom interaction to the map on drawstart, and remove it again on drawend:

const stopPointermove = new ol.interaction.Interaction({
  handleEvent(event) {
    return event.type !== 'pointermove';
  } 
});
draw.on('drawstart', () => map.addInteraction(stopPointermove));
draw.on(['drawend', 'drawcancel'], () => map.removeInteraction(stopPointermove));

@alexerisov
Copy link

alexerisov commented Apr 22, 2024

Yes, I thought about same solution but it intercepts 'pointermove' at all, so sketch layer isn't updating (similar to your first suggestion with style: () => undefined).
May be you have any ideas how to intercept 'pointermove' but keep skecth layer updating?

@ahocevar
Copy link
Member

Sketch layer updates means re-compose the map image, so you can either have one or the other.

@alexerisov
Copy link

alexerisov commented Apr 22, 2024

Hm, I moved sketch layer to another canvas (by adding className to Draw.overlay_). Should map re-compose this case?

p.s. Also, regarding the editing of the rendering-logic, is it possible to enforce such change for openlayers? E.g on a commercial basis ? If possible, what about time/cost? We are interested in such optimization (and it would be beneficial for the community - prevent the layers from uneccessary renders.

@ahocevar
Copy link
Member

Hm, I moved sketch layer to another canvas (by adding className to Draw.overlay_). Should map re-compose this case?

Yes, the map will always be re-composed when anything on any layer changes.

p.s. Also, regarding the editing of the rendering-logic, is it possible to enforce such change for openlayers? E.g on a commercial basis ? If possible, what about time/cost? We are interested in such optimization (and it would be beneficial for the community - prevent the layers from uneccessary renders.

Like I said, the change you're envisioning would add a lot of code and require the user to split the map into render sections, i.e. by setting className, which is something I'd like to avoid. I see the className as an option that is possible due to an implementation detail, but it would be better to remove it.

A more straightforward alternative I could think of would be to uncouple unmanaged layers (i.e. those you don't add to the map with map.addLayer(layer), but layer.setMap(map), which is what the Draw interaction uses, from the map renderer's main rendering loop. I'd be willing to make such a change if you sponsor me on https://github.com/sponsors/ahocevar/ with a monthly contribution of 200$ for at least 12 months.

@alexerisov
Copy link

alexerisov commented Apr 22, 2024

by setting className

It was just unsuccessful attempt, because map updates all layers independently from which canvas it use.

I could think of would be to uncouple unmanaged layers

Hm, interesting way. Firstly I thought about providing much control under layer-renders. At the moment, Layer.render() calls from different cases and can't be controlled\overrided. It would be easier to define render-condition for each Layer

@ahocevar
Copy link
Member

ahocevar commented Apr 22, 2024

At the moment, Layer.render() calls from different cases and can't be controlled\overrided

Well, you can call render() on a layer any time you want, and if you have made sure the layer has its own separate canvas (by setting a distinct className), it will do what you expeced, i.e. only render that layer. But it won't help you with your use case, because you want the opposite, i.e. to avoid that other layers' canvases get re-rendered. Which you cannot, because a change in a geometry of the draw layer's source triggers a re-render of the whole map.

It would be easier to define render-condition for each Layer

Yes, and that might be a sensible addition of the API, but would currently be unintuitive because of the required combination with the className setting.

So my idea was to make the most common use cases as intuitive as possible. And I think the most common use case is overlays with draw sketches or feature selections. For both, an unmanaged layer makes sense. And it is a clear distinction by which we can say: a layer/source change does not trigger a map render cycle, but only a render cycle of the unmanaged layers. The composite map renderer could measure the time spent in a whole render cycle, and render the unmanaged layers to a separate canvas when more frame budget than a certain threshold was spent in the last render cycle. A clean solution with not too much additional code, and nothing the user needs to do to make it work.

@alexerisov
Copy link

Alright. Thanks for the answer :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants