Re: [PATCH v2 1/1] ui/cocoa: add zoom-to-fit display option

[Carwyn Ellis]

> On 27 Oct 2023, at 16:24, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> On 2023/10/28 0:09, carwynellis@gmail.com wrote:
>> From: Carwyn Ellis <carwynellis@gmail.com>
>> Provides a display option, zoom-to-fit, that enables scaling of the
>> display when full-screen mode is enabled.
>> Also ensures that the corresponding menu item is marked as enabled when
>> the option is set to on.
>> Signed-off-by: Carwyn Ellis <carwynellis@gmail.com>
>> ---
>>  qapi/ui.json |  8 ++++++--
>>  ui/cocoa.m   | 35 ++++++++++++++++++++---------------
>>  2 files changed, 26 insertions(+), 17 deletions(-)
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 006616aa77..fd12791ff9 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1409,13 +1409,17 @@
>>  #     codes match their position on non-Mac keyboards and you can use
>>  #     Meta/Super and Alt where you expect them.  (default: off)
>>  #
>> -# Since: 7.0
>> +# @zoom-to-fit: Scale display to fit when full-screen enabled.
>> +#     Defaults to "off".
>> +#
>> +# Since: 8.2
> 
> I don't think this new option will affect only when full-screen enabled, but 
> probably it will affect also in a windowed mode. Perhaps you can just copy 
> the description for DisplayGTK except the statement regarding virtio-gpu.

Ok.

> 
> Also don't replace "Since: 7.0". It denotes the version that introduced the 
> structure, not an individual member.

Ok, I’ll flip it back to 7.0.

> 
>>  ##
>>  { 'struct': 'DisplayCocoa',
>>    'data': {
>>        '*left-command-key': 'bool',
>>        '*full-grab': 'bool',
>> -      '*swap-opt-cmd': 'bool'
>> +      '*swap-opt-cmd': 'bool',
>> +      '*zoom-to-fit': 'bool'
>>    } }
>>    ##
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index d95276013c..903adb85a1 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -104,7 +104,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>>  static int left_command_key_enabled = 1;
>>  static bool swap_opt_cmd;
>>  -static bool stretch_video;
>> +static bool stretch_video = false;
> 
> You don't need to assign false here. C initializes it as false by default.

Ahh of course.

Thanks again for getting back to me so quickly! :)