From 9add650b75f920f03b97252aa222a4f2bc8090c7 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Sun, 1 Jul 2018 16:01:42 +0100 Subject: [PATCH] Refactorings (#65) * Refactorings * Responding to feedback --- src/components/app/index.tsx | 113 +++++++++++++++++++------------- src/components/output/index.tsx | 8 ++- src/lib/util.ts | 4 +- 3 files changed, 75 insertions(+), 50 deletions(-) diff --git a/src/components/app/index.tsx b/src/components/app/index.tsx index c80c087f..3306b51e 100644 --- a/src/components/app/index.tsx +++ b/src/components/app/index.tsx @@ -35,6 +35,28 @@ interface State { error?: string; } +async function compressImage( + source: SourceImage, + encodeData: EncoderState, +): Promise { + // Special case for identity + if (encodeData.type === identity.type) return source.bmp; + + const compressedData = await (() => { + switch (encodeData.type) { + case mozJPEG.type: return mozJPEG.encode(source.data, encodeData.options); + default: throw Error(`Unexpected encoder name`); + } + })(); + + const blob = new Blob([compressedData], { + type: encoderMap[encodeData.type].mimeType, + }); + + const bitmap = await createImageBitmap(blob); + return bitmap; +} + export default class App extends Component { state: State = { loading: false, @@ -43,15 +65,15 @@ export default class App extends Component { encoderState: { type: identity.type, options: identity.defaultOptions }, loadingCounter: 0, loadedCounter: 0, - loading: false + loading: false, }, { encoderState: { type: mozJPEG.type, options: mozJPEG.defaultOptions }, loadingCounter: 0, loadedCounter: 0, - loading: false - } - ] + loading: false, + }, + ], }; constructor() { @@ -69,18 +91,18 @@ export default class App extends Component { onEncoderChange(index: 0 | 1, type: EncoderType, options?: EncoderOptions): void { const images = this.state.images.slice() as [EncodedImage, EncodedImage]; - const image = images[index]; + const oldImage = images[index]; // Some type cheating here. // encoderMap[type].defaultOptions is always safe. // options should always be correct for the type, but TypeScript isn't smart enough. const encoderState: EncoderState = { type, - options: options ? options : encoderMap[type].defaultOptions + options: options ? options : encoderMap[type].defaultOptions, } as EncoderState; images[index] = { - ...image, + ...oldImage, encoderState, }; @@ -95,7 +117,9 @@ export default class App extends Component { const { source, images } = this.state; for (const [i, image] of images.entries()) { - if (source !== prevState.source || image !== prevState.images[i]) { + // The image only needs updated if the encoder settings have changed, or the source has + // changed. + if (source !== prevState.source || image.encoderState !== prevState.images[i].encoderState) { this.updateImage(i); } } @@ -122,6 +146,7 @@ export default class App extends Component { const bmp = await createImageBitmap(file); // compute the corresponding ImageData once since it only changes when the file changes: const data = await bitmapToImageData(bmp); + this.setState({ source: { data, bmp, file }, error: undefined, @@ -133,57 +158,53 @@ export default class App extends Component { } async updateImage(index: number): Promise { - const { source, images } = this.state; + const { source } = this.state; if (!source) return; - let image = images[index]; + let images = this.state.images.slice() as [EncodedImage, EncodedImage]; - // Each time we trigger an async encode, the ID changes. - image.loadingCounter = image.loadingCounter + 1; - const loadingCounter = image.loadingCounter; + // Each time we trigger an async encode, the counter changes. + const loadingCounter = images[index].loadingCounter + 1; - image.loading = true; - this.setState({ }); + const image = images[index] = { + ...images[index], + loadingCounter, + loading: true, + }; - const result = await this.updateCompressedImage(source, image.encoderState); + this.setState({ images }); - image = this.state.images[index]; - // If a later encode has landed before this one, return. - if (loadingCounter < image.loadedCounter) return; - image.bmp = result; - image.loading = image.loadingCounter !== loadingCounter; - image.loadedCounter = loadingCounter; - this.setState({ }); - } - - async updateCompressedImage(source: SourceImage, encodeData: EncoderState): Promise { - // Special case for identity - if (encodeData.type === identity.type) return source.bmp; + let result; try { - const compressedData = await (() => { - switch (encodeData.type) { - case mozJPEG.type: return mozJPEG.encode(source.data, encodeData.options); - default: throw Error(`Unexpected encoder name`); - } - })(); - - const blob = new Blob([compressedData], { - type: encoderMap[encodeData.type].mimeType - }); - - const bitmap = await createImageBitmap(blob); - this.setState({ error: '' }); - return bitmap; + result = await compressImage(source, image.encoderState); } catch (err) { - this.setState({ error: `Encoding error (type=${encodeData.type}): ${err}` }); + this.setState({ error: `Encoding error (type=${image.encoderState.type}): ${err}` }); throw err; } + + const latestImage = this.state.images[index]; + // If a later encode has landed before this one, return. + if (loadingCounter < latestImage.loadedCounter) { + this.setState({ error: '' }); + return; + } + + images = this.state.images.slice() as [EncodedImage, EncodedImage]; + + images[index] = { + ...images[index], + bmp: result, + loading: image.loadingCounter !== loadingCounter, + loadedCounter: loadingCounter, + }; + + this.setState({ images, error: '' }); } - render({ }: Props, { loading, error, images, source }: State) { + render({ }: Props, { loading, error, images }: State) { const [leftImageBmp, rightImageBmp] = images.map(i => i.bmp); - loading = loading || images.some(image => image.loading); + const anyLoading = loading || images.some(image => image.loading); return ( @@ -209,7 +230,7 @@ export default class App extends Component { onOptionsChange={this.onOptionsChange.bind(this, index)} /> ))} - {loading && Loading...} + {anyLoading && Loading...} {error && Error: {error}} diff --git a/src/components/output/index.tsx b/src/components/output/index.tsx index 2d83ae87..6321a90e 100644 --- a/src/components/output/index.tsx +++ b/src/components/output/index.tsx @@ -8,7 +8,7 @@ import { twoUpHandle } from './custom-els/TwoUp/styles.css'; type Props = { leftImg: ImageBitmap, - rightImg: ImageBitmap + rightImg: ImageBitmap, }; type State = {}; @@ -39,13 +39,17 @@ export default class Output extends Component { } } + shouldComponentUpdate(nextProps: Props) { + return this.props.leftImg !== nextProps.leftImg || this.props.rightImg !== nextProps.rightImg; + } + @bind onPinchZoomLeftChange(event: Event) { if (!this.pinchZoomRight || !this.pinchZoomLeft) throw Error('Missing pinch-zoom element'); this.pinchZoomRight.setTransform({ scale: this.pinchZoomLeft.scale, x: this.pinchZoomLeft.x, - y: this.pinchZoomLeft.y + y: this.pinchZoomLeft.y, }); } diff --git a/src/lib/util.ts b/src/lib/util.ts index a7fa3cd3..cf0de306 100644 --- a/src/lib/util.ts +++ b/src/lib/util.ts @@ -44,9 +44,9 @@ export async function bitmapToImageData(bitmap: ImageBitmap): Promise } /** Replace the contents of a canvas with the given bitmap */ -export function drawBitmapToCanvas(canvas: HTMLCanvasElement, img: ImageBitmap) { +export function drawBitmapToCanvas(canvas: HTMLCanvasElement, bitmap: ImageBitmap) { const ctx = canvas.getContext('2d'); if (!ctx) throw Error('Canvas not initialized'); ctx.clearRect(0, 0, canvas.width, canvas.height); - ctx.drawImage(img, 0, 0); + ctx.drawImage(bitmap, 0, 0); }