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

SubViewportContainer does not scale Viewport properly with CanvasItems project scale mode. #77149

Open
EMBYRDEV opened this issue May 17, 2023 · 14 comments

Comments

@EMBYRDEV
Copy link
Contributor

Godot version

4.0.2.stable

System information

Windows 11, Vulkan, RTX2070 Super

Issue description

  • CanvasItems scale mode is required for nice UI scaling.
  • SubViewports being used for splitscreen gaming.

SubViewportContainer does not scale the underlying viewport appropriately to maintain quality in this mode.

Potentially related to #70791 and #76450.

image

Steps to reproduce

See reproduction project.

image

Minimal reproduction project

ViewportScalingRepro.zip

@EMBYRDEV EMBYRDEV changed the title SubViewportContainer does not scale properly with CanvasItems project scale mode. SubViewportContainer does not scale Viewport properly with CanvasItems project scale mode. May 17, 2023
@EMBYRDEV
Copy link
Contributor Author

EMBYRDEV commented May 17, 2023

Upon some investigation it appears that scaling that it can handle one axis of scaling but not both. Either vertical or horizonal scaling works properly but not combined.

@Calinou
Copy link
Member

Calinou commented May 23, 2023

This doesn't happen automatically by design, as changing viewport size behind the user's back may break expectations (especially when using viewport for off-screen rendering, such as procedural textures).

You need to resize the SubViewports by listening to the size_changed signal on the root Viewport, as done in the 3D in 2D demo. See godotengine/godot-demo-projects#891.

@Calinou Calinou closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2023
@EMBYRDEV
Copy link
Contributor Author

EMBYRDEV commented May 23, 2023

This doesn't happen automatically by design, as changing viewport size behind the user's back may break expectations (especially when using viewport for off-screen rendering, such as procedural textures).

You need to resize the SubViewports by listening to the size_changed signal on the root Viewport, as done in the 3D in 2D demo. See godotengine/godot-demo-projects#891.

That commit removes any reference to SubViewportContainer from the example.

Is this not the point of SubViewportContainer's stretch option? To keep the viewport at a 1:1 pixel ratio on screen.

I think one of us is missing something here. If you look at the way the size of the container actually changes in the example, it isnt as simple as just multiplying it by the scaling factor. The containers laying it out etc for splitscreen actually keep one of the dimensions in a weird way.

@Calinou
Copy link
Member

Calinou commented May 23, 2023

Is this not the point of SubViewportContainer's stretch option? To keep the viewport at a 1:1 pixel ratio on screen.

The Stretch option makes the SubViewportContainer stretch to cover the container's size. It does not resize the underlying Viewport.

@EMBYRDEV
Copy link
Contributor Author

Is this not the point of SubViewportContainer's stretch option? To keep the viewport at a 1:1 pixel ratio on screen.

The Stretch option makes the SubViewportContainer stretch to cover the container's size. It does not resize the underlying Viewport.

image
That is not what it states.

https://github.com/godotengine/godot/assets/20043270/048f0e2e-7152-4ca7-8671-78fe5913836f
Also see that the container does not report it's size in a way that is able to calculate how much I should manually scale the viewport by.

@Calinou Calinou reopened this May 23, 2023
@EMBYRDEV
Copy link
Contributor Author

It does what it says on the tin to be fair, the problem is that when canvas_items scale mode is set, the control's size seems to have absolutely no obvious relation to screen size any more so maintaining the correct pixel size on the display doesn't have an immediately obvious solution.

I think canvas_items scale mode is a little bit of a rabbit hole that links back to these #54030 #76450. Another example that could cause issues is if you design your games HUD at 720p and use the scaling mode but then want to do split screen. You'd want the canvas items in each viewport to scale down accordingly.

Perhaps the scaling mode should be set per-viewport instead of per-project?

@Calinou
Copy link
Member

Calinou commented May 23, 2023

Perhaps the scaling mode should be set per-viewport instead of per-project?

There was some talk about moving the content scale options from Window to Viewport, but this likely breaks compatibility.

@rcorre
Copy link
Contributor

rcorre commented May 24, 2023

That commit removes any reference to SubViewportContainer from the example.

I saw that demo as answering the common question "How do I scale the 3D resolution for performance", which no longer requires a SubViewport in the forward renderer, as you can set the 3D scaling directly on the main viewport.

I split out a separate compatibility demo to show the "legacy" way of scaling with a SubViewport (which should also apply to e.g. multiple SubViewports for split-screen), where I think I hit this issue: godotengine/godot-demo-projects#891 (comment)

@OhiraKyou
Copy link

Just ran into this while trying to add 3D collectible counter icons to a UI. I use a base resolution of 1024x768. When I maximize the window on my 2560x1440 monitor, the icons look extra blocky, with 2-pixel slope steps.

