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

Options UI #39

Merged
merged 40 commits into from
Jun 29, 2018
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a2bec39
Initial work to add Options config
developit May 23, 2018
837e38a
Use a single encoder instance and retry up to 10 times on failure.
developit May 24, 2018
64eb3f9
Switch both sides to allow encoding from the source image, add option…
developit May 25, 2018
22ef181
Styling for options (and a few tweaks for the app)
developit May 25, 2018
8b73be0
Dep updates.
developit May 25, 2018
25c0805
Remove commented out code.
developit May 25, 2018
59a3517
Fix Encoder typing
developit May 25, 2018
06e4483
Fix lint issues
developit May 25, 2018
bb4ac4c
Apparently I didnt have tslint autofix enabled on the chromebook
developit May 25, 2018
4d4d9b1
Attempt to fix layout/panning issues
developit Jun 6, 2018
70d2927
Fix missing custom element import!
developit Jun 7, 2018
e3ca13c
Fix variable naming, remove dynamic encoder names, remove retry, allo…
developit Jun 19, 2018
b069c7f
Refactor state management to use an Array of objects and immutable up…
developit Jun 19, 2018
d4c4ae4
Add Identity encoder, which is a passthrough encoder that handles the…
developit Jun 19, 2018
7d136db
Drop comlink-loader into the project and add ".worker" to the jpeg en…
developit Jun 19, 2018
11e6197
lint fixes.
developit Jun 19, 2018
13cac89
cleanup
developit Jun 19, 2018
9776670
smaller PR feedback fixes
developit Jun 19, 2018
820d6f5
rename "jpeg" codec to "MozJpeg"
developit Jun 19, 2018
8e7d468
Formatting fixes for Options
developit Jun 19, 2018
148faa8
Colocate codecs and their options UIs in src/codecs, and standardize …
developit Jun 19, 2018
c21413a
Handle canvas errors
developit Jun 19, 2018
8e27c5e
Throw if quality is undefined, add default quality
developit Jun 19, 2018
e83628a
add note about temp styles
developit Jun 19, 2018
e7fce7d
add note about temp styles [2]
developit Jun 19, 2018
b4ea56f
Renaming updateOption
jakearchibald Jun 26, 2018
d85218a
Clarify option input bindings
developit Jun 26, 2018
984b5fe
Move updateCanvas() to util and rename to drawBitmapToCanvas
developit Jun 26, 2018
62fa05d
use generics to pass through encoder options
developit Jun 26, 2018
9fffc2a
Remove unused dependencies
developit Jun 26, 2018
f1cbd26
fix options type
developit Jun 26, 2018
453e315
const
developit Jun 26, 2018
da6d1c4
Use `Array.prototype.some()` for image loading check
developit Jun 26, 2018
a7f53ed
Display encoding errors in the UI.
developit Jun 26, 2018
d618310
I fought typescript and I think I won
jakearchibald Jun 28, 2018
3ee32c9
This doesn't need to be optional
jakearchibald Jun 28, 2018
3d3034f
Quality isn't optional
jakearchibald Jun 28, 2018
d0ad2b2
Simplifying comlink casting
jakearchibald Jun 29, 2018
c94e6dc
Splitting counters into loading and displaying
jakearchibald Jun 29, 2018
cbd84d3
Still loading if the loading counter isn't equal.
jakearchibald Jun 29, 2018
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
14,491 changes: 0 additions & 14,491 deletions package-lock.json

This file was deleted.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@
},
"dependencies": {
"classnames": "^2.2.5",
"comlink": "^3.0.3",
"comlink-loader": "^1.0.0",
"material-components-web": "^0.32.0",
"material-radial-progress": "git+https://gist.github.com/02134901c77c5309924bfcf8b4435ebe.git",
"preact": "^8.2.7",
"preact-i18n": "^1.2.0",
"preact-material-components": "^1.3.7",
"preact-material-components-drawer": "git+https://gist.github.com/a78fceed440b98e62582e4440b86bfab.git",
"preact-router": "^2.6.0"
}
}
15 changes: 15 additions & 0 deletions src/codecs/encoders.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import * as mozJPEG from './mozjpeg/encoder';
import { EncoderState as MozJPEGEncodeData, EncodeOptions as MozJPEGEncodeOptions } from './mozjpeg/encoder';
import * as identity from './identity/encoder';
import { EncoderState as IdentityEncodeData, EncodeOptions as IdentityEncodeOptions } from './identity/encoder';

export type EncoderState = IdentityEncodeData | MozJPEGEncodeData;
export type EncoderOptions = IdentityEncodeOptions | MozJPEGEncodeOptions;
export type EncoderType = keyof typeof encoderMap;

export const encoderMap = {
[identity.type]: identity,
[mozJPEG.type]: mozJPEG
};

export const encoders = Array.from(Object.values(encoderMap));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to edit this file as we add new encoders. It isn't as DRY as I'd like, but I couldn't get TypeScript to do better.

