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

osc: font scaling option for timecodes and titlebar #3952

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@bitingsock
Contributor

bitingsock commented Dec 28, 2016

adds the option to scale the font size for timecodes and title so that fonts with larger kerning (monospace) don't overlap other elements (or any other reason).

I agree that my changes can be relicensed to LGPL 2.1 or later.

OSC font scaling
adds the option to scale the font size for timecodes and title so that fonts with larger kerning (monospace) don't overlap other elements (or any other reason).
@wm4

This comment has been minimized.

Contributor

wm4 commented Dec 28, 2016

Missing proper commit messages (why the fuck does nobody read the contribution guidelines?), missing documentation, I'm leaving it to @ChrisK2 and @Akemi to discuss whether this is a needed feature or if it's correctly implemented.

@bitingsock bitingsock changed the title from OSC font scaling so that fonts with larger kerning (monospace) don't overlap other elements to osc: font scaling option for timecodes and titlebar Dec 28, 2016

@bitingsock

This comment has been minimized.

Contributor

bitingsock commented Dec 28, 2016

I'm trying to conform it to the guidelines, unfortunately I suck at git so it's probably wrong. Here is the associated documentation change: c29b4c3

@wiiaboo

This comment has been minimized.

Member

wiiaboo commented Dec 28, 2016

Don't open another PR, commit to this one.

@bitingsock

This comment has been minimized.

Contributor

bitingsock commented Dec 28, 2016

Ok, got it thanks.

@ChrisK2

This comment has been minimized.

Member

ChrisK2 commented Dec 28, 2016

Wouldn't it be better to adjust the layouts accordingly? Can you provide screenshots and/or the font where this happens?

@bitingsock

This comment has been minimized.

Contributor

bitingsock commented Dec 28, 2016

untitled
I use Dejavu Sans Mono, but I imagine any monospace font would have similar problems.

@wiiaboo

This comment has been minimized.

Member

wiiaboo commented Dec 29, 2016

I did adjust them with DejaVu Sans Mono back then, but I didn't account for the negative sign.

I'm more for the fontscale option. Look at all that wasted space with a non-mono font:
image

@ChrisK2

This comment has been minimized.

Member

ChrisK2 commented Dec 29, 2016

all that wasted space with a non-mono font

... with a downscaled non-mono font.

Anyway, wasting some space is an inherent problem with that layout since we can't know text width, but I don't think that warrants another option. Also some extra space makes stuff less cluttered.

@wiiaboo

This comment has been minimized.

Member

wiiaboo commented Dec 29, 2016

Sure, I was comparing DejaVu Sans Mono with Source Sans Pro. Comparing with DejaVu Sans Bold they're mostly similar in width.

@wiiaboo

This comment has been minimized.

Member

wiiaboo commented Dec 29, 2016

If we're going for max width "just in case" we could use LastResort as base :^)
image
image

@wm4

This comment has been minimized.

Contributor

wm4 commented Jan 26, 2017

So do we want this or not?

@wiiaboo

This comment has been minimized.

Member

wiiaboo commented Jan 26, 2017

I'd be fine with increasing the width a bit to fit DejaVu Sans Mono, and also having an option in case someone wants a particularly wide font.

I've been using local tcW = (state.tc_ms) and 170 or 110 since this PR and it doesn't look too awful with Source Sans.

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Feb 4, 2017

osc: bottom/topbar: increase timecodes width a bit
Compensates for wider fonts like DejaVu Sans Mono.
Further compensate for the minus sign in the right timecode by 10px.

Closes mpv-player#3952

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Feb 5, 2017

osc: bottom/topbar: increase timecodes width a bit
Compensates for wider fonts like DejaVu Sans Mono.
Further compensate for the minus sign in the right timecode by 10px.

Closes mpv-player#3952

@wm4 wm4 force-pushed the mpv-player:master branch from e543a3d to 34b7d52 Feb 13, 2017

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Mar 24, 2017

osc: bottom/topbar: increase timecodes width a bit
Compensates for wider fonts like DejaVu Sans Mono.
Further compensate for the minus sign in the right timecode by 10px.

Closes mpv-player#3952

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Mar 24, 2017

osc: bottom/topbar: increase timecodes width a bit
Compensates for wider fonts like DejaVu Sans Mono.
Further compensate for the minus sign in the right timecode by 10px.

Closes mpv-player#3952

@wiiaboo wiiaboo closed this in aee08e3 Mar 24, 2017

atomnuker added a commit to atomnuker/mpv that referenced this pull request Jun 4, 2017

osc: bottom/topbar: increase timecodes width a bit
Compensates for wider fonts like DejaVu Sans Mono.
Further compensate for the minus sign in the right timecode by 10px.

Closes mpv-player#3952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment