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

Refactor QgsTextRenderer internal methods, optimise buffer render #59414

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nyalldawson
Copy link
Collaborator

@nyalldawson nyalldawson commented Nov 12, 2024

Rework internals of QgsTextRenderer horizontal text rendering, so that:

  1. It's easier to read, cleaning up years of accumulated cruft
  2. Split out bits into smaller functions
  3. Reduce the amount of duplicate code for text layout, so that there's only one function responsible for horizontal text layout instead of multiple
  4. Add shortcut optimisations for rendering text + buffer/shadow at the same time, instead of always drawing these completely independantly of each other and incurring the cost of text path calculation multiple times for the same bit of text. In a simple benchtest this reduces the time required for rendering many text fragments with buffers from 49 seconds to 27 seconds.

This also fixes situations where text shadow could be slightly cropped, with a couple of pixels (or more) missing from the edges.

All test mask + reference image updates are correct, and have been checked careful to ensure no regressions (just slightly different subpixel text placement, or improvements over the previous reference images)

There's still more we could do here, but it's a start...

Rework internals of QgsTextRenderer horizontal text rendering,
so that:

1. It's easier to read, cleaning up years of accumulated cruft
2. Split out bits into smaller functions
3. Reduce the amount of duplicate code for text layout, so that
there's only one function responsible for horizontal text layout
instead of multiple
4. Add shortcut optimisations for rendering text + buffer/shadow
at the same time, instead of always drawing these completely
independantly of each other and incurring the cost of text path
calculation multiple times for the same bit of text. In a simple
benchtest this reduces the time required for rendering many
text fragments with buffers from 40 seconds to 27 seconds.

There's still more we could do here, but it's a start...
@nyalldawson nyalldawson added Optimization I feel the need... the need for speed! Cleanup Code cleanup labels Nov 12, 2024
@github-actions github-actions bot added this to the 3.42.0 milestone Nov 12, 2024
Copy link

github-actions bot commented Nov 12, 2024

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit a96d228)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit be4dc03)

@qgis qgis deleted a comment from github-actions bot Nov 12, 2024
@qgis qgis deleted a comment from github-actions bot Nov 12, 2024
@qgis qgis deleted a comment from github-actions bot Nov 13, 2024
Copy link
Contributor

@nirvn nirvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this morning but forgot to hit the button.

src/core/textrenderer/qgstextrenderer.cpp Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code cleanup Optimization I feel the need... the need for speed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants