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

fix(qml): Improve knobs by applying selective 4xMSAA on the Arc shape #12541

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jan 9, 2024

This is way less resource-consuming than doing MSAA on the entire GUI.

Fixes #12536.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jan 9, 2024

@acolombier please have a look if that works for you, too

@Swiftb0y
Copy link
Member

Swiftb0y commented Jan 9, 2024

lgtm, but enabling layers for an item that might be onscreen often may also have performance issues. https://doc.qt.io/qt-6/qml-qtquick-item.html#memory-and-performance

Also, an item using a layer can not be batched during rendering. This means that a scene with many layered items may have performance problems.

Layering can be convenient and useful for visual effects, but should in most cases be enabled for the duration of the effect and disabled afterwards.

@Holzhaus
Copy link
Member Author

Sure, but practically I didn't notice any performance issues on my laptop (in contrast to using 16xMSAA globally which made the QML completely unresponsive and unusable for me).

@acolombier
Copy link
Member

acolombier commented Jan 10, 2024

@Holzhaus it works but the issue is that other component also shows aliasing (see the waveform pre-roll triangle in the issue). Would we want to apply local multi sampling to each components subject to aliasing? Also, do we want to hardcode the multi-sampling level? Alternatively, from @Swiftb0y's comment, I've started some work to allow setting a custom multi-sampling level in the setting

Edit: hardcode the multi-sampling level - perhaps this could still use a configurable value and resolve it like the here?

@Swiftb0y
Copy link
Member

I'd probably say both. I'm guessing that as long as the offscreen framebuffers are smaller than the screen, doing AA per component is probably cheaper.

Would we want to apply local multi sampling to each components subject to aliasing?

It depends on the skin, I'd say that on overage 50% of screen content probably needs AA (for 2 decks, maybe 80% for 4 decks), but when the library is maximized that drops down to 10% as the library is pretty square anyways and text is already AA'd anyways. Doing AA for the entire application in that case is probably quite wasteful. So only doing it per-component would avoid the overhead when those components aren't shown (theoretically, not sure if Qt Quick is implemented like that).

@Holzhaus have you measured doing just 4xMSAA on the entire GUI? afaik that should only have 1/4th of the AA overhead while still providing nice results.

Anyways, I think its too soon to decide purely based on performance. The QML gui is already not super well optimized (lots of draw calls, suboptimal batching due to the abundance of transparent vector graphics) so I think we can postpone optimization work like that for later.

@Holzhaus
Copy link
Member Author

No, I didn't measure anything. I just tested this commit Holzhaus@057ca11 and the qml screen remained blank for roughly 10 secs before it started to render, and even then it didn't react to changes and was basically too laggy to be usable.

And that was with 8x MSAA (I saw a warning message on the terminal that 16 is not supported and 8 is used).

With this patch the knobs look nicer and didn't notice a performance downgrade.

@Swiftb0y
Copy link
Member

I'm wondering whether the fallback to 8xMSAA is an indicator for a driver issue... I'd be surprised if 2020 hardware can't handle 16x.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Thank you. This should probably be lightweight enough for most systems. @acolombier volunteered to build upon this in #12546 to make the quality configurable.

@Swiftb0y Swiftb0y merged commit 66f3f89 into mixxxdj:main Jan 11, 2024
12 checks passed
@ronso0
Copy link
Member

ronso0 commented Jan 11, 2024

🎉

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.

QML UI shows aliasing on certaint components
4 participants