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

UI refresh() is unusually expensive #99

Closed
felixfaire opened this issue Oct 26, 2020 · 10 comments
Closed

UI refresh() is unusually expensive #99

felixfaire opened this issue Oct 26, 2020 · 10 comments

Comments

@felixfaire
Copy link

Currently performing pane.refresh() is taking upwards of 120ms to complete and triggers various layout shifts. Is there a way to update ui input values that have been changed in a script without causing this expensive reflow? For example only updating the components whos values have changed?

image

Also moving the colour picker also seems to be a very expensive operation and causes the animation loop to start juddering. This harms the user experience somewhat when working with realtime graphical applications.

Again thanks for the awesome framework, it's been incredibly useful so far. I wish I could help iron out these performance issues but I am a native / graphics developer and do not have much experience with browser processes etc.

(Ps. you should add a donate button to your site)

@felixfaire felixfaire changed the title UI refresh() is extremely expensive UI refresh() is unusually expensive Oct 26, 2020
@felixfaire
Copy link
Author

i've noticed in sv-palette.ts the colour palette drawing and updating could be optimised. I might be able to have a look at this at some point. Also my friend mentioned that calling the update on mousemove (vs synced to the redraw/frame rate) will do lots of extra work. However that optimisation might require a callback throttle at the higher level to the animation frame.

@felixfaire
Copy link
Author

I wasnt able to build the repo using 'npm install and npm start' (returned lots of script syntax errors with node 10.15.3 and npm 6.4.1). but if you want to dramatically speed up the sv-palette i would recommend uploading colours to a 64x64 canvas as an array of rgba bytes using ImageData instead of using ctx.fillRect for every pixel. Then scale the canvas to the size you want with css. This will avoid lots of expensive canvas rect antialiasing operations etc and may even give you a smoother gradient image due to the canvas filtering. See more here https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Pixel_manipulation_with_canvas

@felixfaire
Copy link
Author

felixfaire commented Oct 27, 2020

Build issues were a windows thing. This change to sv-palette.ts optimised the colour picker rendering. This could be even faster by rendering to a smaller canvas and scaling with css. I think this will help the refresh() performance too.

// import * as ColorConverter from '../../converter/color';
import {ClassName} from '../../misc/class-name';
import * as DisposingUtil from '../../misc/disposing-util';
import * as DomUtil from '../../misc/dom-util';
import {NumberUtil} from '../../misc/number-util';
import {PaneError} from '../../misc/pane-error';
import {Color} from '../../model/color';
import {InputValue} from '../../model/input-value';
import {View, ViewConfig} from '../view';

const className = ClassName('svp', 'input');

/* accepts parameters
* c = [h, s, v]
* where h s v are 0.0 - 1.0
*/
function HSVtoRGB(c: Number[]): Number[] {
	var r, g, b, i, f, p, q, t, h, s, v;
	h = c[0];
	s = c[1];
	v = c[2];
	i = Math.floor(h * 6);
	f = h * 6 - i;
	p = v * (1 - s);
	q = v * (1 - f * s);
	t = v * (1 - (1 - f) * s);
	switch (i % 6) {
			case 0: r = v, g = t, b = p; break;
			case 1: r = q, g = v, b = p; break;
			case 2: r = p, g = v, b = t; break;
			case 3: r = p, g = q, b = v; break;
			case 4: r = t, g = p, b = v; break;
			case 5: r = v, g = p, b = q; break;
	}
	return [r, g, b];
}

interface Config extends ViewConfig {
	value: InputValue<Color>;
}

/**
 * @hidden
 */
export class SvPaletteInputView extends View {
	public readonly value: InputValue<Color>;
	private canvasElem_: HTMLCanvasElement | null;
	private markerElem_: HTMLDivElement | null;

