On Fri, 22 Mar 2024 at 12:25, Akihiko Odaki <akihiko.od...@daynix.com> wrote:
>
> On 2024/03/22 21:22, Peter Maydell wrote:
> > On Mon, 18 Mar 2024 at 07:53, Akihiko Odaki <akihiko.od...@daynix.com>
> > wrote:
> >>
> >> [NSWindow setContentAspectRatio:] does not trigger window resize itself,
> >> so the wrong aspect ratio will persist if nothing resizes the window.
> >> Call [NSWindow setContentSize:] in such a case.
> >>
> >> Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen")
> >> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> >> ---
> >> ui/cocoa.m | 23 ++++++++++++++++++++++-
> >> 1 file changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/ui/cocoa.m b/ui/cocoa.m
> >> index fa879d7dcd4b..d6a5b462f78b 100644
> >> --- a/ui/cocoa.m
> >> +++ b/ui/cocoa.m
> >> @@ -508,6 +508,25 @@ - (void) drawRect:(NSRect) rect
> >> }
> >> }
> >>
> >> +- (NSSize)fixAspectRatio:(NSSize)original
> >> +{
> >> + NSSize scaled;
> >> + NSSize fixed;
> >> +
> >> + scaled.width = screen.width * original.height;
> >> + scaled.height = screen.height * original.width;
> >> +
> >> + if (scaled.width < scaled.height) {
> >
> > Is this a standard algorithm for scaling with a fixed
> > aspect ratio? It looks rather weird to be comparing
> > a width against a height here, and to be multiplying a
> > width by a height.
>
> Not sure if it's a standard, but it's an algorithm with least error I
> came up with.
OK. Maybe a comment would help (at least it helps me in thinking
through the code :-))
/*
* Here screen is our guest's output size, and original is the
* size of the largest possible area of the screen we can display on.
* We want to scale up (screen.width x screen.height) by either:
* 1) original.height / screen.height
* 2) original.width / screen.width
* With the first scale factor the scale will result in an output
* height of original.height (i.e. we will fill the whole height
* of the available screen space and have black bars left and right)
* and with the second scale factor the scaling will result in an
* output width of original.width (i.e. we fill the whole width of
* the available screen space and have black bars top and bottom).
* We need to pick whichever keeps the whole of the guest output
* on the screen, which is to say the smaller of the two scale factors.
* To avoid doing more division than strictly necessary, instead
* of directly comparing scale factors 1 and 2 we instead
* calculate and compare those two scale factors multiplied by
* (screen.height * screen.width).
*/
Having written that out, it seems to me that the variable
names here could more clearly reflect what they're doing
(eg "screen" is not the size of the screen we're displaying
on, "original" is not the old displayable area size but the
new one we're trying to fit into, scaled doesn't actually
contain a (width, height) that go together with each other,
and it doesn't contain the actual scale factor we're going to
be using either).
> >> + fixed.width = scaled.width / screen.height;
> >> + fixed.height = original.height;
> >> + } else {
> >> + fixed.width = original.width;
> >> + fixed.height = scaled.height / screen.width;
> >> + }
> >> +
> >> + return fixed;
> >> +}
> >> +
thanks
-- PMM
