osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

Expose tablet OutputSize

Open DanielPower opened this issue 1 year ago • 8 comments

Resolves #6150 Related to https://github.com/ppy/osu/issues/12098 Required by https://github.com/ppy/osu/pull/31141

Description

Exposes new bindables OutputAreaSize and OutputAreaPosition, so that absoluteOutputMode.Output can be updated from osu.Game. This is used by https://github.com/ppy/osu/pull/31141 to scale the tablet output when screen scaling is set to "Everything".

DanielPower avatar Dec 16 '24 04:12 DanielPower

Seems like a duplicate of https://github.com/ppy/osu-framework/pull/6166?

hwsmm avatar Dec 16 '24 05:12 hwsmm

Seems like a duplicate of #6166?

That's a good catch. I can close this and instead use #6166 as a base for https://github.com/ppy/osu/pull/31141 if that would be preferable. Their PR is more complete as it also includes the output position, which may be useful for future configurability.

DanielPower avatar Dec 16 '24 05:12 DanielPower

#6166 is too outdated to work off of directly due to conflicts with osu.Game. So I've:

  1. merged @Feodor0090's changes into this branch
  2. updated https://github.com/ppy/osu/pull/31141 accordingly
  3. Updated some wording and simplified logic

Apologies, this has made the commit history a little messy. I can rebase if desired, but I'll leave it as-is for now since CONTRIBUTING.md discourages rebasing.

DanielPower avatar Dec 17 '24 01:12 DanielPower

aside from the above this looks roughly correct, but was there even any testing done here? no changes to test scenes to even add sliders to play with this framework side or anything, so i question whether this was tested to work at all.

bdach avatar Dec 26 '24 11:12 bdach

I assumed cross-testing with https://github.com/ppy/osu/pull/31141, which looks to be working well for at least the user. I haven't personally tested, if we can have some minimal framework tests that definitely would be appreciated.

peppy avatar Dec 26 '24 11:12 peppy

but was there even any testing done here?

I've cross-tested with https://github.com/ppy/osu/pull/31141 and included a video in that PR demonstrating the new behaviour. I included a note in that PR asking for advice on how to do testing (since automated testing would require a way to mock tablet input). But I should have included that note on this PR as well.

I'll look into adding some sliders to the test framework, so that it can be tested manually.

Edit: Adding sliders to osu-framework's visual testing was a great callout. The behaviour isn't quite what I was expecting on the framework side, and I was compensating for it on the osu-game side. I'll temporarily draft this PR, and will aim to work out a better solution shortly after New Years.

DanielPower avatar Dec 31 '24 01:12 DanielPower

Sorry it's taken so long to get back to this. It's much later than the "Shortly after New Years" that I promised 😅

Changes since the last review include:

  • Added testing sliders for output area size, output area offset, and rotation.
  • Addressed positioning bugs from the previous implementation.
  • Removed the unnecessary call to updateOutputArea.
  • Added default values for OutputAreaSize and OutputAreaOffset.
  • Merged in the latest master branch, since there were conflicts.

On the note of the default values, I found that when restarting the test framework, if there are no sliders for the values, the last used values will be used instead of the default values. I'm assuming there's a config file somewhere storing these values that I should be clearing when testing. I'm currently trying to figure out where that is, so I can validate that the default values will work correctly on a fresh or existing installation.

One thing this PR hasn't changed is the tablet area visualization. Output area and rotation are not reflected in the visualization.

Edit: I've now included a tablet area visualization for output area.

DanielPower avatar Jun 18 '25 23:06 DanielPower

As a bit of a tangent, I implemented OutputAreaOffset so that the output area remains within the bounds of the screen. (0, 0) is aligned to the top-left, and (1, 1) is aligned to the bottom-right. This is different from the existing input area implementation, where you position the center of the area directly, allowing the input area to be partially outside the tablet.

Screenshot 2025-06-18 at 9 39 01 PM

I think it would make sense to adjust the input area to fix this behaviour. But it would cause differences for anyone who has changed their offset unless we could migrate saved config values. It's outside the scope of this PR, but figured I'd mention it.

DanielPower avatar Jun 19 '25 00:06 DanielPower

@peppy This PR is ready for review. The related PR https://github.com/ppy/osu/pull/31141 is also ready for review, but I've left it in draft for now since it depends on the framework changes from this PR.

If there's anything further I can do in terms of documentation, testing, etc. that would help this get merged, please let me know.

DanielPower avatar Aug 17 '25 01:08 DanielPower

@bdach can you give this 5 minutes to check you're okay with it now?

peppy avatar Aug 22 '25 03:08 peppy