SVG targeted loader to fix Firefox issues

[mike-000]

Fixes the poor quality SVG reprojection noted in #14945 (comment)

With a single source (either Image or Tile without gutter) reprojection does not need to use a stitchContext, the image could be drawn directly to the output context. However for tiles that could result in changes to quality if second tile comes into view, so it is suppressed by only doing so if sourceResolution is passed as zero (as it is only needed to create the stitchContext). ReprojImage passes 0 unconditionally as even for a bitmap a single scaled draw should produce slightly better results than stretching into a stitchContext followed by further scaling.

Added example https://deploy-preview-15013--ol-site.netlify.app/en/latest/examples/reprojection-image-svg.html

[github-actions]

📦 Preview the website for this branch here: https://deploy-preview-15013--ol-site.netlify.app/.

[mike-000]

Apart from the expected quality improvement in one test (removal of duplicated row) were no tests or examples which this change might have broken. Image WMS (or ArcGISRest) reprojection with pixelRatio > 1, with or without hidpi setting, if examples existed, might have been have been affected if this change was incorrect, but both appear to be working correctly:

https://jsbin.com/ralejivoro/edit?html,output

https://jsbin.com/rudegukuli/edit?html,output

as do their SVG format equivalents.

[mike-000]

I have found a issue with some SVG on Firefox. drawImage always preserves the aspect ratio of the viewBox meaning reprojection does not work correctly

This https://jsbin.com/nadaquhodu/edit?html,output displays as expected in Chrome, but not Firefox (where it is also very slow), so suppressing the use of stitchContext may need to be browser dependent.

Also when these SVGs are used as icons two dimensional scaling does not work.

This can be fixed in #14933 (it is also needed for non-reprojected SVGs whose extent does not match the aspect ratio) but reprojection performance remains unacceptably slow in Firefox unless a raster image is used.

[tschaub]

This is a puzzling example to me.

It says "there is minimal blurring (or pixelation if interpolation is disabled) when zooming in."

Should I notice any difference when clicking on the "Interpolate" checkbox? I don't notice any difference when recording the animation above.

The text says "minimal blurring ... when zooming in." Does this mean blurring during a zoom transition (like "while" zooming in) or blurring at a high zoom level (like "when" zoomed in)?

Is the change you are making only relevant when using the interpolate option? If not, I would remove the checkbox and any mention of it from the example.

[mike-000]

The text says "minimal blurring ... when zooming in." Does this mean blurring during a zoom transition (like "while" zooming in) or blurring at a high zoom level (like "when" zoomed in)?

Some blurring or pixelation (depending on the interpolate setting) is noticeable when zooming. It will only be noticeable when zoomed if you are using a inappropriate pixel ratio for your device (or you are using Firefox, when the fallback will be very noticeable).

[tschaub]

I’ll try to rephrase my question. What is the purpose of the checkbox?

[mike-000]

I’ll try to rephrase my question. What is the purpose of the checkbox?

It does not have much purpose, interpolation could be left either on or off. I think pixelation with interpolation off is more noticeable than interpolated blurring in this example if making a comparison between the two examples.

[ahocevar]

Instad of introducing a special meaning of 0, wouldn't it be more a more general solution to compare sourceResolution and targetResolution, and enter this code path when they are equal?

[mike-000]

Equal or not we always want to do it for Image reprojection (unless Firefox because it is too slow), and do not want to do it for Tile reprojection (we could if there was only one source tile, but the quality would change as soon as a second tile comes into view). Using zero avoids passing another parameter.

[ahocevar]

I see the rationale behind it, I'm just not a fan of misusing existing properties with special values to trigger entering different code paths. A clean solution would be to use the same sourceResolution and targetResolution, and add a new boolean property stitch or something.

[tschaub]

I think this pull request could be improved with a rendering test that demonstrated what it does. I'm not sure that the example is necessary.

[mike-000]

I think this pull request could be improved with a rendering test that demonstrated what it does. I'm not sure that the example is necessary.

Test added. If run in Firefox it would fail because (a) we are using a rasterized fallback and (b) even without the fallback Firefox would not scale that image correctly (unless as in #14933 we use an SVG specific loader to inject preserveAspectRatio="none" into the SVG header).

The example is similar to having both Single Image WMS and WMS loader with SVG format. That one shows how to use SVG format, but the difference in output quality is barely noticeable because raster WMS is served to match the view resolution. Comparing static images with a fixed resolution does at least show a difference in output quality when zoomed in.

[mike-000]

Test added. If run in Firefox it would fail

The test should now work on all browsers, with a simple canvas fallback loader which does not need CORS or rely on user agent. Rebased to include #15039 without which the example would produce large amounts of canvas for garbage collection in Firefox.

[julienrf]

I would be interested in seeing this merged as it will fix #15144. Is there anything I could do to help?

[mike-000]

I would be interested in seeing this merged as it will fix #15144. Is there anything I could do to help?

I have rearranged the commits slightly so the first commit alone will fix #15144 in any browser.

As a result the example in the second commit will not use a Firefox fallback and requires the third commit otherwise it might freeze if run in Firefox.

[ahocevar]

@mike-000 Thanks for your continued effort on this. Could you pull 231d37a out into a separate pull request - that fix looks straightforward and is a candidate for merging without additional discussion.