Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AssetLib is not updating size and stuck #84471

Open
kosartofiq opened this issue Nov 5, 2023 · 18 comments
Open

AssetLib is not updating size and stuck #84471

kosartofiq opened this issue Nov 5, 2023 · 18 comments

Comments

@kosartofiq
Copy link

kosartofiq commented Nov 5, 2023

Godot version

v4.1.1.stable.official [bd6af8e]

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

@Chaosus
Copy link
Member

Chaosus commented Nov 5, 2023

I think It's fixed in #80555, that PR may be back ported to 4.1.x in order to fix this.

@AThousandShips
Copy link
Member

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

@kosartofiq
Copy link
Author

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

@GrammAcc
Copy link
Contributor

GrammAcc commented Nov 6, 2023

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!

@kosartofiq
Copy link
Author

kosartofiq commented Nov 6, 2023

@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

@GrammAcc
Copy link
Contributor

GrammAcc commented Nov 7, 2023

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 #84471 (comment) 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
Copy link
Contributor

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
Copy link
Contributor

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

@GrammAcc
Copy link
Contributor

@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. :)

@akien-mga
Copy link
Member

akien-mga commented Apr 15, 2024

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

@akien-mga akien-mga added this to the 4.3 milestone Apr 15, 2024
@kosartofiq
Copy link
Author

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

@akien-mga akien-mga reopened this May 1, 2024
@GrammAcc
Copy link
Contributor

GrammAcc commented May 1, 2024

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. :)

@Chaosus
Copy link
Member

Chaosus commented May 27, 2024

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

project_manager_bug

@KoBeWi KoBeWi removed this from the 4.3 milestone Jul 24, 2024
@romlok
Copy link
Contributor

romlok commented Oct 11, 2024

At least part of the issue is definitely not Windows-specific. I'm encountering an oversized and unshrinkable AssetLib with v4.3.stable.official [77dcf97] under Linux (Debian+KDE+Wayland).

System info Godot v4.3.stable - Debian GNU/Linux trixie/sid trixie - Wayland - Vulkan (Mobile) - dedicated AMD Radeon RX 7600 (RADV NAVI33) - AMD Ryzen 5 7600 6-Core Processor (12 Threads)

Funnily enough though, only the first page of the assetlib! 😖

2024-10-11.11-35-40.mp4

In case it matters my main monitor (where the windows initially open) is 125% scaling, but the one I captured on is 100%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants