From aad9ad2d6315e2fca35806d42a5e392be635b385 Mon Sep 17 00:00:00 2001 From: Erik Onarheim Date: Thu, 7 Mar 2024 22:44:03 -0600 Subject: [PATCH] fix: fit content resizing incorrectly (#2960) Fixed issue where unbounded containers would grow infinitely when using the following display modes: * `DisplayMode.FillContainer` * `DisplayMode.FitContainer` * `DisplayMode.FitContainerAndFill` * `DisplayMode.FitContainerAndZoom` This works by using CSS percent to fill the container instead of calculating the size which would cause n+1 resizing when setting the size. Viewport screen dimensions will now support percent based sizes --- CHANGELOG.md | 8 ++- src/engine/Director/Loader.ts | 28 ++++----- src/engine/Screen.ts | 106 +++++++++++++++++++++++++--------- src/spec/ScreenSpec.ts | 26 +++++++-- src/spec/util/TestUtils.ts | 1 - 5 files changed, 121 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9117a1f4f..0a55f61cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixed issue where logo was sometimes not loaded during `ex.Loader` +- Fixed issue where unbounded containers would grow infinitely when using the following display modes: + * `DisplayMode.FillContainer` + * `DisplayMode.FitContainer` + * `DisplayMode.FitContainerAndFill` + * `DisplayMode.FitContainerAndZoom` - Fixed issue where `ex.ParticleEmitter` z-index did not propagate to particles - Fixed incongruent behavior as small scales when setting `transform.scale = v` and `transform.scale.setTo(x, y)` - Fixed `ex.coroutine` TypeScript type to include yielding `undefined` @@ -34,7 +40,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Changed -- +- Simplified `ex.Loader` viewport/resolution internal configuration diff --git a/src/engine/Director/Loader.ts b/src/engine/Director/Loader.ts index 0fc09a5b9..838435c0c 100644 --- a/src/engine/Director/Loader.ts +++ b/src/engine/Director/Loader.ts @@ -11,6 +11,7 @@ import { DefaultLoader, DefaultLoaderOptions } from './DefaultLoader'; import { Engine } from '../Engine'; import { Screen } from '../Screen'; import { Logger } from '../Util/Log'; +import { Future } from '../Util/Future'; export interface LoaderOptions extends DefaultLoaderOptions { /** @@ -133,9 +134,11 @@ export class Loader extends DefaultLoader { public backgroundColor: string = '#176BAA'; protected _imageElement: HTMLImageElement; + protected _imageLoaded: Future = new Future(); protected get _image() { if (!this._imageElement) { this._imageElement = new Image(); + this._imageElement.onload = () => this._imageLoaded.resolve(); this._imageElement.src = this.logo; } @@ -218,6 +221,10 @@ export class Loader extends DefaultLoader { this.screen = engine.screen; this.canvas.width = this.engine.canvas.width; this.canvas.height = this.engine.canvas.height; + this.screen.events.on('resize', () => { + this.canvas.width = this.engine.canvas.width; + this.canvas.height = this.engine.canvas.height; + }); } /** @@ -312,18 +319,9 @@ export class Loader extends DefaultLoader { private _configuredPixelRatio: number | null = null; public override async onBeforeLoad(): Promise { - this._configuredPixelRatio = this.screen.pixelRatioOverride; - // Push the current user entered resolution/viewport - this.screen.pushResolutionAndViewport(); - // Configure resolution for loader, it expects resolution === viewport - this.screen.resolution = this.screen.viewport; - this.screen.pixelRatioOverride = 1; - this.screen.applyResolutionAndViewport(); - - this.canvas.width = this.engine.canvas.width; - this.canvas.height = this.engine.canvas.height; - - await this._image?.decode(); // decode logo if it exists + const image = this._image; + await this._imageLoaded.promise; + await image?.decode(); // decode logo if it exists } // eslint-disable-next-line require-await @@ -336,8 +334,10 @@ export class Loader extends DefaultLoader { private _positionPlayButton() { if (this.engine) { - const screenHeight = this.engine.screen.viewport.height; - const screenWidth = this.engine.screen.viewport.width; + const { + width: screenWidth, + height: screenHeight + } = this.engine.canvas.getBoundingClientRect(); if (this._playButtonRootElement) { const left = this.engine.canvas.offsetLeft; const top = this.engine.canvas.offsetTop; diff --git a/src/engine/Screen.ts b/src/engine/Screen.ts index 7454fedac..c6a8d8aec 100644 --- a/src/engine/Screen.ts +++ b/src/engine/Screen.ts @@ -140,9 +140,13 @@ export class Resolution { } } +export type ScreenUnit = 'pixel' | 'percent'; + export interface ScreenDimension { width: number; height: number; + widthUnit?: ScreenUnit; + heightUnit?: ScreenUnit; } export interface ScreenOptions { @@ -436,9 +440,15 @@ export class Screen { } public set resolution(resolution: ScreenDimension) { + if (resolution.heightUnit === 'percent' || resolution.widthUnit === 'percent') { + throw Error('Screen resolution only supports pixels not percentage sizes'); + } this._resolution = resolution; } + /** + * Returns screen dimensions in pixels or percentage + */ public get viewport(): ScreenDimension { if (this._viewport) { return this._viewport; @@ -517,8 +527,12 @@ export class Screen { this._canvas.style.imageRendering = 'crisp-edges'; } } - this._canvas.style.width = this.viewport.width + 'px'; - this._canvas.style.height = this.viewport.height + 'px'; + + const widthUnit = this.viewport.widthUnit === 'percent' ? '%' : 'px'; + const heightUnit = this.viewport.heightUnit === 'percent' ? '%' : 'px'; + + this._canvas.style.width = this.viewport.width + widthUnit; + this._canvas.style.height = this.viewport.height + heightUnit; // After messing with the canvas width/height the graphics context is invalidated and needs to have some properties reset this.graphicsContext.updateViewport(this.resolution); @@ -836,6 +850,10 @@ export class Screen { }; this._contentArea = BoundingBox.fromDimension(this.resolution.width, this.resolution.height, Vector.Zero); this._unsafeArea = BoundingBox.fromDimension(this.resolution.width, this.resolution.height, Vector.Zero); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } private _computeFitScreenAndFill() { @@ -844,21 +862,34 @@ export class Screen { const vw = window.innerWidth; const vh = window.innerHeight; this._computeFitAndFill(vw, vh); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } private _computeFitContainerAndFill() { - document.body.style.margin = '0px'; - document.body.style.overflow = 'hidden'; - const parent = this.canvas.parentElement; - const vw = parent.clientWidth; - const vh = parent.clientHeight; - this._computeFitAndFill(vw, vh); + this.canvas.style.width = '100%'; + this.canvas.style.height = '100%'; + + this._computeFitAndFill( + this.canvas.offsetWidth, + this.canvas.offsetHeight, { + width: 100, + widthUnit: 'percent', + height: 100, + heightUnit: 'percent' + }); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } - private _computeFitAndFill(vw: number, vh: number) { - this.viewport = { + private _computeFitAndFill(vw: number, vh: number, viewport?: ScreenDimension) { + this.viewport = viewport ?? { width: vw, height: vh }; @@ -912,20 +943,25 @@ export class Screen { const vh = window.innerHeight; this._computeFitAndZoom(vw, vh); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } private _computeFitContainerAndZoom() { - document.body.style.margin = '0px'; - document.body.style.overflow = 'hidden'; - this.canvas.style.position = 'absolute'; + this.canvas.style.width = '100%'; + this.canvas.style.height = '100%'; + this.canvas.style.position = 'relative'; const parent = this.canvas.parentElement; - parent.style.position = 'relative'; parent.style.overflow = 'hidden'; - - const vw = parent.clientWidth; - const vh = parent.clientHeight; + const { offsetWidth: vw, offsetHeight: vh } = this.canvas; this._computeFitAndZoom(vw, vh); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } private _computeFitAndZoom(vw: number, vh: number) { @@ -990,20 +1026,32 @@ export class Screen { const aspect = this.aspectRatio; let adjustedWidth = 0; let adjustedHeight = 0; + let widthUnit: ScreenUnit = 'pixel'; + let heightUnit: ScreenUnit = 'pixel'; const parent = this.canvas.parentElement; if (parent.clientWidth / aspect < parent.clientHeight) { - adjustedWidth = parent.clientWidth; - adjustedHeight = parent.clientWidth / aspect; + this.canvas.style.width = '100%'; + adjustedWidth = 100; + widthUnit = 'percent'; + adjustedHeight = this.canvas.offsetWidth / aspect; } else { - adjustedWidth = parent.clientHeight * aspect; - adjustedHeight = parent.clientHeight; + this.canvas.style.height = '100%'; + adjustedHeight = 100; + heightUnit = 'percent'; + adjustedWidth = this.canvas.offsetHeight * aspect; } this.viewport = { width: adjustedWidth, - height: adjustedHeight + widthUnit, + height: adjustedHeight, + heightUnit }; this._contentArea = BoundingBox.fromDimension(this.resolution.width, this.resolution.height, Vector.Zero); + this.events.emit('resize', { + resolution: this.resolution, + viewport: this.viewport + } satisfies ScreenResizeEvent); } private _applyDisplayMode() { @@ -1026,12 +1074,18 @@ export class Screen { */ private _setResolutionAndViewportByDisplayMode(parent: HTMLElement | Window) { if (this.displayMode === DisplayMode.FillContainer) { + this.canvas.style.width = '100%'; + this.canvas.style.height = '100%'; + this.viewport = { + width: 100, + widthUnit: 'percent', + height: 100, + heightUnit: 'percent' + }; this.resolution = { - width: ( parent).clientWidth, - height: ( parent).clientHeight + width: this.canvas.offsetWidth, + height: this.canvas.offsetHeight }; - - this.viewport = this.resolution; } if (this.displayMode === DisplayMode.FillScreen) { diff --git a/src/spec/ScreenSpec.ts b/src/spec/ScreenSpec.ts index bebacad16..e0cb9405e 100644 --- a/src/spec/ScreenSpec.ts +++ b/src/spec/ScreenSpec.ts @@ -164,8 +164,12 @@ describe('A Screen', () => { expect(sut.resolution.height).toBe(800); expect(sut.contentArea.width).toBe(800); expect(sut.contentArea.height).toBe(600); - expect(sut.viewport.width).toBe(1300); - expect(sut.viewport.height).toBe(1300); + expect(sut.viewport.width).toBe(100); + expect(sut.viewport.widthUnit).toBe('percent'); + expect(sut.viewport.height).toBe(100); + expect(sut.viewport.heightUnit).toBe('percent'); + expect(sut.canvas.offsetWidth).toBe(1300); + expect(sut.canvas.offsetHeight).toBe(1300); }); @@ -192,8 +196,12 @@ describe('A Screen', () => { expect(sut.resolution.height).toBe(600); expect(sut.contentArea.width).toBe(800); expect(sut.contentArea.height).toBe(600); - expect(sut.viewport.width).toBe(1300); - expect(sut.viewport.height).toBe(800); + expect(sut.viewport.width).toBe(100); + expect(sut.viewport.widthUnit).toBe('percent'); + expect(sut.viewport.height).toBe(100); + expect(sut.viewport.heightUnit).toBe('percent'); + expect(sut.canvas.offsetWidth).toBe(1300); + expect(sut.canvas.offsetHeight).toBe(800); }); @@ -317,7 +325,10 @@ describe('A Screen', () => { expect(sut.resolution.width).toBe(800); expect(sut.resolution.height).toBe(600); expect(sut.viewport.width).toBe(800 * sut.aspectRatio); - expect(sut.viewport.height).toBe(800); + expect(sut.viewport.height).toBe(100); + expect(sut.viewport.heightUnit).toBe('percent'); + expect(sut.canvas.offsetHeight).toBe(800); + expect(sut.canvas.offsetWidth).toBe(Math.ceil(800 * sut.aspectRatio)); }); it('will adjust to width', () => { @@ -340,8 +351,11 @@ describe('A Screen', () => { expect(sut.parent).toBe(parentEl); expect(sut.resolution.width).toBe(800); expect(sut.resolution.height).toBe(600); - expect(sut.viewport.width).toBe(1000); + expect(sut.viewport.width).toBe(100); + expect(sut.viewport.widthUnit).toBe('percent'); expect(sut.viewport.height).toBe(1000 / sut.aspectRatio); + expect(sut.canvas.offsetHeight).toBe(1000 / sut.aspectRatio); + expect(sut.canvas.offsetWidth).toBe(1000); }); }); diff --git a/src/spec/util/TestUtils.ts b/src/spec/util/TestUtils.ts index ad54fd2ef..17e1ee89a 100644 --- a/src/spec/util/TestUtils.ts +++ b/src/spec/util/TestUtils.ts @@ -49,7 +49,6 @@ export namespace TestUtils { } const clock = engine.clock as ex.TestClock; const start = engine.start(loader); - // If loader if (loader) { await loader.areResourcesLoaded(); clock.step(200);