godot icon indicating copy to clipboard operation
godot copied to clipboard

AssetLib is not updating size and stuck

Open kosartofiq opened this issue 2 years ago • 16 comments

Godot version

v4.1.1.stable.official [bd6af8e0e]

System information

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Forward+) - integrated Intel(R) UHD Graphics 620 () - Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz (8 Threads)

Issue description

in Assetlib tab will be so big size until can't see write panel and buttons. Screenshot 2023-11-05 045923 Screenshot 2023-11-05 050845

Steps to reproduce

after open a project, click in asssitlib tab, for a short time will show tab in perfect size, but after one second will update itself, and hide a part of inspector(screenshot 1), also some of button for test scene and play to test scene also will be hidden, normally it resize all window to bigger than screen. but when clicking 2d, 3d and script , the window comes back to normal size(maximized) when restore it and drag right edge of the screen to show all contents of the window, (I have 2 screen) it needs wide of the 2 screen to show all the contents correctly, but when maximized the window again. half of the window hidden , then when click others tabs , will come back to correct windows(screenshot 2) I restarted many times. result is the same

Minimal reproduction project

KurdishCasino.zip

kosartofiq avatar Nov 05 '23 04:11 kosartofiq

I think It's fixed in https://github.com/godotengine/godot/pull/80555, that PR may be back ported to 4.1.x in order to fix this.

Chaosus avatar Nov 05 '23 10:11 Chaosus

It was, please try 4.1.3, 4.1.1 is no longer supported

AThousandShips avatar Nov 05 '23 10:11 AThousandShips

I updated to 4.1.3, sorry for that I didn't know there is update. It is one problem of this program, it doesn't notify user about new update. anyway. there is still some bug left. when open window for first time. it show correctly(not updating itself for bigger size like in v4.1.1). and change between tabs have no problems. until when restore window, then I drag right side to make wider windows (I have 2 screen), then maximize it, the problem show again. Screenshot 2023-11-05 133513

kosartofiq avatar Nov 05 '23 12:11 kosartofiq

Thanks for the CC!

I was unable to reproduce on my machine on 4.1.3-stable, but I'm on Linux with a bespoke WM.

The fix I provided recalculates the width of the asset lib contents based on the total width of the window, so that the asset lib won't cause the window to expand past the monitor size, but it does this when the asset lib plugin is instantiated, so I don't think it recalculates when resizing the window, so probably I should add an event handler that triggers the recalculation on resize events.

In any case, if the window is reporting that its size is the total width of both screens or just that it is wider than the single screen, the problem would still occur when maximizing.

@kosartofiq Are your two monitors both the same resolution, or is one larger than the other? If they are different sizes, does it maximize correctly on the larger monitor? Does it work if the editor is originally started on the smaller monitor?

I will look into this more after work. :)

Thanks!

GrammAcc avatar Nov 06 '23 14:11 GrammAcc

@GrammAcc , really I have 3 monitor, first laptop primary monitor, my laptop on a dock to provide other 2 monitor. I tested in both additional monitor. both of them same brand, same resolution, same size. I tested in all three monitor , results are same. normally it start in secondary monitor in my computer. this problem is appears only in Assetlib tab, when clicking others , it shows normally

kosartofiq avatar Nov 06 '23 17:11 kosartofiq

@GrammAcc , really I have 3 monitor, first laptop primary monitor, my laptop on a dock to provide other 2 monitor. I tested in both additional monitor. both of them same brand, same resolution, same size. I tested in all three monitor , results are same. normally it start in secondary monitor in my computer. this problem is appears only in Assetlib tab, when clicking others , it shows normally

Thank you for confirming!

It's odd that it is happening when both monitors are the same size. The content doesn't get resized after it is created, so I would think that the UI shouldn't overflow unless the engine starts on a screen that is larger than the screen it is moved to.

I tried setting up a Windows 10 VM to see if I could reproduce on Windows, but Godot won't start in the VM due to OpenGL version issues in virtualbox, and I couldn't get it to pick up the mesa dll. :/

I'm not sure what is causing the issue when maximizing after resizing on Windows, but regardless, I think the original solution I provided in #80555 is flawed. The overflow problem can still occur when shrinking a window as well:

screen-1_shot_2023-11-06_18:18:50

These seem to be edge cases, but it's an indication that the solution isn't adequate for an application that needs to support all different kinds of screen orientations and dynamic scaling and whatnot.

Recalculating the dynamic column width on window resize events might work, but that would probably have performance implications. I'll start working on an improved solution and report back in this thread once I have a PR ready. :)

GrammAcc avatar Nov 07 '23 00:11 GrammAcc

Just a heads up that I'm still working on this. I usually only have time for OSS on Saturdays, so it'll probably be another week or two before I have a PR ready.

GrammAcc avatar Nov 11 '23 21:11 GrammAcc

Progress report:

TL;DR: I think this is actually a Windows platform bug, and not a problem with the fix provided in #80555, but I can't confirm for sure because I don't have access to a windows machine to test on. The current fix does recalculate the columns on resize but it does not recalculate the acceptable length for asset titles, so I will submit a PR adding this in the next week or two.

I have been unable to reproduce the overflow issue when maximizing after resizing on Linux with X11 and Fluxbox, and after exploring the code for a while, I have not found any data paths that could lead to the issue as reported, so I think it is probably a bug in the way Windows is handling the resize events.