6 changes: 6 additions & 0 deletions src/codecs/identity/encoder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export interface EncodeOptions {}
export interface EncoderState { type: typeof type; options: EncodeOptions; }

export const type = 'identity';
export const label = 'Original image';
export const defaultOptions: EncodeOptions = {};
17 changes: 17 additions & 0 deletions src/codecs/mozjpeg/encoder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Encoder from './encoder.worker';

export interface EncodeOptions { quality?: number; }
export interface EncoderState { type: typeof type; options: EncodeOptions; }

export const type = 'mozjpeg';
export const label = 'MozJPEG';
export const mimeType = 'image/jpeg';
export const extension = 'jpg';
export const defaultOptions: EncodeOptions = { quality: 7 };

export async function encode(data: ImageData, options: EncodeOptions = {}) {
// This is horrible because TypeScript doesn't realise that
// Encoder has been comlinked.
const encoder = new Encoder() as any as Promise<Encoder>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that sucks. I had wondered about having encoders export an async factory function instead just to make the worker boundary transparent.

return (await encoder).encode(data, options);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All encoders will export these things, except identity which doesn't encode, or have a static extensions / mime type etc.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All encoders will export these things, except identity which doesn't encode, or have a static extensions / mime type etc.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Encoder } from './codec';

import mozjpeg_enc from '../../../codecs/mozjpeg_enc/mozjpeg_enc';
// Using require() so TypeScript doesn’t complain about this not being a module.
import { EncodeOptions } from './encoder';
const wasmBinaryUrl = require('../../../codecs/mozjpeg_enc/mozjpeg_enc.wasm');

// API exposed by wasm module. Details in the codec’s README.
Expand All @@ -15,9 +14,10 @@ interface ModuleAPI {
get_result_size(): number;
}

