-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Greatly optimise selective masking vector exports #57799
Conversation
@troopa81 here's the next optimisation for selective masking exports. I consider this one the "holy grail", which squashes the last of the remaining big limitations of the vectorised selective masking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a first quick review, I'll try to finish it in a few hours.
Having a tailored mask for each feature is interesting, but I think it brings a lot of complexity to the code (specially in QgsSymbol::renderFeature).
Like I said before, I'm wondering if we could not have improved Qt by sharing/referening clippingPath when writing PDF. This way, we would have had one global mask (maybe built and simplified using the geometry paint device) applied to all features. In the end, the crash appears when serializing, not painting.
Therefore, the more feature you have, the bigger the file will be because you multiply masks, while with a global mask your will size stay constant.
Allows us to know in startRender whether the symbol is actually a subsymbol for a QgsSymbolLayer. Currently unused.
Optimise the logic used when the new geometry backend for selective masking is in effect: Whenever its SAFE, instead of calculating an "entire map" clipping path and then applying this for every feature being rendered, we now defer the calculation of the clipping path until we are rendering individual features. Then, we create a clipping path which contains ONLY the mask paths which are within the area being drawn over. This avoids having the entire map clipping path being used for EVERY feature being rendered, which results in huge PDF/SVG exports when masks are in effect, and instead results in clipping paths which are confined just to a sensible area around each rendered feature. In some complex test projects this reduces the PDF export size by a factor of 0.01!! (and results in PDFs/SVGs which open much quicker in viewers and editors, and don't grind their operation to a halt).
These are the symbol layer classes where there's no special logic required relating to feature rendering and features are rendered one-by-one, with no sub symbols.
Thanks!
That sounds good, and it would definitely be nice to see this happen in some future Qt release! 👍 I've now added a render context/layout flag which forces the "entire map" clipping paths to always be used. It's opt in, but if Qt does improve the situation then we'll easily be able to switch back by just enabling this flag. (At some stage I'd love to make some advanced dialog for configuring layout export presets, and then we could even expose this setting to users who want fine control over this, and all the other mask simplification parameters too). Pragmatically though, there's no related Qt improvements in current Qt 6 versions and use of local clip masks fixes the bulk of the issues with exporting complex layouts, so we should make this default behaviour for now.
It's caused during serializing the painter state, because the clip path is too complex for the structures Qt internally is using to do this. So we'd actually need two upstream improvements -- one would be the "write clip path once when exporting to SVG/PDF" improvement, and the other would be "adapt QPainter state storage to handle complex clip paths". On the other hand, with the changes here I can confirm that use of local clip paths fixes all the crashing projects from the linked tickets with current Qt stable versions. |
This is a new opt-in flag for map settings/render context/layouts. If set, then when applying clipping paths for selective masking, we always use global ("entire map") paths, instead of calculating local clipping paths per rendered feature. This results in considerably more complex vector exports in all current Qt versions, but gives us a way to force this IF/when a future Qt version adds optimisations which make global masks desirable.
Instead of the extra argument to startRender
Yes, it's relating to that qt issue regarding paths bleeding out of the map frame's clip area. I'm fairly certain I can fix that now by doing the intersection of map item clip path and selective masking clip path via the geometry approach, but I'll do that in a different PR. And if it works I'll regenerate all the reference images so that none of them have the bleed anymore. |
Optimise the logic used when the new geometry backend for selective masking is in effect:
Whenever its SAFE, instead of calculating an "entire map" clipping path and then applying this for every feature being rendered,
we now defer the calculation of the clipping path until we are rendering individual features. Then, we create a clipping path
which contains ONLY the mask paths which are within the area being drawn over.
This avoids having the entire map clipping path being used for EVERY feature being rendered, which results in huge PDF/SVG
exports when masks are in effect, and instead results in clipping paths which are confined just to a sensible area
around each rendered feature.
In some complex test projects this reduces the PDF export size by a factor of 0.01, without any loss of quality or regressions in export time!! (Eg 60Mb -> 700kb). It also results in PDFs/SVGs which open much quicker in viewers and editors, and don't grind their operation to a halt.
Fixes #50734
Fixes #54788