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(MarkableTimeBar): align segments to timebar #7055

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

FineFindus
Copy link
Contributor

The segments drawn on the timeline seem to be off by one pixel. I'm not quite sure what's causing this, as the calculation is done differently to the timeline itself.
Since the calculation depends on the device, and I haven't been able to test this on multiple devices, further testing should be done before merging.

Misaligned segment

@Bnyro
Copy link
Member

Bnyro commented Feb 3, 2025

IMG_20250203_181108

That seems to be off for 1 or 2 pixels on my device :/

@FineFindus
Copy link
Contributor Author

Damn :(
Looks a bit like this is exactly the pixel that is being moved here:

val marginY = canvas.height / 2 - progressBarHeight / 2 - 1

Was it aligned correctly before? I guess this means we need to follow the height calculation more closely (or fork and make the height public; that would also enable #4575 :))

@Bnyro
Copy link
Member

Bnyro commented Feb 3, 2025

Was it aligned correctly before?

It was for me, yes.

I guess this means we need to follow the height calculation more closely

Yes, there's probably some error in the general way we try to calculate the height with, probably rebuilding that calculation from scratch makes sense.

@Bnyro Bnyro marked this pull request as draft February 10, 2025 17:01
@FineFindus FineFindus force-pushed the fix/timebar-alignment branch from 9e5cd92 to 2c556ab Compare February 15, 2025 18:22

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Due to a error in the timebar position calculation, segments could
be msialigned by a pixel. This is fixed by using a caculation that more
closely matches how the actual timebar is drawn.
@FineFindus FineFindus force-pushed the fix/timebar-alignment branch from 2c556ab to ea6483d Compare February 15, 2025 18:24
@FineFindus
Copy link
Contributor Author

FineFindus commented Feb 15, 2025

I've updated the calculation to be pretty much the same as in exoplayer. Could you please re-test?

@FineFindus FineFindus marked this pull request as ready for review February 15, 2025 18:31
Copy link
Member

@Bnyro Bnyro left a comment

Choose a reason for hiding this comment

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

That does the job, thank you!

@Bnyro Bnyro merged commit 47c3bf0 into libre-tube:master Feb 17, 2025
2 of 3 checks passed
@FineFindus FineFindus deleted the fix/timebar-alignment branch February 17, 2025 20:28
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.

None yet

2 participants