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

Auto-Resize #584

Merged
merged 8 commits into from
Jul 31, 2023
Merged

Auto-Resize #584

merged 8 commits into from
Jul 31, 2023

Conversation

hobnob
Copy link
Member

@hobnob hobnob commented Jul 27, 2023

This PR closes #469 by removing debounce completely and adding automatic resizing throughout Indigo. This is another breaking change I'm afraid, as the GameConfig has now been given a new property called resizePolicy, which determines the way Indigo reacts to a resize. The new default behaviour is for Indigo to always resize depending on how big the container is. Whether this is sensible or not remains to be seen, but it felt sensible at the time 🙂

The resize policy can be changed in the GameConfig using either: withResizePolicy(resizePolicy), noResize, autoResize, or autoResizePreserveAspect.

This is all implemented using the Resize Observer, on the parent container. This is the recomended way of observing size changes, and will fire whenever the containing element changes size and waits for resource to be free before it does so. Indigo will fire an ApplicationResize event to let the app know it's being resized (for developer information really), and will then resize the canvas according to the policy, either not taking any action (NoResize), resizing to the entire width and height of the container (AutoResize) or resizing while maintaining the aspect ratio of the canvas element (AutoResizePreserveAspect).

It's worth noting that any CSS that overrides the width and height attributes of the canvas will interfere with the resizing policy and may produce unexpected results.

Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

Right. All of this looks really good except for two things:

  1. Have we lost the delay on resize?

Maybe you're saying we don't need it anymore? But the point of the debounce code was to not resize on every little twitch of the dimensions, because you end up with ugly graphical tearing or squash n' stretch rendering. It also means the WebGL context is being adjusted a lot which is bad news for performance. We've lost the debounce code (yay!) but not replaced it with an alternative, it seems?

  1. Do we really need the ApplicationResize event?

The real difference between ApplicationResize and ViewportResize is that the former reports any change in the size of the container whenever that happens, and ViewportResize reports any change in the size of the useable rendering area after the rendering context has been reinitialised.

I expect only ViewportResize is interesting to the majority users. The only use I can think of for ApplicationResize, is manually re-implementing the debounce function as mentioned in point (1).

@@ -2,32 +2,8 @@ package indigoplugin.templates

object SupportScriptTemplate {

def template(autoSize: Boolean): String =
def template(): String =
s"""
Copy link
Member

Choose a reason for hiding this comment

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

I am happy to see this go! ❤️

def autoResize: GameConfig =
withResizePolicy(ResizePolicy.Resize)
def autoResizePreserveAspect: GameConfig =
withResizePolicy(ResizePolicy.ResizePreserveAspect)
Copy link
Member

Choose a reason for hiding this comment

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

💪

@hobnob
Copy link
Member Author

hobnob commented Jul 29, 2023

1. Have we lost the delay on resize?

Yes.. but also no. The delay now is down to resource. If JS has enough resource to deal with a resize event then it will, rather than stressing the browser. If it doesn't then it won't and it will wait until it does. This means we're no longer waiting a set time to decide we've waited long enough, the browser is deciding based on the machine capabilities.

2. Do we really need the `ApplicationResize` event?

Possibly not. If you don't think it's useful, I'm happy to remove it :)

davesmith00000
davesmith00000 previously approved these changes Jul 30, 2023
Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

Excellent work @hobnob - delighted to see the debounce script deprecated! I've tested it locally and it's a big improvement. 🎉

@davesmith00000 davesmith00000 merged commit 76a5477 into main Jul 31, 2023
2 checks passed
@davesmith00000 davesmith00000 deleted the features/autoResize branch July 31, 2023 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fullscreen rendering bug (debounce)
2 participants