X Tutup
Skip to content

Replace global oversampling with overridable per-viewport oversampling.#104872

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
bruvzg:per_vp_os
Apr 15, 2025
Merged

Replace global oversampling with overridable per-viewport oversampling.#104872
Repiteo merged 1 commit intogodotengine:masterfrom
bruvzg:per_vp_os

Conversation

@bruvzg
Copy link
Member

@bruvzg bruvzg commented Apr 1, 2025 •
edited by KoBeWi
Loading

  • Allows independent per-viewport oversampling (required for automatic DPI scaling). Previously was global value and only truly supported for the main window.
  • Allows overriding oversampling factor manually.
  • Oversampling can reuse existing font data if possible (e.g, if you have 16 and 32 size fonts already in use, and oversampling of 2x, oversampled 16 font will reuse 32 font data).
  • Change of oversampling factor won't reload all fonts.
  • Oversampling use 16.6 fractions units for font size (have more scaling steps, and won't reload font if scaling change is too small).
  • Adds performance monitors for TextServer fonts and oversampling.

@bruvzg bruvzg added this to the 4.5 milestone Apr 1, 2025
@bruvzg bruvzg marked this pull request as ready for review April 1, 2025 08:49
@bruvzg bruvzg requested review from a team as code owners April 1, 2025 08:49
@bruvzg bruvzg force-pushed the per_vp_os branch 2 times, most recently from 0c5c4c0 to 5981e7e Compare April 4, 2025 04:54
@bruvzg bruvzg mentioned this pull request Apr 6, 2025
Draft
Calinou
Calinou reviewed Apr 9, 2025
View reviewed changes
Copy link
Member

@Calinou Calinou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master a8598cd), I noticed an issue with embolden using a radius that's way too large in the editor. You can reproduce this by using push_error()/push_warning() in a @tool script's _ready() method, then using Scene > Reload Saved Scene:

image

Also, this PR doesn't fix #86563:

Godot 3 Godot 4 - master Godot 4 - This PR
Godot 3 Godot 4 Master Godot 4 PR

At default window size, kerning has changed slightly on the text at the bottom-right corner, but the rest is identical:

Godot 3 Godot 4 - master Godot 4 - This PR
Godot 3 Godot 4 Master Godot 4 PR

@bruvzg
Copy link
Member Author

bruvzg commented Apr 9, 2025 •
edited
Loading

Fixed embolden and issue with subpixel positioning activated at wrong size (was causing minor kerning shifts).

I'll check #86563 (it looks perfectly sharp to me, will check on a different monitor and OS later).

clayjohn
clayjohn reviewed Apr 9, 2025
View reviewed changes
KoBeWi
KoBeWi reviewed Apr 13, 2025
View reviewed changes
@KoBeWi
Copy link
Member

KoBeWi commented Apr 13, 2025

I added a Label in a SubViewport in a SubViewportContainer and setting oversampling doesn't have visible effect:

godot.windows.editor.dev.x86_64_XZs9kGcNJo.mp4

Also oversampling_override of 0 is somewhat buggy. When I set its value and then reset to 0, the override does not reset.

@bruvzg
Copy link
Member Author

bruvzg commented Apr 13, 2025

When I set its value and then reset to 0, the override does not reset.

Fixes.

I added a Label in a SubViewport in a SubViewportContainer and setting oversampling doesn't have visible effect

Default oversampling is based on "Size 2D override/stretch" values (which is set to content scale for windows), and will take effect only is it's enabled.

Screen.Recording.2025-04-13.at.21.33.48.mov

@KoBeWi
Copy link
Member

KoBeWi commented Apr 13, 2025

Shouldn't it have effect when I set the override though?

@bruvzg
Copy link
Member Author

bruvzg commented Apr 13, 2025

Shouldn't it have effect when I set the override though?

Yes, if override is set it always applies.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 13, 2025

Then the oversampling property is misleading. It's difficult to tell when it has effect and when not. It should be clarified in the description, and the property can be disabled when irrelevant.

@bruvzg
Copy link
Member Author

bruvzg commented Apr 13, 2025

Also, there's some max limit for absolute font size (4096 for single glyph dimensions), so if you set large font size and excessively large oversampling it might break (underlaying font size is multiple of these two).

KoBeWi
KoBeWi reviewed Apr 13, 2025
View reviewed changes
KoBeWi
KoBeWi reviewed Apr 13, 2025
View reviewed changes
@bruvzg
Copy link
Member Author

bruvzg commented Apr 13, 2025

Added notes about new arguments and Viewport.oversampling usage and removed monitors (will move to #105083).

KoBeWi
KoBeWi reviewed Apr 13, 2025
View reviewed changes
KoBeWi
KoBeWi reviewed Apr 13, 2025
View reviewed changes
KoBeWi
KoBeWi approved these changes Apr 13, 2025
View reviewed changes
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

It's a complex change, but overall looks fine.

@bruvzg bruvzg mentioned this pull request Apr 14, 2025
Merged
AThousandShips
AThousandShips reviewed Apr 14, 2025
View reviewed changes
@Repiteo Repiteo merged commit 9b6d927 into godotengine:master Apr 15, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Apr 15, 2025

Thanks!

@bruvzg bruvzg mentioned this pull request Apr 15, 2025
Merged
@Cykyrios Cykyrios mentioned this pull request Apr 15, 2025
Closed
@bruvzg bruvzg mentioned this pull request Apr 15, 2025
Merged
@timoschwarzer timoschwarzer mentioned this pull request Apr 15, 2025
Merged
@arkology arkology mentioned this pull request Apr 15, 2025
Closed
@bruvzg bruvzg mentioned this pull request Apr 15, 2025
Merged
@darksylinc
Copy link
Contributor

darksylinc commented Apr 15, 2025 •
edited
Loading

This PR completely broke Godot on Xubuntu 24.04 LTS (Godot v4.5.dev (d1ad5da2e) - Ubuntu 24.04.2 LTS 24.04 on X11 - X11 display driver, Multi-window, 2 monitors - Vulkan (Mobile) - dedicated AMD Radeon RX 6800 XT (RADV NAVI21) - AMD Ryzen 9 5900X 12-Core Processor (24 threads)):

image

That's all I get when opening the Editor.

Edit: I see it's already reported.

@youngminz youngminz mentioned this pull request Apr 28, 2025
Closed
@precup precup mentioned this pull request May 28, 2025
Closed
@zenorbi zenorbi mentioned this pull request Aug 6, 2025
Merged
@Calinou Calinou mentioned this pull request Aug 29, 2025
Merged
@youngminz youngminz mentioned this pull request Oct 7, 2025
Closed
@akien-mga akien-mga changed the title Replace global oversampling with overrideable per-viewport oversampling. Replace global oversampling with overridable per-viewport oversampling. Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Calinou Calinou Calinou left review comments

@clayjohn clayjohn clayjohn left review comments

@AThousandShips AThousandShips AThousandShips left review comments

@KoBeWi KoBeWi KoBeWi approved these changes

Assignees

No one assigned

Labels

enhancement topic:gui

Projects

None yet

Milestone

4.5

Development

Successfully merging this pull request may close these issues.

7 participants

@bruvzg @KoBeWi @Repiteo @darksylinc @Calinou @clayjohn @AThousandShips
X Tutup