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

Fix image sizing penalty to use CSS terms #99

Merged
merged 7 commits into from Jul 19, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions index.bs
Expand Up @@ -199,13 +199,13 @@ In order to <dfn export>potentially add a {{LargestContentfulPaint}} entry</dfn>
1. Let |rootWidth| be |root|'s <a>viewport</a>'s width, excluding any scrollbars.
1. Let |rootHeight| be |root|'s <a>viewport</a>'s height, excluding any scrollbars.
1. If |size| is equal to |rootWidth| times |rootHeight|, return.
1. If |imageRequest| is not null, run the following steps:
1. Let |naturalWidth| and |naturalHeight| be the outputs of running the same steps for an <{img}>'s {{img/naturalWidth}} and {{img/naturalHeight}} attribute getters, but using |imageRequest| as the image.
1. Let |naturalSize| be <code>|naturalWidth| * |naturalHeight|</code>.
1. Let |displayWidth| and |displayHeight| be the outputs of running the same steps for an <{img}>'s {{img/width}} and {{img/height}} attribute getters, but using |imageRequest| as the image.
1. Let |displaySize| be <code>|displayWidth| * |displayHeight|</code>.
1. Let |penaltyFactor| be <code>min(|displaySize|, |naturalSize|) / |displaySize|</code>.
1. Multiply |size| by |penaltyFactor|.
1. If |imageRequest| is not null, run the following steps to reduce the computed size of upscaled images:
1. Let (|naturalWidth|, |naturalHeight|) be |imageRequest|'s [=natural dimension=].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be natural dimensions ie plural? Also this is clear but not technically correct as those are a triple and here you have two variables. I think you can use natural sizes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an existing reference to CSS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

1. Let (|concreteWidth|, |concreteHeight|) be |imageRequest|'s [=concrete object size=].
noamr marked this conversation as resolved.
Show resolved Hide resolved
1. Let |naturalArea| be <code>|naturalWidth| * |naturalHeight|</code>.
1. Let |concreteArea| be <code>|concreteWidth| * |concreteHeight|</code>.
1. Let |scaleFactor| be <code>|concreteArea| / |naturalArea|</code>.
yoavweiss marked this conversation as resolved.
Show resolved Hide resolved
1. If |scaleFactor| is greater than 1, then divide |size| by |scaleFactor|.
noamr marked this conversation as resolved.
Show resolved Hide resolved
1. If |size| is less than or equal to |document|'s [=largest contentful paint size=], return.
1. Let |contentInfo| be a <a>map</a> with |contentInfo|["size"] = |size|, |contentInfo|["url"] = |url|, |contentInfo|["id"] = |id|, |contentInfo|["renderTime"] = |renderTime|, |contentInfo|["loadTime"] = |loadTime|, and contentInfo["element"] = |element|.
1. <a>Create a LargestContentfulPaint entry</a> with |contentInfo|, and |document| as inputs.
Expand Down