Skip to content

Commit

Permalink
fix: Oscillation issue with pixel snapping + pixelToSnap default fa…
Browse files Browse the repository at this point in the history
…lse (excaliburjs#2348)

This PR fixes an issue that could cause graphical oscillations to occur when near a pixel boundary. To fix this we add a small epsilon to the pixel snap to nudge pixels over a boundary.

This PR also switches the default `snapToPixel` default back to `false`
  • Loading branch information
eonarheim authored Jul 2, 2022
1 parent 359a662 commit 5fa595d
Show file tree
Hide file tree
Showing 23 changed files with 262 additions and 43 deletions.
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

0 comments on commit 5fa595d

Please sign in to comment.