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

mark rendering improvements #13969

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

mark rendering improvements #13969

wants to merge 2 commits into from

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Dec 3, 2024

These changes were originally made in the rendergraph PR (and in code already using rendergraph):
#13599
but I am splitting that PR into multiple ones. This is a OpenGL backport of those changes, but it will result in a smaller delta once I have the PR for the rendergraph base waveform mark renderers.

The improvements here are:

  • Not using empty space in the mark textures to align them. For left and right aligned markers, half the image would be fully transparent. With this PR only the space needed it uses, and the mark is drawn with an offset to align them to the marker position.

  • Round drawing to pixels. With OpenGL this doesn't seem necessary, but with QML / Qt Scenegraph this causes artifacts in the rendering.

Copy link
Member

@acolombier acolombier left a comment

Choose a reason for hiding this comment

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

This is looking good, excited to see the rendergraph effort starting to be merged 🚀

Looks like you got couple of missing cast and this is good to go IMO.

Comment on lines +138 to +140
qreal w = std::round(metrics.horizontalAdvance(text) *
devicePixelRatio / devicePixelRatio) +
space + space + 1.0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
qreal w = std::round(metrics.horizontalAdvance(text) *
devicePixelRatio / devicePixelRatio) +
space + space + 1.0;
qreal w = std::round(metrics.horizontalAdvance(text)) +
space + space + 1.0;

Or should the division be after the round?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants