Fix inconcequent use of DPI at generating the WMS legend #52015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Consider DPI of QgsWmsRenderContext what is the OGC default (0.28 mm per pixel) or the passed WMS parameter - this is done by creating the QgsRenderContext by the mapSettings (with BBOX) or applying the dotsPerMm to the scaleFactor. - Additionally the image size needs to be calculated according to the QgsRenderContext now, what means it needs to be generated before. - The QPainter needs to be applied after to the context (since it's not passed by creating the context anymore).
… context settings according to the mapSettings since it NEVER has a BBOX (since of RULE)
1 task
…correct CRS and care about meter based crs. This fixes qgis#50366
m-kuhn
reviewed
Feb 24, 2023
| @@ -186,6 +208,18 @@ namespace QgsWms | |||
| QgsLegendSettings settings = legendSettings(); | |||
| QgsLayerTreeModelLegendNode::ItemContext ctx; | |||
| ctx.painter = painter.get(); | |||
|
|
|||
| // create context | |||
| QgsRenderContext context = QgsRenderContext::fromQPainter( painter.get() ); | |||
There was a problem hiding this comment.
This block is identical to the one above?
Can it be deduplicated?
There was a problem hiding this comment.
I thought about that. Would you make a new method just for that? I decided against it, but would be fine for me.
There was a problem hiding this comment.
There's a certain risk of forgetting upgrades in the future. It aligns well with configureMapSettings(). I would.
|
I need to replace a bunch of control_images now. What is the appropriated way with |
If they are changes that are permanent and not platform specific, just replace the existing reference images directly and don't add variants or masks. I don't know if you've capacity for it, but all the server legend tests are broken in that they don't use the correct standard qgis test font and use tiny font sizes. They are really fragile as a result. If you're going to replace all the reference images and can find the time to do so, also changing the font to the standard test font in bold at least size 12 point would be a massive improvement. |
|
You mean by adding those parameters LAYERFONTBOLD, LAYERFONTSIZE, LAYERFONTFAMILY (and same ITEM) to the request of every getlegendgraphic-testcase? @nyalldawson? Yes. I can do that. |
…labels and having not yet a fix font with "LAYERFONTBOLD": "TRUE", "LAYERFONTSIZE": "12", "LAYERFONTFAMILY": self.fontFamily, "ITEMFONTBOLD": "TRUE", "ITEMFONTSIZE": "12", "ITEMFONTFAMILY": self.fontFamily
m-kuhn
reviewed
Mar 3, 2023
|
This can be merged right? And backported to 3.30..? |
m-kuhn
added
backport queued_ltr_backports
|
Yes, I have also tagged it for backport to LTR (remove label if you think it's a bad idea) |
signedav
added
backport queued_ltr_backports
|
The backport to stderrstdoutTo backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-queued_ltr_backports queued_ltr_backports
# Navigate to the new working tree
cd .worktrees/backport-queued_ltr_backports
# Create a new branch
git switch --create backport-52015-to-queued_ltr_backports
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 10aa91a6a6a64ab8fcafc7c8ef8a0a1eacef85e8,e667ca423636c84cc3ef8dd411461eb233d2a269,6ec0c75d6de011cc5d808631ed3c6dd00348d197,e1d43b01a860951bb3a4ba280b66320003a6d0c3,cc3c2ec0228ccf453cc3aa4a06f69d35f4663c9a,c90f7579fa670b6293bf94f1c3a4e57e7d2bd38b,2be8f073080aedc9e86e28b0a987c27195345b9d,7574039fd54045cff29c6af8c9c7cf3dc9804bb1,d3448f18369228fea891fd362a1a9fe14e5c4d76,1042c66e26b11659e248a86fbe9db882289b9071,a85019498fc6dd2d547b2cb06b1657dc687071a2
# Push it to GitHub
git push --set-upstream origin backport-52015-to-queued_ltr_backports
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-queued_ltr_backportsThen, create a pull request where the |
qgis-bot
added
the
failed backport
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Consider DPI of QgsWmsRenderContext what is the OGC default (0.28 mm per pixel) or the passed WMS parameter - this is done by creating the QgsRenderContext by the mapSettings (with BBOX) or applying the dotsPerMm to the scaleFactor.
Additionally the image size needs to be calculated according to the QgsRenderContext now, what means it needs to be generated before.
Distance area when using defaultMapUnitsPerMm to consider the correct CRS and care about meter based crs. This fixes WMS legend symbol missing for "Meters at Scale" symbology and projected CRS #50366
Replaces #51772