Ideally, the SubViewportContainer would just have a setting to enforce pixel-perfect rendering.

@OhiraKyou
Copy link

OhiraKyou commented Nov 16, 2023

For anyone seeking a solution, my current workaround is to replace the SubViewportContainer with a TextureRect whose texture is set to New ViewportTexture and assigned the associated SubViewport. Then, in script, I reproduce the size scale calculation and apply the result to the SubViewport as a multiplier for its initial size.

Setup

For TextureRect scaling, I use a parent MarginContainer with a custom_minimum_size equal to the SubViewport's initial size. The TextureRect 's container sizing (both horizontal and vertical) is set to "Fill". To prevent unintended additional TextureRect scaling, I set its expand mode to "Ignore Size" and stretch mode to "Keep Aspect Centered".

Also, I set the TextureRect's texture_filter to "Near" and texture_repeat to "Disabled". The filter setting prevents camera background bleeding, which is particularly important when using a transparent background.

Script

class_name ViewportSizer
extends Node


@export var target_viewport: SubViewport

var _base_render_size: Vector2 = Vector2()
var _window: Window


func _ready():
	_window = get_tree().root as Window
	_window.size_changed.connect(update_size)
	_base_render_size = target_viewport.size


## Calculates a scale for width and height, and applies the lower of the two.
##
## This assumes the following display settings:
## - display/window/stretch/mode: canvas_items
## - display/window/stretch/aspect: expand
func update_size():
	# Cast from Vector2i (integer) to Vector2 (float) for smooth results
	var current_viewport_size: Vector2 = _window.size
	var reference_viewport_size: Vector2 = _window.content_scale_size
	
	var viewport_scale: Vector2 = current_viewport_size / reference_viewport_size
	var size_scale: float = minf(viewport_scale.x, viewport_scale.y)
	
	# Round back to integer size to prevent truncation
	var scaled_size: Vector2i = (_base_render_size * size_scale).round()
	target_viewport.size = scaled_size

@newobj
Copy link
Contributor

newobj commented Jan 9, 2024

@OhiraKyou this solution does not work for me. any steps you may have forgot to detail?

also, does this solution break object picking for yo?

@OhiraKyou
Copy link

@newobj

any steps you may have forgot to detail?

  • Although the script's process update mode is left at its default value of Inherit, it is a distant child of a basic Node (to contain the UI) whose process mode is set to Always (to ensure that it updates while the game is paused).
  • The SubViewport render target update mode is set to Always.
  • The target_viewport variable exported from the ViewportSizer script needs to be assigned in the inspector (set to the SubViewport used to render the texture).
  • The SubViewport and its camera need to be configured to render the desired layers. I enabled own_world_3d on the SubViewport so that it doesn't render any external objects (such as other icons) to the texture. This makes them invisible in the viewport, but they're visible at runtime.
  • Comments in the ViewportSizer script include the project display settings, which this implementation relies on.

does this solution break object picking for yo?

Assuming you're referring to selecting objects by clicking on them from the viewport, I can still click on the TextureRect node. Any objects being rendered to or behind the texture can be selected in the hierarchy instead.

@BenLubar
Copy link

Here's my version of @OhiraKyou's script, this time as a replacement subclass for SubViewportContainer

# https://github.com/godotengine/godot/issues/77149
class_name SubViewportContainerBug77149
extends SubViewportContainer

@export var stretch_without_bug_77149 : bool

func _ready() -> void:
	resized.connect(_on_resized)
	_on_resized()

var _dont_recurse : bool

func _on_resized() -> void:
	assert(not stretch)
	if not stretch_without_bug_77149 or _dont_recurse:
		return

	var window := get_window()
	var viewport_size := Vector2(window.size)
	var reference_size := Vector2(window.content_scale_size)
	var viewport_scale := viewport_size / reference_size
	var size_scale := minf(viewport_scale.x, viewport_scale.y)

	var scaled_size := Vector2i((size * scale * size_scale).round()) / stretch_shrink
	if Vector2i(size.round()) == scaled_size:
		return

	_dont_recurse = true
	# we need to set the container's size first or it'll get resized by the viewport resize (ugh)
	scale = Vector2(1.0 / size_scale, 1.0 / size_scale)
	size = scaled_size

	for subviewport : SubViewport in find_children("*", "SubViewport", false, false):
		# avoid reallocating textures we don't need to reallocate
		if subviewport.size != scaled_size:
			subviewport.size = scaled_size

	_dont_recurse = false

@KaruroChori
Copy link

Sadly this problem is poisoning the entire hierarchy stack.
For example, opening new windows or confirmation dialogs will exhibit the same issue, as they are all derived from ViewportContainer.
image
As you can see in the example, text drawn as part for the root window and the decoration of the dialog are fine, but the inside is not.

Aside from patching the engine directly, is there any feasible way to override this problem?

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

7 participants