openlayers / openlayers

OpenLayers
https://openlayers.org
BSD 2-Clause "Simplified" License
11.41k stars 3.03k forks source link

Non-tiled WMS is rendered inefficiently #16155

Closed sampov2 closed 1 month ago

sampov2 commented 2 months ago

Describe the bug Image.ts uses createImageBitmap() to read data from images before drawing the image data on the canvas. This function causes overhead in cases where there is no need to manipulate the image but instead the image is going to be displayed as-is. This is a considerable issue for applications such as those with multi-maps with time animation loops.

https://github.com/openlayers/openlayers/blob/fff95c7da08515f7b295ce4129934f55d9dd92c1/src/ol/Image.js#L331

ahocevar commented 2 months ago

Do you have an example for a map where this is inefficient? In most scenarios, this speeds up rendering during panning significantly, because no further decoding is necessary on the frequent drawImage() calls during panning.

mike-000 commented 2 months ago

The benefit of using an ImageBitmap does seems to differ considerably between systems, see https://www.measurethat.net/Benchmarks/ListResults/5067

To always use an Image directly see https://openlayers.org/en/latest/examples/wms-image-svg.html

ahocevar commented 2 months ago

ImageBitmap only has a benefit when drawImage is called more than once from that ImageBitmap. This is the case whenever the map is panned. In time animations with static map images, it may indeed be better to use decodeFallback() or load() instead of decode() as load function.

mike-000 commented 2 months ago

This is the case whenever the map is panned.

only if ratio is >1 Both the ImageWMS and ImageArcGISRest examples use ratio 1. Only the ImageMapGuide example uses ratio 2.

ahocevar commented 2 months ago

This is not correct, @mike-000. A new image will only be loaded when no animation or interaction takes place, so during panning the existing image will always be re-used.

sampov2 commented 1 month ago

Do you have an example for a map where this is inefficient? In most scenarios, this speeds up rendering during panning significantly, because no further decoding is necessary on the frequent drawImage() calls during panning.

I can provide an example shortly. But in my use-case, normal usage is a continuous, repeating timeseries animation on a static view. In this case, panning performance is not a big consideration.

mike-000 commented 1 month ago

A new image will only be loaded when no animation or interaction takes place

Yes, that is the problem with ratio: 1. If instead of continuous interaction you are nudging the view a few pixels at a time (or using keyboard pan) there are frequent reloads.

sampov2 commented 1 month ago

I made a quick sandbox for single-image and tiled versions of the same scenario. 4 maps side by side animating two layers in a looping time-series. All maps are using the same layers, but that should not exacerbate the issue but quite the reverse (as each map requires the same image, there should be less processing of individual responses from WMS servers):

Both versions take up some CPU at first (when loading the images and process them at first). Once everything is initialised, CPU usage of the tiled version drops to near zero while the single image version still churns a considerable amount (~30% according to top on my laptop, 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz the difference in performance was also verified on an Apple M1)

ahocevar commented 1 month ago

@sampov2 Now because you're not interacting with those maps, it would make sense to call the createLoader() function with a different load function:


    import { load } from "ol/Image";

    createLoader({
      load,
      url: conf.url,
      params: {
        LAYERS: conf.layername,
        TIME: timeValue,
      },
    }),

Use cases like this are the reason why we made the Image layer modular.

ahocevar commented 1 month ago

@sampov2 You could also lower the burden on the servers and the network by using ratio: 1, which means only the visible extent will be loaded:


    createLoader({
      load,
      ratio: 1,
      url: conf.url,
      params: {
        LAYERS: conf.layername,
        TIME: timeValue,
      },
    }),
sampov2 commented 1 month ago

Changing the load function removes the performance issue. However it raises the question: why is the much slower decode the default load function?

ahocevar commented 1 month ago

Like I explained above, because interacting with a map (where performance benefits from the decode load function) is more common than showing a static map with time series.

sampov2 commented 1 month ago

Ah, I re-read your earlier comments and now, I figured out the role of the load function. Makes sense, very nice flexibility. Thanks for the help.

I'm closing the issue, but It might be worth considering writing something about this in the createLoader() load prop. Something that would help developers understand in which cases they should take a deeper look at that property.