Re: [PATCH] ui/cocoa: Fixed modeswitching glitch in zoomed fullscreen mode

[Imran Yusuff]

Hi,

Many thanks for the feedback. Actually I have investigated this by writing printfs in between statements to print out the 'self rect' values, and apparently the statement [normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x, ... modifies this 'self rect' values, for reason I am not sure why, hence the suggested additional statement to undo this. So yes, case 2 seems to make more sense, but it will not be executed in windowed (non-fullscreen) mode.

Many thanks for the code rewrite patch. It seems like a more proper work and I will definitely test this. I will get back to you soon once done.

Regards,

Imran Yusuff

On Sun, Jun 19, 2022 at 5:00 AM Akihiko Odaki <akihiko.odaki@gmail.com> wrote:

On 2022/06/19 9:41, Imran Yusuff wrote:
> This applies only on macOS using cocoa UI library.
>
> In zoom-to-fit fullscreen mode, upon graphics mode switch,
> the viewport size is wrong, and the usual consequence
> is only a part of the screen is visible. One have to exit
> and reenter fullscreen mode to fix this.
>
> This is reproducible by setting up a Windows 3.11 system,
> booting into DOS, enable zoom-to-fit, enter fullscreen mode and
> start Windows by 'win'. Then you can see only part of the screen.
>
> This commit fixes this problem, by including one line of code
> which is from the fullscreen mode initialization.
>
> Signed-off-by: Imran Yusuff <imranyusuff@gmail.com>
> ---
>   ui/cocoa.m | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 84c84e98fc..bd602817cd 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -636,6 +636,7 @@ - (void) switchSurface:(pixman_image_t *)image
>       if (isFullscreen) {
>           [[fullScreenWindow contentView] setFrame:[[NSScreen mainScreen] frame]];
>           [normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x, [normalWindow frame].origin.y - h + oldh, w, h + [normalWindow frame].size.height - oldh) display:NO animate:NO];
> +        [self setFrame:NSMakeRect(cx, cy, cw, ch)];
>       } else {
>           if (qemu_name)
>               [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]];

[self setFrame:NSMakeRect(cx, cy, cw, ch)] already exist in the earlier
part of the method. It is redundant and not a proper way to fix. It is
necessary to understand why adding the same statement fixes the problem,
and to fix it without duplicate statements. I can think of two
possibilities:
1. The isResize variable is not enough to cover all the situations when
the statement needs to be executed.
2. The statement needs to be executed after [normalWindow setFrame:].

In case 1, you need to assign a correct value to isResize or add [self
setFrame:] to somewhere else, but not in [-CocoaView switchSurface].
Putting the statement in [-CocoaView switchSurface] may cause redundant
execution of the statement when isResize is true as I explained.

In case 2, the existing statement should be simply moved.

By the way, I have a patch to rewrite the code implementing full screen
so you may try it. If you tested the patch, please give Tested-by.
https://patchew.org/QEMU/20220316060244.46669-1-akihiko.odaki@gmail.com/

Regards,
Akihiko Odaki