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.

Sign up for GitHub

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

Jump to bottom

Faster Transform Speed: sort output transform last #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Faster Transform Speed: sort output transform last #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b39997
refactor(tree.ts): sort output transform option last. transforms faster
lablancas Nov 19, 2021
633284f
test(tree.spec.ts): update tests to verify output transform is last i…
lablancas Nov 19, 2021
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
22 changes: 11 additions & 11 deletions src/tree.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe('makePictureTree', () => {
keys: false,
});
const imgSrcset = `${result(baseURL, 'resize=width:320')} 320w, ${result(baseURL, 'resize=width:640')} 640w`;
const url = `${result(baseURL, 'output=format:webp/resize=width:320')} 320w, ${result(baseURL, 'output=format:webp/resize=width:640')} 640w`;
const url = `${result(baseURL, 'resize=width:320/output=format:webp')} 320w, ${result(baseURL, 'resize=width:640/output=format:webp')} 640w`;
const expected = {
sources: [
{
Expand Down Expand Up @@ -213,23 +213,23 @@ describe('makePictureTree', () => {
{
media: '(min-width: 640px)',
sizes: '90vw',
srcSet: `${result(baseURL, 'output=format:webp/resize=width:640')} 640w`,
srcSet: `${result(baseURL, 'resize=width:640/output=format:webp')} 640w`,
type: 'image/webp',
},
{
media: '(min-width: 640px)',
sizes: '90vw',
srcSet: `${result(baseURL, 'output=format:jpg/resize=width:640')} 640w`,
srcSet: `${result(baseURL, 'resize=width:640/output=format:jpg')} 640w`,
type: 'image/jpg',
},
{
sizes: '80vw',
srcSet: `${result(baseURL, 'output=format:webp/resize=width:640')} 640w`,
srcSet: `${result(baseURL, 'resize=width:640/output=format:webp')} 640w`,
type: 'image/webp',
},
{
sizes: '80vw',
srcSet: `${result(baseURL, 'output=format:jpg/resize=width:640')} 640w`,
srcSet: `${result(baseURL, 'resize=width:640/output=format:jpg')} 640w`,
type: 'image/jpg',
},
],
Expand All @@ -255,7 +255,7 @@ describe('makePictureTree', () => {
sources: [
{
sizes: '700px',
srcSet: `${result(baseURL, 'output=format:webp/resize=width:640')} 640w`,
srcSet: `${result(baseURL, 'resize=width:640/output=format:webp')} 640w`,
type: 'image/webp',
},
],
Expand All @@ -281,10 +281,10 @@ describe('makePictureTree', () => {
keys: false,
});
const imgSrcset = `${result(baseURL, 'resize=width:320')} 320w, ${result(baseURL, 'resize=width:640')} 640w`;
const srcSet1 = `${result(baseURL, 'output=format:jpg/resize=width:320')} 320w, ${result(baseURL, 'output=format:jpg/resize=width:640')} 640w`;
const srcSet2 = `${result(baseURL, 'output=format:webp/resize=width:320')} 320w, ${result(baseURL, 'output=format:webp/resize=width:640')} 640w`;
const srcSet3 = `${result(baseURL, 'output=format:jpg/resize=width:320')} 320w, ${result(baseURL, 'output=format:jpg/resize=width:640')} 640w`;
const srcSet4 = `${result(baseURL, 'output=format:webp/resize=width:320')} 320w, ${result(baseURL, 'output=format:webp/resize=width:640')} 640w`;
const srcSet1 = `${result(baseURL, 'resize=width:320/output=format:jpg')} 320w, ${result(baseURL, 'resize=width:640/output=format:jpg')} 640w`;
const srcSet2 = `${result(baseURL, 'resize=width:320/output=format:webp')} 320w, ${result(baseURL, 'resize=width:640/output=format:webp')} 640w`;
const srcSet3 = `${result(baseURL, 'resize=width:320/output=format:jpg')} 320w, ${result(baseURL, 'resize=width:640/output=format:jpg')} 640w`;
const srcSet4 = `${result(baseURL, 'resize=width:320/output=format:webp')} 320w, ${result(baseURL, 'resize=width:640/output=format:webp')} 640w`;
const expected = {
sources: [
{
Expand Down Expand Up @@ -470,7 +470,7 @@ describe('makePictureTree', () => {
};
const expected = {
img: {
src: 'https://cdn.filestackcontent.com/output=format:webp/quality=value:5/sepia=tone:70/seW1thvcR1aQBfOCF8bX',
src: 'https://cdn.filestackcontent.com/quality=value:5/sepia=tone:70/output=format:webp/seW1thvcR1aQBfOCF8bX',
Copy link

@gr00by gr00by Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately it will have a negative impact in this case scenario. If the input file was an image that doesn't support the quality setting, let's say a png image, the quality task wouldn't be applied, as the image would still be a png at this point. In the old example the image would be converted to webp before running the quality task, making it possible to apply the quality setting. Notice the difference in image quality in the below examples:
https://cdn.filestackcontent.com/quality=v:1/output=f:webp/LJNf24eDS1O7GsbpaEMs
https://cdn.filestackcontent.com/output=f:webp/quality=v:1/LJNf24eDS1O7GsbpaEMs

I'm not familiar with the adaptive package, but it looks to me that it might require a greater refactoring in order to make it possible to apply the output task as the last task in chain (which makes perfect sense) with being backwards compatible at the same time.

@pcholuj @sebastian-wec @promanski

Copy link
Author

@lablancas lablancas Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@gr00by87 I could update the sort function so that quality is sorted after output, but I am not sure if sepia (or other transformations) should also be sorted after the output transform?

},
};
const tree = makePictureTree(handle, options);
Expand Down
8 changes: 4 additions & 4 deletions src/tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ const createFileLink = (handle: FileHandle, fileLinkOptions: FileLinkOptions) =>
fileLink.setUseValidator(false);
}

Object.keys(fileLinkOptions.transform).sort(outputFirstSort).forEach((key: keyof TransformOptions) => {
Object.keys(fileLinkOptions.transform).sort(outputLastSort).forEach((key: keyof TransformOptions) => {
fileLink = fileLink.addTask(key, fileLinkOptions.transform[key]);
});
if (fileLinkOptions.cname) {
Expand All @@ -151,11 +151,11 @@ const createFileLink = (handle: FileHandle, fileLinkOptions: FileLinkOptions) =>
};

/**
* Sort array of keys in a way that 'output' is always the first
* Sort array of keys in a way that 'output' is always the last
* @param previousKey - First key to be compared in a sort function
*/
const outputFirstSort = (previousKey: string, nextKey: string) => {
return previousKey === 'output' ? -1 : nextKey === 'output' ? 1 : 0;
const outputLastSort = (previousKey: string, nextKey: string) => {
return previousKey === 'output' ? 1 : nextKey === 'output' ? -1 : 0;
};

const getWidth = (width?: number | string) => (resolution: number | string) => {
Expand Down