	constructor(document: Document, config: Config) {
		super(document, config);

		this.onValueChange_ = this.onValueChange_.bind(this);

		this.value = config.value;
		this.value.emitter.on('change', this.onValueChange_);

		this.element.classList.add(className());
		this.element.tabIndex = 0;

		const canvasElem = document.createElement('canvas');
		canvasElem.classList.add(className('c'));
		this.element.appendChild(canvasElem);
		this.canvasElem_ = canvasElem;

		const markerElem = document.createElement('div');
		markerElem.classList.add(className('m'));
		this.element.appendChild(markerElem);
		this.markerElem_ = markerElem;

		this.update();

		config.model.emitter.on('dispose', () => {
			this.canvasElem_ = DisposingUtil.disposeElement(this.canvasElem_);
			this.markerElem_ = DisposingUtil.disposeElement(this.markerElem_);
		});
	}

	get canvasElement(): HTMLCanvasElement {
		if (!this.canvasElem_) {
			throw PaneError.alreadyDisposed();
		}
		return this.canvasElem_;
	}

	public update(): void {
		if (!this.markerElem_) {
			throw PaneError.alreadyDisposed();
		}

		const ctx = DomUtil.getCanvasContext(this.canvasElement);
		if (!ctx) {
			return;
		}

		const c = this.value.rawValue;
		const hsvComps = c.getComponents('hsv');
		const width = this.canvasElement.width;
		const height = this.canvasElement.height;

		// This can be optimised further by using a smaller canvas
		let imageData = ctx.getImageData(0, 0, width, height);
		let data = imageData.data;
		const hue = hsvComps[0] / 360.0;

		for (let x = 0; x < width; ++x)
		{
			for (let y = 0; y < height; ++y)
			{
				const i = (x + width * y) * 4;
				const s = NumberUtil.map(x, 0, width - 1, 0, 1.0);
				const v = NumberUtil.map(y, 0, height - 1, 1.0, 0);
				const rgb = HSVtoRGB([hue, s, v]);
				data[i + 0] = rgb[0] * 255;
				data[i + 1] = rgb[1] * 255;
				data[i + 2] = rgb[2] * 255;
				data[i + 3] = 255;
			}
		}
			
		ctx.putImageData(imageData, 0, 0);

		const left = NumberUtil.map(hsvComps[1], 0, 100, 0, 100);
		this.markerElem_.style.left = `${left}%`;
		const top = NumberUtil.map(hsvComps[2], 0, 100, 100, 0);
		this.markerElem_.style.top = `${top}%`;
	}

	private onValueChange_(): void {
		this.update();
	}
}

@cocopon
Copy link
Owner

cocopon commented Oct 27, 2020

Thank you for using Tweakpane and addressing the issue.

  • Could you share your pane settings? I'd also like to use it for benchmark.
  • I'll try your suggestion later.
  • I think that I can also improve other palettes (alpha, hue) by replacing canvas with CSS gradient.

@felixfaire
Copy link
Author

Which settings do you mean? My tweakpane object is initialised like this:

  const ui = new Tweakpane({
    title: 'Parameters',
    expanded: true,
    container: document.getElementById('tweakpane-container'),
  });

and features:

  • 6 folders
  • 1 dropdown
  • 5 colours
  • 17 sliders
  • 5 separators

If that helps.

cocopon added a commit that referenced this issue Oct 28, 2020
@cocopon
Copy link
Owner

cocopon commented Oct 28, 2020

Applied your optimization to the branch optimize-palettes. Could you try it before merging?
https://github.com/cocopon/tweakpane/tree/optimize-palettes

@cocopon
Copy link
Owner

cocopon commented Oct 29, 2020

Pushed additional performance improvements.

@felixfaire
Copy link
Author

Amazing. The colour selector performance is greatly improved by the latest commits. I haven't tried the manual refresh situation yet but will do so if these changes make their way onto npm (to use with my other project).

@cocopon
Copy link
Owner

cocopon commented Oct 29, 2020

Just published the latest version 1.5.6 to npm.

@felixfaire
Copy link
Author

The latest version on npm is much faster with the colour palettes and the pane refresh. Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants