Re: [PATCH pve-manager v4 2/3] ui: qemu: add VirGl feature selection in DisplayEdit

[Dominik Csapak <d.csapak@proxmox.com>]

code looks good to me, but a small feedback on ux:

IMO i would prefer it to now show the virgl fields
at all when non virgl display types are selected,
this is distracting and the user does not gain anything
by always seeing options that don't make sense for the
majority of users.

So either only show them when virgl is selected
(should be easy, just bind on hidden too, not only disabled)
or move them to the advanced section.

Also i find it a bit weird that the memory label
chagnes too when virtio-gpu is selected.
It's not wrong per se, but the '2d memory limit' label
really only makes sense with the other virgl options together.

So i'd limit that change to viritio-gl gpus.

On 8/31/26 11:46 AM, Markus Frank wrote:
> The user can choose which feature set to use:
> - the default option with OpenGL 4.3 support
> - a option with OpenGL 4.6 support
> - a option with both OpenGL 4.6 and Vulkan support
> 
> As the feature sets other than the default require a VirGL memory
> window, the user can change the size of the default window in a
> different combo box.
> 
> Add a controller to reset the combo boxes when they cannot be used and
> add formulars to disable them.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
> v4: change the memory label to '2D Memory Limit' when using VirtIO to
>   make the difference to the 'Host Memory Window' clearer.
> 
>   www/manager6/qemu/DisplayEdit.js | 79 +++++++++++++++++++++++++++++++-
>   1 file changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js
> index 3f583adb..62624589 100644
> --- a/www/manager6/qemu/DisplayEdit.js
> +++ b/www/manager6/qemu/DisplayEdit.js
> @@ -11,10 +11,37 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>           return { vga: ret };
>       },
>   
> +    controller: {
> +        xclass: 'Ext.app.ViewController',
> +
> +        onTypeChange: function (type) {
> +            let me = this;
> +            let vm = me.getViewModel();
> +
> +            if (type.getValue() !== 'virtio-gl') {
> +                vm.set('virglMemoryWindow', '__default__');
> +                vm.set('virglFeatures', '__default__');
> +                vm.notify();
> +            }
> +        },
> +
> +        onFeaturesChange: function (features) {
> +            let me = this;
> +            let vm = me.getViewModel();
> +
> +            if (features.getValue() === '__default__') {
> +                vm.set('virglMemoryWindow', '__default__');
> +                vm.notify();
> +            }
> +        },
> +    },
> +
>       viewModel: {
>           data: {
>               type: '__default__',
>               clipboard: '__default__',
> +            virglFeatures: '__default__',
> +            virglMemoryWindow: '__default__',
>           },
>           formulas: {
>               matchNonGUIOption: function (get) {
> @@ -35,6 +62,15 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>                       return Proxmox.Utils.defaultText;
>                   }
>               },
> +            memoryLabel: function (get) {
> +                if (get('type').match(/^virtio/)) {
> +                    return gettext('2D Memory Limit') + ' (MiB)';
> +                } else {
> +                    return gettext('Memory') + ' (MiB)';
> +                }
> +            },
> +            isVirgl: (get) => get('type') === 'virtio-gl',
> +            virglIsDefault: (get) => get('virglFeatures') === '__default__',
>               isVNC: (get) => get('clipboard') === 'vnc',
>               hideDefaultHint: (get) => get('isVNC') || get('matchNonGUIOption'),
>               hideVNCHint: (get) => !get('isVNC') || get('matchNonGUIOption'),
> @@ -58,6 +94,7 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>                   }
>                   return true;
>               },
> +            listeners: { change: 'onTypeChange' },
>               bind: {
>                   value: '{type}',
>               },
> @@ -65,16 +102,50 @@ Ext.define('PVE.qemu.DisplayInputPanel', {
>           {
>               xtype: 'proxmoxintegerfield',
>               emptyText: Proxmox.Utils.defaultText,
> -            fieldLabel: gettext('Memory') + ' (MiB)',
>               minValue: 4,
>               maxValue: 512,
>               step: 4,
>               name: 'memory',
>               bind: {
> +                fieldLabel: '{memoryLabel}',
>                   emptyText: '{memoryEmptyText}',
>                   disabled: '{matchNonGUIOption}',
>               },
>           },
> +        {
> +            xtype: 'proxmoxKVComboBox',
> +            name: 'virgl-features',
> +            deleteEmpty: false,
> +            fieldLabel: gettext('VirGL Features'),
> +            listeners: { change: 'onFeaturesChange' },
> +            comboItems: [
> +                ['__default__', Proxmox.Utils.defaultText + ' (OpenGL 4.3)'],
> +                ['opengl4.6', 'OpenGL 4.6 support'],
> +                ['opengl4.6+vulkan', 'OpenGL 4.6 and Vulkan support'],
> +            ],
> +            bind: {
> +                value: '{virglFeatures}',
> +                disabled: '{!isVirgl}',
> +            },
> +        },
> +        {
> +            xtype: 'proxmoxKVComboBox',
> +            name: 'virgl-memory-window',
> +            deleteEmpty: false,
> +            fieldLabel: gettext('VirGL Host Memory Window'),
> +            comboItems: [
> +                ['__default__', Proxmox.Utils.defaultText + ' (2048 MiB)'],
> +                ['512', '512 MiB'],
> +                ['1024', '1024 MiB'],
> +                ['2048', '2048 MiB'],
> +                ['4096', '4096 MiB'],
> +                ['8192', '8192 MiB'],
> +            ],
> +            bind: {
> +                value: '{virglMemoryWindow}',
> +                disabled: '{virglIsDefault}',
> +            },
> +        },
>       ],
>   
>       advancedItems: [
> @@ -141,7 +212,11 @@ Ext.define('PVE.qemu.DisplayEdit', {
>       vmconfig: undefined,
>   
>       subject: gettext('Display'),
> -    width: 350,
> +    width: 450,
> +
> +    fieldDefaults: {
> +        labelWidth: 180,
> +    },
>   
>       items: [
>           {