I first tried experimenting with recalculating columns on the viewport's size_changed signal, but in a facepalm moment, I realized after digging into the asset lib source file a bit further that it already calls the _update_asset_items_columns function on NOTIFICATION_RESIZED for the parent container, and after taking a closer look at the way the UI is updating when I resize the engine, it does actually update the number of columns when resizing the window on my machine, and it updates correctly when maximizing and un-maximizing as well. However, that event hook was in place before I added my fix, so it does not recalculate the truncation threshold for the asset titles. I will submit a PR that adds this extra recalculation to the NOTIFICATION_RESIZED hook to make sure the UI is consistent after resizes, but I don't think there is anything in the current solution that would cause the overflow as reported.

I also thought there might be a race condition, so I took a look at the code related to the resize events in control.cpp, and they are using thread guard macros, and I don't think it should make a difference anyway since the code to recalculate the columns should run in the next frame after the resize event.

I thought that another possibility for a race condition of sorts would be if the resize event is fired, and the column recalculation doesn't happen fast enough, then the container would automatically resize to fit its content on the next tick, but the columns would not have been recalculated yet, so the contents would be wider than the new size of the container, so it would basically snap back to its size before it was resized by the user, and then the column recalculation would run, which would keep the same number of columns as before the window resize since it uses the size of the parent container after the resize to calculate the new columns. I tested this by adding a sleep at the top of the _update_asset_items_columns function, and it still calculated the correct number of columns after the delay. I tried it with a one frame delay and a one second delay, and both still worked on my machine.

I can't think of anything else that could cause this issue, so I think this is very likely an issue with the WM on Windows handling the resize events differently than Linux.

As an aside, the overflow when shrinking the window I pointed out previously in this thread is also unrelated to this fix (mostly). I think it is caused by the way Godot's UI handles scaling with text nodes since font sizes are absolute. I have a few ideas for how this could be improved, but that's proposal territory and out of scope for this issue.

Here is a screenshot of the same UI overflow when shrinking the editor without the Asset Lib open:

screen-2_shot_2023-11-28_13:03:53

In conclusion, I will work on a PR to add the truncation recalculation to the resize event and to take care of the TODO comments in the original fix, but I don't think I can do anything about the issue as reported since I don't have access to a Windows machine to work on anything platform specific.

Please let me know if there is anything I can help with in further diagnosing this issue.

GrammAcc avatar Nov 28 '23 20:11 GrammAcc

Just an FYI, I think I spoke too soon. This might not be a Windows issue. I tried a few different things, and the columns are not properly recalculated when shrinking the window while using the compatibility renderer on Linux. It still works when maximizing/un-maximizing and when growing the window, but there is definitely a problem with the resize hook on the OpenGL backend on Linux. I will investigate this further and report my findings, but it might still be a bug in the renderer, and not in the asset lib. This however is something that I can work on since it's not specific to Windows. :)

GrammAcc avatar Nov 29 '23 15:11 GrammAcc

I'm very sorry for the wait. I have had very little time to work on Godot lately, even on the weekends. I've been mostly working in small bits over my lunch breaks, so not very productive. :(

I submitted a PR that recalculates the title truncation length on window resize. It fixes the issues I noticed with the compatibility renderer as well as the inconsistent column widths after resize.

I am unable to confirm if it fixes the issue as reported in https://github.com/godotengine/godot/issues/84471#issuecomment-1793725120 since I have not been able to reproduce that specific problem, but I think it will probably fix it since it forces recalculation of the width of the column contents, but if it's some kind of race condition or something in the Windows WM, then this might not help. Of course, in that case, it would definitely be a separate issue anyway, but I just wanted to clarify that the new PR might fix the issue as reported here. :)

GrammAcc avatar Dec 11 '23 23:12 GrammAcc

Okay, very strange. The fix I added was working on an older commit, but after rebasing over latest master, it's not recalculating properly every time I resize the editor. I have marked the PR as a draft so I can figure this out.

GrammAcc avatar Dec 12 '23 00:12 GrammAcc

Okay, I figured out what was wrong with the PR and got it fixed, so I have submitted it for review. :)

GrammAcc avatar Dec 12 '23 01:12 GrammAcc

@kosartofiq #88761 was merged recently, and I think it should fix this problem. That PR is included in latest Master and in the 4.2.2.rc3 release candidate. Are you able to test the latest build on your machine to confirm if this issue is resolved?

I don't have a Windows machine available to test this on. :)

GrammAcc avatar Apr 13 '24 00:04 GrammAcc

Assuming fixed by #88761, please let us know if it isn't the case.

akien-mga avatar Apr 15 '24 22:04 akien-mga

I updated to v4.2.2.stable.official [15073afe3], unfortunately the problem still exist.

kosartofiq avatar May 01 '24 02:05 kosartofiq

Hmm, I don't think there's much else I can do on this one. I was never able to reproduce the issue as described on my Linux box, so I had assumed that it was a Windows-specific issue related to the dynamic scaling in the Asset Lib.

The changes I made to fix this were pretty much the same as the changes in #88761, so if that didn't fix this issue, I don't know what else to do with it.

If a Windows user wants to take a shot at this one, feel free. I can provide context around the dynamic scaling in the Asset Lib if you have any questions, but I don't know how to diagnose this further. :)

GrammAcc avatar May 01 '24 12:05 GrammAcc

On Windows AssetLib for the project manager templates seems to be stuck too:

project_manager_bug

Chaosus avatar May 27 '24 17:05 Chaosus