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

Improve blurred content of embedded windows #97745

Merged

Conversation

scgm0
Copy link
Contributor

@scgm0 scgm0 commented Oct 2, 2024

Helps: #54030
Helps: #95387

I honestly don't know if I'm doing it the right way, but the issue seems to be forgotten and after a flurry of attempts I'm getting good results.
before:
图片
after:
图片
example:
embedded-window.zip

@hsvfan-jan
Copy link

In the screenshot, the circles of Test2 and Test3 seem to have become an octagon (the newly introduced white outline probably making it a full circle I'd assume)

@scgm0
Copy link
Contributor Author

scgm0 commented Oct 2, 2024

In the screenshot, the circles of Test2 and Test3 seem to have become an octagon (the newly introduced white outline probably making it a full circle I'd assume)

It seems to be a problem caused by viewport_set_global_canvas_transform. I don’t know where the specific problem is.

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from c328cec to 7646cfa Compare October 2, 2024 20:59
@KoBeWi KoBeWi added this to the 4.4 milestone Oct 2, 2024
@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from 7646cfa to 4e10173 Compare October 3, 2024 14:02
@scgm0
Copy link
Contributor Author

scgm0 commented Oct 3, 2024

In the screenshot, the circles of Test2 and Test3 seem to have become an octagon (the newly introduced white outline probably making it a full circle I'd assume)

When I manually assign the radio_unchecked icon in the theme (radio_unchecked.svg was copied from godot), there is no white border around the icon, so maybe the problem is related to the theme cache?
图片
图片

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch 6 times, most recently from 335d9b1 to 02bb81e Compare October 5, 2024 08:23
@scgm0
Copy link
Contributor Author

scgm0 commented Oct 5, 2024

The problem of viewport_set_global_canvas_transform should not be within the scope of this PR. What needs to be discussed now is whether the solution of this PR is qualified and whether there are any side effects.

@Summersay415
Copy link
Contributor

Summersay415 commented Oct 6, 2024

Tested locally, works fine. It doesn't remove all blurriness, but decreases it a lot.
Before:
изображение
After:
изображение

@scgm0
Copy link
Contributor Author

scgm0 commented Oct 6, 2024

Tested locally, works fine. It doesn't remove all blurriness, but decreases it a lot. Before: изображение After: изображение

Have you enabled the font msdf?

@Summersay415
Copy link
Contributor

No

@scgm0
Copy link
Contributor Author

scgm0 commented Oct 6, 2024

No

Give it a try?

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from 02bb81e to d852dcf Compare October 6, 2024 09:59
@eldidou
Copy link
Contributor

eldidou commented Oct 9, 2024

Hello, It seems that this fix solves #95387, with no side-effect seen so far.

@scgm0
Copy link
Contributor Author

scgm0 commented Oct 9, 2024

Hello, It seems that this fix solves #95387, with no side-effect seen so far.

That's great if there's no problem.

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from d852dcf to cf64625 Compare October 21, 2024 22:19
scene/main/viewport.cpp Outdated Show resolved Hide resolved
@clayjohn clayjohn requested a review from timothyqiu November 9, 2024 04:03
Copy link
Member

@timothyqiu timothyqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and it greatly reduced blurriness 👍

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from 6a0094f to 4ec0c32 Compare November 10, 2024 22:51
@akien-mga akien-mga requested a review from bruvzg November 11, 2024 20:09
@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch 2 times, most recently from ad66cd1 to eb7b86d Compare November 16, 2024 15:52
@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from eb7b86d to 55e1414 Compare November 24, 2024 11:35
@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch 2 times, most recently from 0bf368d to 8e7bfff Compare December 6, 2024 13:27
@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch 6 times, most recently from cbc9bef to c7f9bbf Compare December 19, 2024 06:47
@scgm0
Copy link
Contributor Author

scgm0 commented Dec 20, 2024

Just wondering if there is something wrong with having this pr so it hasn't been merged yet?

@scgm0 scgm0 force-pushed the Fix-blurred-content-of-embedded-windows branch from c7f9bbf to 0bc2c26 Compare December 20, 2024 14:44
@akien-mga
Copy link
Member

Just wondering if there is something wrong with having this pr so it hasn't been merged yet?

There's nothing wrong a priori, but it needs reviews from maintainers familiar with the Viewport and Window code (such as @bruvzg or @Sauermann).

@bruvzg
Copy link
Member

bruvzg commented Dec 20, 2024

Code seems fine, will fully check it tomorrow.

@akien-mga akien-mga merged commit 88e6997 into godotengine:master Dec 21, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

10 participants