From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id BC6B81FF15C
for <inbox@lore.proxmox.com>; Wed, 26 Mar 2025 11:42:55 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 161C8343EA;
Wed, 26 Mar 2025 11:42:49 +0100 (CET)
Message-ID: <c9d67e4c-ddc2-4093-b971-dbea051bdbab@proxmox.com>
Date: Wed, 26 Mar 2025 11:42:44 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250318090900.725706-1-d.csapak@proxmox.com>
<c8235584-6863-4021-b0dc-b36dd9bcc52c@proxmox.com>
<37855440-f4af-45ce-a7ac-781bb0151ed1@proxmox.com>
<2732d922-8d1c-4483-9a01-43bfcb00b7f5@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <2732d922-8d1c-4483-9a01-43bfcb00b7f5@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.022 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [mozilla.org]
Subject: Re: [pve-devel] [PATCH] fix #6223: fit terminal after 'OK' message
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On 3/26/25 11:19, Thomas Lamprecht wrote:
> Am 26.03.25 um 09:11 schrieb Dominik Csapak:
>> On 3/25/25 19:44, Thomas Lamprecht wrote:
>>> Am 18.03.25 um 10:09 schrieb Dominik Csapak:
>>>> instead of simply waiting 250ms after we send the credentials, wait
>>>> until after the server responded with 'OK' to fit the terminal size.
>>>> Still keep the timeout to not do that in the onmessage handler itself,
>>>> but rather at a later point in time.
>>>
>>> potential dumb question, but what's the reason to keep the 250ms in
>>> that case?
>>
>> not a dumb question at all, and you're right: the exact value of 250ms is strictly not necessary.
>> I wanted to keep the code in a timeout, so it does not block the 'onmessage' handler,
>> but rather that it runs later when the browser has idle cycles.
>>
>> We could of course reduce the timeout, but in my experience, sometimes browsers behave unexpected
>> when it's too short (e.g., it then runs immediatly after the JS code, without a render cycle in
>> between, which is what i want to avoid here)
>
> any reference for that, especially w.r.t. unexpected behavior, as that
> rather just sounds like expected behavior as nothing in the setTimeout
> function is designed for being able to order with (re)paint events.
>
>
Not really. It's maybe also just unexpected to me. I happened to stumble
over similar behavior a few times in the past in extjs, where e.g. a
setTimeout callback was triggered before the browser would update the
dom from the extjs changes immediately before.
>>
>> In practice, omitting the timeout here would naturally work too here, but possibly delay the content
>> of the terminal in favor of resizing.
> I mean, lowering to something between 20 an 50 ms would be IMO a better
> heuristic with less latency, as if the tab is active repaints will happen
> at display rate if anything changes and assuming 50 Hz (20 ms period) as
> lower bound seems OK, if we want to play it safe then 50 ms would be OK
> to.
>
> Alternatively, if what you actually want is to wait one paint we could also
> use requestAnimationFrame [0] for that, e.g. something like the following
>
> // wait at least one or more frames
> function callbackAfterRepaint(callbackFn) {
> let firstTimestamp;
> let wrapperFn;
> wrapperFn = timestamp => {
> if (firstTimestamp === undefined) {
> firstTimestamp = timestamp;
> requestAnimationFrame(wrapperFn);
> } else if (firstTimestamp === timestamp) {
> requestAnimationFrame(wrapperFn);
> } else {
> callbackFn();
> }
> };
> requestAnimationFrame(wrapperFn);
> }
>
>
> I think comparing the timestamp isn't even a requirement, as nesting this
> will lead to two calls for separate frames, but that would need checking
> more closely. And the time comparison was based on the following docs:
>
>> When multiple callbacks queued by requestAnimationFrame() begin to fire
>> in a single frame, each receives the same timestamp even though time has
>> passed during the computation of every previous callback's workload.
>
> But again, probably not required as requisting an animation frame from
> inside the callback allways gives the next one already anyway.
>
> [0]: https://developer.mozilla.org/en-US/docs/Web/API/Window/requestAnimationFrame
I'd interpret this in the same way, so a simple
requestAnimationFrame(() => requestAnimationFrame(() => { .. my callback code .. }));
should also work?
>
> Using requestAnimationFrame is not a must, I just stumbled upon this again
> and wanted to try it, and it feels a bit nicer than waiting some arbitrary
> amount if letting pass at least one paint cycle is the goal; I can also
> apply as is and just lower the wait period to 50 ms, just tell me what you
> think after reading this.
Sure, I can do that, but after thinking a bit, it probably does not really matter either way
and I'm over complicating things. I'm fine with a reduced timeout or omitting it altogether.
I'll send a v2 if that's less work for you
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel