On Mon, 18 Dec 2023, Akihiko Odaki wrote:
On 2023/12/17 20:39, BALATON Zoltan wrote:
On Sun, 17 Dec 2023, Akihiko Odaki wrote:
This change brings two new features:
- The window will be resizable if "Zoom To Fit" is eanbled
- The window can be made full screen by clicking full screen button
provided by the platform. (The left-top green button.)
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
Tested-by: Rene Engel <reneenge...@emailn.de>
---
V5 -> V6:
Rebased.
---
Changes in v7:
- Fixed zoom-to-fit option. (Marek Glogowski)
- Link to v6:
https://lore.kernel.org/r/20231211-cocoa-v6-1-49f3be019...@daynix.com
---
ui/cocoa.m | 542
+++++++++++++++++++++++++++++--------------------------------
1 file changed, 258 insertions(+), 284 deletions(-)
Is ir possible to break this patch up into smaller ones for easier review?
E.g. separate patch moving mouse event handling out of handleEventLocked,
replacing stretch_video flag with NSWindowStyleMaskResizable and whatever
else can be done as independent steps? Not sure if that's possible or needs
the whole chnage at once but this patch seems to be too big. Some more
comments below.
I split it into three patches with v8, but most changes are still in one
patch because they depend on the change to unify the full screen window and
normal window and vice-versa.
It still helps to remove unrelated changes from the big patch so it's
more obvious what's needed for that change and thus easier to review.
[...]
@@ -513,36 +504,43 @@ - (void) drawRect:(NSRect) rect
}
}
-- (void) setContentDimensions
+- (NSSize) fixZoomedFullScreenSize:(NSSize)proposedSize
{
- COCOA_DEBUG("QemuCocoaView: setContentDimensions\n");
+ NSSize size;
- if (isFullscreen) {
- cdx = [[NSScreen mainScreen] frame].size.width /
(float)screen.width;
- cdy = [[NSScreen mainScreen] frame].size.height /
(float)screen.height;
+ size.width = (CGFloat)screen.width * proposedSize.height;
+ size.height = (CGFloat)screen.height * proposedSize.width;
One of these will be overwritten in the next if below so maybe drop this
init and do the calculation in the if legs which is then also clearer to
show that this would scale one of these with screen.width/screen.height or
the inverse of that.
This also removes stretch_video flag and the calculation to preserve aspect
ratio. Is that correct? Would it now distort the image when zooming to full
screen if guest resolution is not the same as host screen? Is that how
zoom-to-fit should work? At leest with -display sdl going to full screen
guest screen is zoomed preserving aspect ratio but maybe sdl does not have
zoom-to-fit option. I don't know how it works with other displays such as
gtk.
The purpose of this method is to fix the aspect ratio for zoom-to-fit by
shrinking width or height. It operates in the three steps:
1. Compute the values necessary either for shrinking width or height.
2. Decide which of width or height to shrink.
3. Compute the final values.
Wouldn't it be simpler to do in two steps:
1. Decide what needs to be scaled
2. Do the computation accordingly
I.e.
if (size.width < size.height) {
size.width = proposedSize.height * (CGFloat)screen.width / screen.height;
size.height = proposedSize.height;
} else {
size.width = proposedSize.width;
size.height = proposedSize.width * (CGFloat)screen.height / screen.width;
}
Seems to me more explicit than doing the scale factor calculation split in
two lines that's harder to follow. I had to think about what that does
while this shows it clearer. If you prefer shorter lines you could also
init size = proposedSize and then scale either width or height afterwards
which would still be clearer than your way I think.
Regaards,
BALATON Zoltan