export class MozJpegEncoder implements Encoder {
export default class MozJpegEncoder {
private emscriptenModule: Promise<EmscriptenWasm.Module>;
private api: Promise<ModuleAPI>;

constructor() {
this.emscriptenModule = new Promise((resolve) => {
const m = mozjpeg_enc({
Expand Down Expand Up @@ -54,18 +54,22 @@ export class MozJpegEncoder implements Encoder {
encode: m.cwrap('encode', '', ['number', 'number', 'number', 'number']),
free_result: m.cwrap('free_result', '', []),
get_result_pointer: m.cwrap('get_result_pointer', 'number', []),
get_result_size: m.cwrap('get_result_size', 'number', []),
get_result_size: m.cwrap('get_result_size', 'number', [])
};
})();
}

async encode(data: ImageData): Promise<ArrayBuffer> {
async encode(data: ImageData, options: EncodeOptions): Promise<ArrayBuffer> {
if (!options.quality) {
throw Error('MozJpeg: options.quality is required');
}

const m = await this.emscriptenModule;
const api = await this.api;

const p = api.create_buffer(data.width, data.height);
m.HEAP8.set(data.data, p);
api.encode(p, data.width, data.height, 2);
api.encode(p, data.width, data.height, options.quality);
const resultPointer = api.get_result_pointer();
const resultSize = api.get_result_size();
const resultView = new Uint8Array(m.HEAP8.buffer, resultPointer, resultSize);
Expand Down
35 changes: 35 additions & 0 deletions src/codecs/mozjpeg/options.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { h, Component } from 'preact';
import { EncodeOptions } from './encoder';
import { bind } from '../../lib/util';

type Props = {
options: EncodeOptions,
onChange(newOptions: EncodeOptions): void
};

export default class MozJpegCodecOptions extends Component<Props, {}> {
@bind
onChange(event: Event) {
const el = event.currentTarget as HTMLInputElement;
this.props.onChange({ quality: Number(el.value) });
}

render({ options }: Props) {
return (
<div>
<label>
Quality:
<input
name="quality"
type="range"
min="1"
max="100"
step="1"
value={'' + options.quality}
onChange={this.onChange}
/>
</label>
</div>
);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed state from this component. The form elements are still controlled, but by the app.

188 changes: 162 additions & 26 deletions src/components/app/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,56 +2,192 @@ import { h, Component } from 'preact';
import { bind, bitmapToImageData } from '../../lib/util';
import * as style from './style.scss';
import Output from '../output';
import Options from '../options';

import {MozJpegEncoder} from '../../lib/codec-wrappers/mozjpeg-enc';
import * as mozJPEG from '../../codecs/mozjpeg/encoder';
import * as identity from '../../codecs/identity/encoder';
import { EncoderState, EncoderType, EncoderOptions, encoderMap } from '../../codecs/encoders';

type Props = {};
interface SourceImage {
file: File;
bmp: ImageBitmap;
data: ImageData;
}

interface EncodedImage {
encoderState: EncoderState;
bmp?: ImageBitmap;
counter: number;
loading: boolean;
}

type State = {
img?: ImageBitmap
};
interface Props {}

interface State {
source?: SourceImage;
images: [EncodedImage, EncodedImage];
loading: boolean;
error?: string;
}

export default class App extends Component<Props, State> {
state: State = {};
state: State = {
loading: false,
images: [
{
encoderState: { type: identity.type, options: identity.defaultOptions },
counter: 0,
loading: false
},
{
encoderState: { type: mozJPEG.type, options: mozJPEG.defaultOptions },
counter: 0,
loading: false
}
]
};

constructor() {
super();
// In development, persist application state across hot reloads:
if (process.env.NODE_ENV === 'development') {
this.setState(window.STATE);
this.componentDidUpdate = () => {
let oldCDU = this.componentDidUpdate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Care to explain? 😅

this.componentDidUpdate = (props, state) => {
if (oldCDU) oldCDU.call(this, props, state);
window.STATE = this.state;
};
}
}

onEncoderChange(index: 0 | 1, type: EncoderType, options?: EncoderOptions): void {
const images = this.state.images.slice() as [EncodedImage, EncodedImage];
const image = 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
} as EncoderState;

images[index] = {
...image,
encoderState,
loading: true,
counter: image.counter++
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++image.counter ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Will fix.

};

this.setState({ images });
}

onOptionsChange(index: 0 | 1, options: EncoderOptions): void {
this.onEncoderChange(index, this.state.images[index].encoderState.type, options);
}

componentDidUpdate(prevProps: Props, prevState: State): void {
const { source, images } = this.state;

for (const [i, image] of images.entries()) {
if (source !== prevState.source || image !== prevState.images[i]) {
this.updateImage(i);
}
}
}
Copy link
Collaborator

@jakearchibald jakearchibald Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like us to have a think about the best kind of debouncing to use in these cases, but not needed for this PR.

  • remove debounce


@bind
async onFileChange(event: Event) {
async onFileChange(event: Event): Promise<void> {
const fileInput = event.target as HTMLInputElement;
if (!fileInput.files || !fileInput.files[0]) return;
// TODO: handle decode error
const bitmap = await createImageBitmap(fileInput.files[0]);
const data = await bitmapToImageData(bitmap);
const encoder = new MozJpegEncoder();
const compressedData = await encoder.encode(data);
const blob = new Blob([compressedData], {type: 'image/jpeg'});
const compressedImage = await createImageBitmap(blob);
this.setState({ img: compressedImage });
}

render({ }: Props, { img }: State) {
const file = fileInput.files && fileInput.files[0];
if (!file) return;
this.setState({ loading: true });
try {
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,
loading: false
});
} catch (err) {
this.setState({ error: 'IMAGE_INVALID', loading: false });
}
}

async updateImage(index: number): Promise<void> {
const { source, images } = this.state;
if (!source) return;
let image = images[index];

// Each time we trigger an async encode, the ID changes.
const id = ++image.counter;
image.loading = true;
this.setState({ });
const result = await this.updateCompressedImage(source, image.encoderState);
image = this.state.images[index];
// If another encode has been initiated since we started, ignore this one.
if (image.counter !== id) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I’m misunderstanding, but if image.counter < id it means an older encoding job has finished and we shouldn’t early return?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I messed up the counter pretty well. Will fix.

image.bmp = result;
image.loading = false;
this.setState({ });
}

async updateCompressedImage(source: SourceImage, encodeData: EncoderState): Promise<ImageBitmap> {
// Special case for identity
if (encodeData.type === identity.type) return source.bmp;

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;
} catch (err) {
this.setState({ error: `Encoding error (type=${encodeData.type}): ${err}` });
throw err;
}
}

render({ }: Props, { loading, error, images, source }: State) {
const [leftImageBmp, rightImageBmp] = images.map(i => i.bmp);

loading = loading || images.some(image => image.loading);

return (
<div id="app" class={style.app}>
{img ?
<Output img={img} />
:
<div>
{(leftImageBmp && rightImageBmp) ? (
<Output leftImg={leftImageBmp} rightImg={rightImageBmp} />
) : (
<div class={style.welcome}>
<h1>Select an image</h1>
<input type="file" onChange={this.onFileChange} />
</div>
}
)}
{images.map((image, index) => (
<span class={index ? style.rightLabel : style.leftLabel}>{encoderMap[image.encoderState.type].label}</span>
))}
{images.map((image, index) => (
<Options
class={index ? style.rightOptions : style.leftOptions}
encoderState={image.encoderState}
onTypeChange={this.onEncoderChange.bind(this, index)}
onOptionsChange={this.onOptionsChange.bind(this, index)}
/>
))}
{loading && <span style={{ position: 'fixed', top: 0, left: 0 }}>Loading...</span>}
{error && <span style={{ position: 'fixed', top: 0, left: 0 }}>Error: {error}</span>}
</div>
);
}
}

Loading