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

Waveform: fix shifted (text) markers whe using odd scale factors #3936

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jun 2, 2021

https://bugs.launchpad.net/mixxx/+bug/1917253

set an non-int scale factor like
export QT_SCREEN_SCALE_FACTORS=1.37
in the images below all three markers are actually exactly one the play position.

before
image

this PR
image

@ronso0 ronso0 added this to the 2.3.0 milestone Jun 2, 2021
@ronso0
Copy link
Member Author

ronso0 commented Jun 2, 2021

tbh I didn't follow all those confusing scale functions. so consider this a quick fix for the skins that are using text markers.

there's still something wrong with markers that use icons (Deere only, at least in 2.3):
range is correct, markers are shifted
image

@ronso0
Copy link
Member Author

ronso0 commented Jun 3, 2021

...and that appearantly depends on the icon width: the wider, the more it is shifted.
dunno how to fix it...the image shift function works, maybe there's an offset problem when pixmaps are loaded
https://github.com/mixxxdj/mixxx/blob/2.3/src/waveform/renderers/waveformrendermark.cpp#L149-L161

@daschuer
Copy link
Member

daschuer commented Jun 3, 2021

I can confirm the issue and the fix with 3 / 2.5 / 2 / 1.5 / 1:
before
grafik
after
grafik

On Ubuntu Groovy

@@ -146,6 +146,7 @@ void WaveformRenderMark::slotCuesUpdated() {

void WaveformRenderMark::generateMarkImage(WaveformMarkPointer pMark) {
// Load the pixmap from file -- takes precedence over text.
float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio();
if (!pMark->m_pixmapPath.isEmpty()) {
QString path = pMark->m_pixmapPath;
QImage image = *WImageStore::getImage(path, scaleFactor());
Copy link
Member

Choose a reason for hiding this comment

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

Here scaleFactor() is in use.
Which value has it?
I did not understand why scaleFactor() works here.
Does it?
When is the code used?

Copy link
Member Author

Choose a reason for hiding this comment

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

scaleFactor or devicePixelRatio makes no difference here.
It's 1.0 regardless of the Qt scale factor. some leftover from custom Mixxx scaling?
this is used when markers use icons, like in Deere (Cue, loop_in/out).
then the entire part below is skipped
= the icon needs to be centered so the icon stroke (if any) aligns with the marker position on the waveform

also, the one initial problem here causing the offset is that the marker sroke is not taken from the samplePosition but directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not understand why scaleFactor() works here.

must admit I don't understand it either.
The fixes apply to all QPainter-related calculations below.

Copy link
Member

Choose a reason for hiding this comment

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

Deere uses it for its cue marker.
I can confirm that it scales fine.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, Thnak you.

@daschuer daschuer merged commit 9e7e0c7 into mixxxdj:2.3 Jun 3, 2021
@ronso0 ronso0 deleted the waveform-markers branch June 3, 2021 16:00
@ronso0
Copy link
Member Author

ronso0 commented Jun 3, 2021

...and that appearantly depends on the icon width: the wider, the more it is shifted.
dunno how to fix it...the image shift function works, maybe there's an offset problem when pixmaps are loaded
https://github.com/mixxxdj/mixxx/blob/2.3/src/waveform/renderers/waveformrendermark.cpp#L149-L161

got it!
#3938

@ronso0 ronso0 changed the title Waveform: fix markers whe using odd scale factors Waveform: fix shifted (text) markers whe using odd scale factors Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants