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

Printing in higher and lower resolution #1407

Merged
merged 22 commits into from
Feb 14, 2022

Conversation

sweco-sefaed
Copy link
Contributor

Proposes to fix #1293.

Elements and components (scalebar etc.) displayed in print preview are resized to fit selected resolution.
Uses vendor specific DPI parameter to get images in higher resolution, resulting in text and symbols not getting smaller/larger in high/low DPI. Vector layers are rescaled by changing layer or feature style.

Setting type on source in index.json is now an option, to ensure what type the source is. This is needed since we're using vendor specific parameter for DPI. If no type is defined URL will be checked for words "geoserver" and "qgis". Valid types are currently Geoserver, QGIS and ArcGIS, though QGIS has not been tested.
Example:

"local": {
   "url": "http://localhost/geoserver/wms",
   "type": "Geoserver"
}

Tested layer types:
WMS, WFS, AGS_MAP, AGS_TILE, AGS_FEATURE

Layer types that are ignored:
OSM, XYZ, WMTS

There were some issues rescaling WFS cluster. Setting cluster distance works, but rescaling each feature is not working at the moment. The WIP code for cluster is currently included.

AGS_MAP has source type ImageArcGISRest which lacks support to alter DPI (except using pixelRatio, which brings other issues), we counter this by creating a ImageWMS source with the same parameters. When print preview closes ImageArcGISRest is restored as source for the affected layers.

Recommend using "Create PDF" since "Save image" is not working very well.

If these changes should not be applied as default, an option could be added to print control similar to supressResolutionsRecalculation.

@steff-o
Copy link
Contributor

steff-o commented Jan 14, 2022

If a featurelayer is missing style in configuration the print preview crashes. It is because layer.get('styleName') returns a name 'default' but viewer.getStyle(styleName) returns null. Maybe the style should be fetched from the layer using layer.getStyle() to get the actual style in case it has been changed instead of from configuration. To be completely accurate the style for each feature should also be checked if it has been overridden. Don't know if it possible to get the style in a form that is possible to rescale that way though. At least the crash should be fixed.

I haven't tested it, but resetWfsThemeLayers() seems to only affect wfs layers (no surprise there), but changeWfsThemeLayer() seems to change all vector layers. But that might be as intended if it only can happen to WFS layers in the first place. I don't know how to create a style that tests this condition.

Other than that, it looks good. I haven't actually printed anything, just zoomed in the pfd:s but that looks as expected. I have only tested WFS and WMS on Geoserver and OSM.

I'm not sure what is best, but maybe tiled layers should not rescale or have an option if it should. Aerial images looks best if they are rescaled, but tiled vector maps like OSM probably looks better without scaling. I think most users would choose a blurry image before a crisp image that has too small texts to be read. Or just leave it like it is and take that in another issue later.

Also, when changing resolution, the selected resolution button is not highlighted, but that may be an old issue.

If the export to image is not working as expected it might be better to just remove it. It is bad enough to print out a scale in a PDF when it is not known on what paper size the user actually prints the map, but in a PNG without DPI information there is no clue at all at which size the scale is supposed to match the image. At least the PDF has a nominal paper size.

@Grammostola
Copy link
Contributor

Hi, thanks for your feedback; Sweco were acting under our supervision so some things are probably on my watch, for instance no style property for vector layers, in what instances would that be true? Most of the time vector layers will probably have a style, I've reasoned, if that's not the case an adaptation along the lines you've reasoned is needed.

I'm not sure what is best, but maybe tiled layers should not rescale or have an option if it should. Aerial images looks best if they are rescaled, but tiled vector maps like OSM probably looks better without scaling. I think most users would choose a blurry image before a crisp image that has too small texts to be read. Or just leave it like it is and take that in another issue later.

OSM should be exempt since there/if there are no higher resolution tiles available, it seems right now it is exempt from the new mechanism and employs the old "rescaling" technique. I agree that a blurry OSM is probably better, that might be the case for every kind of background layer where there are no known higher res tiles (it's a slightly tricky subject on its own).

Cached WMS background maps look good (gwc-geoserver integration) with this change. There are multiple more cases and they belong to the above subject.

As for image and pdf I agree that the image option should probably be hidden as the PR currently stands. As for pdf prints they do have a document size as you say and to some degree it is up to the individual doing the printing to make sure they know how to handle pdf documents, though we have elected to include the paper size chosen in the pdf in our current print module, perhaps that might be an option here too.

@steff-o
Copy link
Contributor

steff-o commented Jan 17, 2022

Yes, a vector layer will almost certainly have a style in a production map in order to be useful. But as the default behavior is to use a simple blue symbol for layers without style this PR introduces a non backwards compatible change, which forces the configuration to be changed if no style was used. It could also be a problem for dynamically added layers and dynamically added styles if origo returns 'default' for layers without style set by origo configuration. As we know little or nothing about these unnamed styles, the simple solution would be to just ignore them from rescaling without crashing.

@jokd
Copy link
Contributor

jokd commented Jan 27, 2022

I have tested with wms and wfs services from a Geoserver and it works well. Even image export looks OK

@Grammostola
Copy link
Contributor

Grammostola commented Jan 28, 2022

Save image at 300DPI makes the layout elements mismatch somewhat with the map (position wise). Not all of them are rescaled either. I haven't seen a crash. Save image isn't lost but it is a tad worse for the wear. I'll see about making it optional.

@Grammostola
Copy link
Contributor

I can push a commit that introduces a suppressNewDPI property for the print control and works kind of like "supressResolutionsRecalculation". It currently doesn't disable or remove the Spara bild button in the print toolbar.. which leads me to my question. The PR does this at 300DPI to Save image; some layout elements are now properly sized and not tiny but not all and some are drawn beside the map like the north arrow:
till_bild_m_pr1_300dpi_mindre

Whether this is acceptable for someone seeking a 300DPI print is ...up to someone. Since this PR fixes the rescaling at higher DPI for WMS background maps, the map is an improvement but if the layout is crucial it is obviously unacceptable. The way I see it the query is to whether to push the commit, document the new property and if someone would like the old Save image back they can use the property to disable the functionality of this PR, or whether the above is acceptable for now and to be fixed later, or whether to try to disable Save image when 300DPI is chosen (with or without the suggested property)

@jokd
Copy link
Contributor

jokd commented Feb 9, 2022

I suggest we merge this with it's minor flaws. Anýone disgree?
@Grammostola If you are with me please resolve the conflicts

@Grammostola
Copy link
Contributor

We could merge as is and deal with known and potentially unknown issues further on, or I can add the param to disable this PR's functionality to this PR first

@jokd
Copy link
Contributor

jokd commented Feb 10, 2022

@Grammostola You can decide what to do^^

@Grammostola Grammostola merged commit e44b9bc into origo-map:master Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Symbols getting smaller when printing higher resolutions
5 participants