Disabling video auto scaling requires workaround

NemoStein commented 2 years ago

OS platform / Browser

Windows 10, Chrome 105

melonJS version

v13.0.0

Bug description

Video always auto scale, even if settings.scale != 'auto'

When not specified, setting.scaleMethod defaults to fit, forcing settings.autoScale to true ((anything || true) == true) https://github.com/melonjs/melonJS/blob/313af5c746d25d8d0d33986bf631f3e18843e857/src/video/video.js#L237-L238

Setting setting.scale to a value bigger than 1 and changing setting.scaleMethod to an empty string (or any string that doesn't math the if above) still doesn't work because, for some reason, the canvas element comes with it's width/height set to width * scale/height * scale instead of just width/height and without style="width:...;height:...;", so everything drawn in the canvas is smaller than it should.

Steps to reproduce the bug

video.init(64, 64, {
  parent: 'screen',
  scale: 8
})

Workaround

The only way to disable auto scaling and make the scaling work as intended is to set setting.scale to 1 and then call video.scale() with the desired scale size

video.init(64, 64, {
  parent: 'screen',
  scale: 1, // Must be 1 to force canvas size to the correct width/height (64x64 in this case)
  scaleMethod: '' // Any invalid string value works
})

video.scale(8, 8) // Scale canvas to the intended size

obiot commented 2 years ago

are you working on the lowrez game jam ? 😄 I was planning to as well but having a hard time finding the time to start anything .... :P

anyway will check this one, but if I may, if you want to limit by how much the canvas is scaled I would rather use css to limit the canvas size and use the automatic scaling, this will prevent the canvas to be oversized on mobile for example or any device where space is more limited. (this said with a 8x scaling on a 64x64 resolution this should be safe....)

obiot commented 2 years ago

this one is fixed, not the most common scaling option used, but nevertheless it should be working, good catch and thank you !

Note: I noticed you cloned the repo, if "urgent" are you able to build it yourself, or can you wait a couple of days so that at least the NineSliceSprite is fixed too ?

NemoStein commented 2 years ago

Almost there, but something still wrong!

The Renderer class is creating a canvas with already zoomed values https://github.com/melonjs/melonJS/blob/709d217e35e3bb6cce0d1debbac14a4b4ff793b4/src/video/renderer.js#L86

This makes the canvas have the width/height multiplied by zoom once and then the canvas style width/height multiplied again.

video.init(64, 64, { parent: stageElement, scale: 8 })

<canvas width="512" height="512" style="width: 4096px; height: 4096px; image-rendering: pixelated;"></canvas>

The following change works for me, but I don't know if (and assume that) it breaks something elsewhere.

createCanvas(this.settings.width, this.settings.height)

are you working on the lowrez game jam ?

Oh no, you got me! *fakes death with hand in chest while falling to the ground in the most dramatic and unrealistic death pose ever*

obiot commented 2 years ago

Oh no, you got me! fakes death with hand in chest while falling to the ground in the most dramatic and unrealistic death pose ever

:):):):)

obiot commented 2 years ago

The following change works for me, but I don't know if (and assume that) it breaks something elsewhere.
createCanvas(this.settings.width, this.settings.height)

I'm confused because this is what is actually in the code right now : https://github.com/melonjs/melonJS/blob/709d217e35e3bb6cce0d1debbac14a4b4ff793b4/src/video/renderer.js#L86

did you mean something else ?

(re-opening this one then until it's properly fixed)

NemoStein commented 2 years ago

You are completely right!

Indeed, the git version is right, but not the published v13.0.0...
I simply copied the line number from my local copy inside node_modules and didn't check the repo...

The change happened 21 days ago, right here: https://github.com/melonjs/melonJS/commit/424480473ea5f1b3b9b4b5257ed0f48781927c0c#diff-0b7e13b3c409d5164be22c6c856ecf54f7e380c249b58418645251705544da78L86

After building from source I can confirm that this fixes the issue!

obiot commented 2 years ago

yes, we had some refactoring going out around the renderer(s) to align both the canvas and WebGL one.

But cool then, I'll close back this one :)

melonjs / melonJS