From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C7BD71FF0AE for ; Tue, 01 Sep 2026 13:35:27 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 17EAF214DC; Tue, 01 Sep 2026 13:35:27 +0200 (CEST) Message-ID: <0b95f3ba-a4d7-480a-90fa-c4d829d5a88f@proxmox.com> Date: Tue, 1 Sep 2026 13:35:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH pve-manager v4 2/3] ui: qemu: add VirGl feature selection in DisplayEdit To: Markus Frank , pve-devel@lists.proxmox.com References: <20260831094605.114396-1-m.frank@proxmox.com> <20260831094605.114396-3-m.frank@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20260831094605.114396-3-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788262521267 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.585 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: AJUPWZFSVMTWHPGL52PZD24QBXZ3AAS4 X-Message-ID-Hash: AJUPWZFSVMTWHPGL52PZD24QBXZ3AAS4 X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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: [ > {