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

fix: Oscillation issue with pixel snapping + pixelToSnap default false #2348

Merged
merged 17 commits into from
Jul 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## Breaking Changes

- `ex.Engine.snapToPixel` now defaults to `false`, it was unexpected to have pixel snapping on by default it has now been switched.
- The `ex.Physics.useRealisticPhysics()` physics solver has been updated to fix a bug in bounciness to be more physically accurate, this does change how physics behaves. Setting `ex.Body.bounciness = 0` will simulate the old behavior.
- `ex.TransformComponent.posChanged$` has been removed, it incurs a steep performance cost
- `ex.EventDispatcher` meta events 'subscribe' and 'unsubscribe' were unused and undocumented and have been removed
Expand Down Expand Up @@ -129,6 +130,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
```

### Fixed

- Fixed issue with `ex.Engine.snapToPixel` where positions very close to pixel boundary created jarring 1 pixel oscillations.
- Fixed bug where a deferred `goToScene` would preserve the incorrect scene so `engine.add(someActor)` would place actors in the wrong scene after transitioning to another.
- Fixed usability issue and log warning if the `ex.ImageSource` is not loaded and a draw was attempted.
- Fixed bug in `ex.Physics.useRealisticPhysics()` solver where `ex.Body.bounciness` was not being respected in the simulation
Expand Down Expand Up @@ -159,6 +162,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- `ex.Engine.snapToPixel` now defaults to `false`
- Most places where `ex.Matrix` was used have been switched to `ex.AffineMatrix`
- Most places where `ex.TransformComponent` was used have been switched to `ex.Transform`

Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
version: '{build}'
image: Visual Studio 2019
image: Visual Studio 2022
# Fix line endings on Windows
init:
- git config --global core.autocrlf true
Expand Down
7 changes: 6 additions & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const process = require('process');
const path = require('path');
const webpack = require('webpack');
process.env.CHROMIUM_BIN = require('puppeteer').executablePath();
process.env.CHROME_BIN = require('puppeteer').executablePath();

const isAppveyor = process.env.APPVEYOR_BUILD_NUMBER ? true : false;
const karmaJasmineSeedReporter = function(baseReporterDecorator) {
Expand Down Expand Up @@ -136,9 +137,13 @@ module.exports = (config) => {
},
browsers: ['ChromiumHeadless_with_audio'],
browserDisconnectTolerance : 1,
browserDisconnectTimeout: 10000,
browserDisconnectTimeout: 60000, // appveyor is slow :(
browserNoActivityTimeout: 60000, // appveyor is slow :(
customLaunchers: {
ChromeHeadless_with_audio: {
base: 'ChromeHeadless',
flags: ['--autoplay-policy=no-user-gesture-required', '--mute-audio', '--disable-gpu', '--no-sandbox']
},
ChromiumHeadless_with_audio: {
base: 'ChromiumHeadless',
flags: ['--autoplay-policy=no-user-gesture-required', '--mute-audio', '--disable-gpu', '--no-sandbox']
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@
],
"scripts": {
"all": "npm run lint && npm run core && npm run build:esm && npm run test && npm run legacy-visual",
"all:ci": "npm run lint && npm run core && npm run build:esm && npm run test:ci && npm run legacy-visual",
"clean": "rimraf .tsbuildinfo ./build/dist ./build ./src/spec/*.js ./src/spec/*.map",
"linux:ci": "npm run clean && npm run all && npm run coveralls && npm run apidocs",
"appveyor": "npm run all && npm run nuget 0.0.1",
"linux:ci": "npm run clean && npm run all:ci && npm run coveralls && npm run apidocs",
"appveyor": "npm run all:ci && npm run nuget 0.0.1",
"browserstack": "karma start karma.conf.browsers.js",
"nuget": ".\\src\\tools\\NuGet.exe pack Excalibur.nuspec -OutputDirectory .\\build\\dist -version",
"start": "npm run core:watch",
Expand All @@ -52,6 +53,7 @@
"sandbox": "serve ./sandbox -l 3001 -n",
"legacy-visual": "npm run sandbox:copy && npm run sandbox:build",
"test": "karma start",
"test:ci": "karma start --browsers=ChromeHeadless_with_audio",
"test:watch": "karma start --auto-watch --single-run=false",
"coveralls": "echo 'Temporarily Remove Coveralls Due To Outage'",
"apidocs": "node scripts/apidocs.js",
Expand Down
2 changes: 1 addition & 1 deletion sandbox/src/game.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var game = new ex.Engine({
pointerScope: ex.Input.PointerScope.Canvas,
displayMode: ex.DisplayMode.FitScreenAndFill,
antialiasing: false,
snapToPixel: true,
snapToPixel: false,
maxFps: 60,
configurePerformanceCanvas2DFallback: {
allow: true,
Expand Down
2 changes: 1 addition & 1 deletion src/engine/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export interface EngineOptions {
canvasElement?: HTMLCanvasElement;

/**
* Optionally snap drawings to nearest pixel
* Optionally snap graphics to nearest pixel, default is false
*/
snapToPixel?: boolean;

Expand Down
51 changes: 34 additions & 17 deletions src/engine/Graphics/Context/ExcaliburGraphicsContext2DCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { ScreenDimension } from '../../Screen';
import { PostProcessor } from '../PostProcessor/PostProcessor';
import { AffineMatrix } from '../../Math/affine-matrix';

const pixelSnapEpsilon = 0.0001;

class ExcaliburGraphicsContext2DCanvasDebug implements DebugDraw {
private _debugText = new DebugText();
constructor(private _ex: ExcaliburGraphicsContext2DCanvas) {}
Expand All @@ -29,10 +31,10 @@ class ExcaliburGraphicsContext2DCanvasDebug implements DebugDraw {
this._ex.__ctx.save();
this._ex.__ctx.strokeStyle = 'red';
this._ex.__ctx.strokeRect(
this._ex.snapToPixel ? ~~x : x,
this._ex.snapToPixel ? ~~y : y,
this._ex.snapToPixel ? ~~width : width,
this._ex.snapToPixel ? ~~height : height
this._ex.snapToPixel ? ~~(x + pixelSnapEpsilon) : x,
this._ex.snapToPixel ? ~~(y + pixelSnapEpsilon) : y,
this._ex.snapToPixel ? ~~(width + pixelSnapEpsilon) : width,
this._ex.snapToPixel ? ~~(height + pixelSnapEpsilon) : height
);
this._ex.__ctx.restore();
}
Expand All @@ -41,8 +43,14 @@ class ExcaliburGraphicsContext2DCanvasDebug implements DebugDraw {
this._ex.__ctx.save();
this._ex.__ctx.beginPath();
this._ex.__ctx.strokeStyle = lineOptions.color.toString();
this._ex.__ctx.moveTo(this._ex.snapToPixel ? ~~start.x : start.x, this._ex.snapToPixel ? ~~start.y : start.y);
this._ex.__ctx.lineTo(this._ex.snapToPixel ? ~~end.x : end.x, this._ex.snapToPixel ? ~~end.y : end.y);
this._ex.__ctx.moveTo(
this._ex.snapToPixel ? ~~(start.x + pixelSnapEpsilon) : start.x,
this._ex.snapToPixel ? ~~(start.y + pixelSnapEpsilon) : start.y
);
this._ex.__ctx.lineTo(
this._ex.snapToPixel ? ~~(end.x + pixelSnapEpsilon) : end.x,
this._ex.snapToPixel ? ~~(end.y + pixelSnapEpsilon) : end.y
);
this._ex.__ctx.lineWidth = 2;
this._ex.__ctx.stroke();
this._ex.__ctx.closePath();
Expand All @@ -54,8 +62,8 @@ class ExcaliburGraphicsContext2DCanvasDebug implements DebugDraw {
this._ex.__ctx.beginPath();
this._ex.__ctx.fillStyle = pointOptions.color.toString();
this._ex.__ctx.arc(
this._ex.snapToPixel ? ~~point.x : point.x,
this._ex.snapToPixel ? ~~point.y : point.y,
this._ex.snapToPixel ? ~~(point.x + pixelSnapEpsilon) : point.x,
this._ex.snapToPixel ? ~~(point.y + pixelSnapEpsilon) : point.y,
pointOptions.size,
0,
Math.PI * 2
Expand Down Expand Up @@ -114,7 +122,7 @@ export class ExcaliburGraphicsContext2DCanvas implements ExcaliburGraphicsContex
this._state.current.tint = color;
}

public snapToPixel: boolean = true;
public snapToPixel: boolean = false;

public get smoothing(): boolean {
return this.__ctx.imageSmoothingEnabled;
Expand Down Expand Up @@ -200,8 +208,14 @@ export class ExcaliburGraphicsContext2DCanvas implements ExcaliburGraphicsContex
this.__ctx.save();
this.__ctx.beginPath();
this.__ctx.strokeStyle = color.toString();
this.__ctx.moveTo(this.snapToPixel ? ~~start.x : start.x, this.snapToPixel ? ~~start.y : start.y);
this.__ctx.lineTo(this.snapToPixel ? ~~end.x : end.x, this.snapToPixel ? ~~end.y : end.y);
this.__ctx.moveTo(
this.snapToPixel ? ~~ (start.x + pixelSnapEpsilon) : start.x,
this.snapToPixel ? ~~(start.y + pixelSnapEpsilon) : start.y
);
this.__ctx.lineTo(
this.snapToPixel ? ~~ (end.x + pixelSnapEpsilon) : end.x,
this.snapToPixel ? ~~(end.y + pixelSnapEpsilon) : end.y
);
this.__ctx.lineWidth = thickness;
this.__ctx.stroke();
this.__ctx.closePath();
Expand All @@ -212,10 +226,10 @@ export class ExcaliburGraphicsContext2DCanvas implements ExcaliburGraphicsContex
this.__ctx.save();
this.__ctx.fillStyle = color.toString();
this.__ctx.fillRect(
this.snapToPixel ? ~~pos.x : pos.x,
this.snapToPixel ? ~~pos.y : pos.y,
this.snapToPixel ? ~~width : width,
this.snapToPixel ? ~~height : height
this.snapToPixel ? ~~(pos.x + pixelSnapEpsilon) : pos.x,
this.snapToPixel ? ~~(pos.y + pixelSnapEpsilon) : pos.y,
this.snapToPixel ? ~~(width + pixelSnapEpsilon) : width,
this.snapToPixel ? ~~(height + pixelSnapEpsilon) : height
);
this.__ctx.restore();
}
Expand All @@ -230,7 +244,10 @@ export class ExcaliburGraphicsContext2DCanvas implements ExcaliburGraphicsContex
this.__ctx.lineWidth = thickness;
}
this.__ctx.fillStyle = color.toString();
this.__ctx.arc(this.snapToPixel ? ~~pos.x : pos.x, this.snapToPixel ? ~~pos.y : pos.y, radius, 0, Math.PI * 2);
this.__ctx.arc(
this.snapToPixel ? ~~(pos.x + pixelSnapEpsilon) : pos.x,
this.snapToPixel ? ~~(pos.y + pixelSnapEpsilon) : pos.y, radius, 0, Math.PI * 2
);
this.__ctx.fill();
if (stroke) {
this.__ctx.stroke();
Expand Down Expand Up @@ -261,7 +278,7 @@ export class ExcaliburGraphicsContext2DCanvas implements ExcaliburGraphicsContex
* @param y
*/
translate(x: number, y: number): void {
this.__ctx.translate(this.snapToPixel ? ~~x : x, this.snapToPixel ? ~~y : y);
this.__ctx.translate(this.snapToPixel ? ~~(x + pixelSnapEpsilon) : x, this.snapToPixel ? ~~(y + pixelSnapEpsilon) : y);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/engine/Graphics/Context/ExcaliburGraphicsContextWebGL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import { Pool } from '../../Util/Pool';
import { DrawCall } from './draw-call';
import { AffineMatrix } from '../../Math/affine-matrix';

export const pixelSnapEpsilon = 0.0001;

class ExcaliburGraphicsContextWebGLDebug implements DebugDraw {
private _debugText = new DebugText();
constructor(private _webglCtx: ExcaliburGraphicsContextWebGL) {}
Expand Down Expand Up @@ -118,7 +120,7 @@ export class ExcaliburGraphicsContextWebGL implements ExcaliburGraphicsContext {
private _state = new StateStack();
private _ortho!: Matrix;

public snapToPixel: boolean = true;
public snapToPixel: boolean = false;

public smoothing: boolean = false;

Expand Down Expand Up @@ -398,7 +400,7 @@ export class ExcaliburGraphicsContextWebGL implements ExcaliburGraphicsContext {
}

public translate(x: number, y: number): void {
this._transform.translate(this.snapToPixel ? ~~x : x, this.snapToPixel ? ~~y : y);
this._transform.translate(this.snapToPixel ? ~~(x + pixelSnapEpsilon) : x, this.snapToPixel ? ~~(y + pixelSnapEpsilon) : y);
}

public rotate(angle: number): void {
Expand Down
17 changes: 16 additions & 1 deletion src/engine/Graphics/Context/circle-renderer/circle-renderer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Color } from '../../../Color';
import { vec, Vector } from '../../../Math/vector';
import { GraphicsDiagnostics } from '../../GraphicsDiagnostics';
import { ExcaliburGraphicsContextWebGL } from '../ExcaliburGraphicsContextWebGL';
import { ExcaliburGraphicsContextWebGL, pixelSnapEpsilon } from '../ExcaliburGraphicsContextWebGL';
import { QuadIndexBuffer } from '../quad-index-buffer';
import { RendererPlugin } from '../renderer';
import { Shader } from '../shader';
Expand Down Expand Up @@ -77,12 +77,27 @@ export class CircleRenderer implements RendererPlugin {
// transform based on current context
const transform = this._context.getTransform();
const opacity = this._context.opacity;
const snapToPixel = this._context.snapToPixel;

const topLeft = transform.multiply(pos.add(vec(-radius, -radius)));
const topRight = transform.multiply(pos.add(vec(radius, -radius)));
const bottomRight = transform.multiply(pos.add(vec(radius, radius)));
const bottomLeft = transform.multiply(pos.add(vec(-radius, radius)));

if (snapToPixel) {
topLeft.x = ~~(topLeft.x + pixelSnapEpsilon);
topLeft.y = ~~(topLeft.y + pixelSnapEpsilon);

topRight.x = ~~(topRight.x + pixelSnapEpsilon);
topRight.y = ~~(topRight.y + pixelSnapEpsilon);

bottomLeft.x = ~~(bottomLeft.x + pixelSnapEpsilon);
bottomLeft.y = ~~(bottomLeft.y + pixelSnapEpsilon);

bottomRight.x = ~~(bottomRight.x + pixelSnapEpsilon);
bottomRight.y = ~~(bottomRight.y + pixelSnapEpsilon);
}

// TODO UV could be static vertex buffer
const uvx0 = 0;
const uvy0 = 0;
Expand Down
18 changes: 9 additions & 9 deletions src/engine/Graphics/Context/image-renderer/image-renderer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { vec } from '../../../Math/vector';
import { GraphicsDiagnostics } from '../../GraphicsDiagnostics';
import { HTMLImageSource } from '../ExcaliburGraphicsContext';
import { ExcaliburGraphicsContextWebGL } from '../ExcaliburGraphicsContextWebGL';
import { ExcaliburGraphicsContextWebGL, pixelSnapEpsilon } from '../ExcaliburGraphicsContextWebGL';
import { QuadIndexBuffer } from '../quad-index-buffer';
import { RendererPlugin } from '../renderer';
import { Shader } from '../shader';
Expand Down Expand Up @@ -175,17 +175,17 @@ export class ImageRenderer implements RendererPlugin {
bottomRight = transform.multiply(bottomRight);

if (snapToPixel) {
topLeft.x = ~~topLeft.x;
topLeft.y = ~~topLeft.y;
topLeft.x = ~~(topLeft.x + pixelSnapEpsilon);
topLeft.y = ~~(topLeft.y + pixelSnapEpsilon);

topRight.x = ~~topRight.x;
topRight.y = ~~topRight.y;
topRight.x = ~~(topRight.x + pixelSnapEpsilon);
topRight.y = ~~(topRight.y + pixelSnapEpsilon);

bottomLeft.x = ~~bottomLeft.x;
bottomLeft.y = ~~bottomLeft.y;
bottomLeft.x = ~~(bottomLeft.x + pixelSnapEpsilon);
bottomLeft.y = ~~(bottomLeft.y + pixelSnapEpsilon);

bottomRight.x = ~~bottomRight.x;
bottomRight.y = ~~bottomRight.y;
bottomRight.x = ~~(bottomRight.x + pixelSnapEpsilon);
bottomRight.y = ~~(bottomRight.y + pixelSnapEpsilon);
}

const tint = this._context.tint;
Expand Down
8 changes: 7 additions & 1 deletion src/engine/Graphics/Context/point-renderer/point-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import pointVertexSource from './point-vertex.glsl';
import pointFragmentSource from './point-fragment.glsl';
import { Vector } from '../../../Math/vector';
import { Color } from '../../../Color';
import { ExcaliburGraphicsContextWebGL } from '../ExcaliburGraphicsContextWebGL';
import { ExcaliburGraphicsContextWebGL, pixelSnapEpsilon } from '../ExcaliburGraphicsContextWebGL';
import { RendererPlugin } from '../renderer';
import { Shader } from '../shader';
import { VertexBuffer } from '../vertex-buffer';
Expand Down Expand Up @@ -56,9 +56,15 @@ export class PointRenderer implements RendererPlugin {

const transform = this._context.getTransform();
const opacity = this._context.opacity;
const snapToPixel = this._context.snapToPixel;

const finalPoint = transform.multiply(point);

if (snapToPixel) {
finalPoint.x = ~~(finalPoint.x + pixelSnapEpsilon);
finalPoint.y = ~~(finalPoint.y + pixelSnapEpsilon);
}

const vertexBuffer = this._buffer.bufferData;
vertexBuffer[this._vertexIndex++] = finalPoint.x;
vertexBuffer[this._vertexIndex++] = finalPoint.y;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Color } from '../../../Color';
import { vec, Vector } from '../../../Math/vector';
import { GraphicsDiagnostics } from '../../GraphicsDiagnostics';
import { ExcaliburGraphicsContextWebGL } from '../ExcaliburGraphicsContextWebGL';
import { ExcaliburGraphicsContextWebGL, pixelSnapEpsilon } from '../ExcaliburGraphicsContextWebGL';
import { QuadIndexBuffer } from '../quad-index-buffer';
import { RendererPlugin } from '../renderer';
import { Shader } from '../shader';
Expand Down Expand Up @@ -87,6 +87,7 @@ export class RectangleRenderer implements RendererPlugin {
// transform based on current context
const transform = this._context.getTransform();
const opacity = this._context.opacity;
const snapToPixel = this._context.snapToPixel;

const dir = end.sub(start);
const length = dir.size;
Expand All @@ -105,6 +106,20 @@ export class RectangleRenderer implements RendererPlugin {
const endTop = transform.multiply(normal.scale(halfThick).add(end));
const endBottom = transform.multiply(normal.scale(-halfThick).add(end));

if (snapToPixel) {
startTop.x = ~~(startTop.x + pixelSnapEpsilon);
startTop.y = ~~(startTop.y + pixelSnapEpsilon);

endTop.x = ~~(endTop.x + pixelSnapEpsilon);
endTop.y = ~~(endTop.y + pixelSnapEpsilon);

startBottom.x = ~~(startBottom.x + pixelSnapEpsilon);
startBottom.y = ~~(startBottom.y + pixelSnapEpsilon);

endBottom.x = ~~(endBottom.x + pixelSnapEpsilon);
endBottom.y = ~~(endBottom.y + pixelSnapEpsilon);
}

// TODO uv could be static vertex buffer
const uvx0 = 0;
const uvy0 = 0;
Expand Down Expand Up @@ -206,12 +221,27 @@ export class RectangleRenderer implements RendererPlugin {
// transform based on current context
const transform = this._context.getTransform();
const opacity = this._context.opacity;
const snapToPixel = this._context.snapToPixel;

const topLeft = transform.multiply(pos.add(vec(0, 0)));
const topRight = transform.multiply(pos.add(vec(width, 0)));
const bottomRight = transform.multiply(pos.add(vec(width, height)));
const bottomLeft = transform.multiply(pos.add(vec(0, height)));

if (snapToPixel) {
topLeft.x = ~~(topLeft.x + pixelSnapEpsilon);
topLeft.y = ~~(topLeft.y + pixelSnapEpsilon);

topRight.x = ~~(topRight.x + pixelSnapEpsilon);
topRight.y = ~~(topRight.y + pixelSnapEpsilon);

bottomLeft.x = ~~(bottomLeft.x + pixelSnapEpsilon);
bottomLeft.y = ~~(bottomLeft.y + pixelSnapEpsilon);

bottomRight.x = ~~(bottomRight.x + pixelSnapEpsilon);
bottomRight.y = ~~(bottomRight.y + pixelSnapEpsilon);
}

// TODO uv could be static vertex buffer
const uvx0 = 0;
const uvy0 = 0;
Expand Down
Loading