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

Discrepancy between element dimensions in pixels on figma designs and dimensions in game units #17940

Open
bdach opened this issue Apr 22, 2022 · 8 comments

Comments

@bdach
Copy link
Collaborator

bdach commented Apr 22, 2022

Type

Cosmetic

Bug description

I've mentioned this several times over, but opening as an issue at @peppy's request.

To implement screen scaling, the game wraps all its content in a DrawSizePreservingFillContainer:

osu/osu.Game/OsuGameBase.cs

Lines 321 to 332 in 7d338fb

Child = CreateScalingContainer().WithChildren(new Drawable[]
{
(MenuCursorContainer = new MenuCursorContainer
{
RelativeSizeAxes = Axes.Both
}).WithChild(content = new OsuTooltipContainer(MenuCursorContainer.Cursor)
{
RelativeSizeAxes = Axes.Both
}),
// to avoid positional input being blocked by children, ensure the GlobalActionContainer is above everything.
globalBindings = new GlobalActionContainer(this)
})

By default, DrawSizePreservingFillContainer.TargetDrawSize is 1024x768, which is a 4:3 ratio.

For desktop intents and purposes, it is not likely that the screen is going to be any "narrower" wrt aspect ratio than that (typical aspect ratios are 4:3 and 16:9, rarely 21:9 or wider), which means that most of the time the container is going to size its children relative to a full height of 768px. For instance, that yields a 1366x768 resolution at 16:9. Please see screenshot below as corroboration of this:

scaling-1

However, this also implies that anything that is a child of the scaling container is no longer using real pixels to scale, but faux-"pixels" relative to 768px height. This means, that if figma designs that specify sizes in pixels are taken at face value and values are copied over verbatim, on typical resolutions the end result is going to be 1080/768 = 1.4x larger than they might have been (were?) intended to be at design time. Please refer to screenshot below as corroboration of this:

scaling-2

This means that decreasing all figma sizes by a factor of 1.4x should yield in a pixel-for-pixel match. Unfortunately there is a complication for this, namely ppy/osu-framework#3271 - the full paper trail is in that issue, but the TL;DR is that as it turns out, framework font sizes in pixels are too large relative to CSS pixel font sizes due to typography foibles, which means that fonts are partially exempt from this downscale operation.

I can see two immediate not-great resolutions:

  • What I've been doing since the start of the mod overlay work (and it's been pretty much only me thus far, at least when it comes to stuff that has actually been merged) is manually downsizing every single measurement by hand by a factor of 1.4x, with the font exemption.
  • The alternative would be to downscale UI globally by using the UI scale slider, so that new 1x is where current 0.71x is (0.71=1/1.4). I'm mentioning this as an option because it was brought up in discord, but this also means that all of the other stuff that looked "fine" at eye value will become potentially too tiny to use after said adjustment.

I don't claim to know what the answer is. It's all a bit of a drag.

CC'ing potentially directly interested parties: @jai-x, @GSculerlor, @frenzibyte, @arflyte (that last one is mostly just for awareness, I don't expect this to be addressed at design level at all)

Screenshots or videos

added screenshots in description above

Version

n/a

Logs

n/a

@peppy
Copy link
Member

peppy commented Apr 23, 2022

So as far as I can see, if we want to allow for 1:1 use of design metrics in the game, adjusting the value of DrawSizePreservingFillContainer and adjusting UI scale in a migration to ensure users get the same experience is am amicable solution, with the only contention being around the font size discrepancy?

I'm wondering if we can fix that in OsuFont (manually specifying the ratios to align them to expectations) until we have a proper framework solution.

@frenzibyte
Copy link
Member

I'm wondering if we can fix that in OsuFont (manually specifying the ratios to align them to expectations) until we have a proper framework solution.

That will require updating the font size of all OsuSpriteTexts we declared across the entire game, or creating a whole font family altogether that has this specific ratio in mind.

But I already have a simplified framework solution which I'll look into PR'ing in just a bit. Short of a boolean in FontUsage which would decide whether the font size measurement should be in absolute pixels (as we currently have it), or CSS-style font size (how we should have it).

@bdach
Copy link
Collaborator Author

bdach commented Apr 23, 2022

adjusting the value of DrawSizePreservingFillContainer and adjusting UI scale in a migration to ensure users get the same experience is am amicable solution

I don't follow why both would be simultaneously required. I think either option would fix on its own - increasing target draw size of the container to 1440x1080 (which is the 4:3 resolution that is 1080px high) will lead to children automatically rendering as smaller, so at that point there should be no need to adjust UI scale.

Unless the goal would be to increase UI scale back up to preserve behaviour...?

@peppy
Copy link
Member

peppy commented Apr 23, 2022

I meant adding a migration for UI scale so that existing users don't get their scale suddenly changed, mainly.

@bdach
Copy link
Collaborator Author

bdach commented Apr 23, 2022

Does this look close to what you'd expect to see? https://github.com/ppy/osu/compare/master...bdach:ui-scale-to-design-size?expand=1

@peppy
Copy link
Member

peppy commented Apr 25, 2022

Yep, looks about correct. I'd also probably want to do a pass over all existing UI to make sure nothing is unexpectedly too small/large at the default 1.0x.

@bdach
Copy link
Collaborator Author

bdach commented Apr 25, 2022

Unfortunately no, with the changes above the new 1.0x scale is a bit unusable, especially around online overlays and their tiny text (another symptom of the CSS-incompatible text sizing issue....)

osu_2022-04-25_23-01-03
osu_2022-04-25_23-01-09

Not sure what to do about this. Easiest thing for now would probably be to temporarily move the default UI scale to 1.4x too, until all of that sizing stuff gets sorted...

@peppy
Copy link
Member

peppy commented Apr 26, 2022

Either way, we should probably wait for the text scaling changes to be finalised framework side before applying fixes at this side.

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

No branches or pull requests

3 participants