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

Allow an image to indicate its own density and correct its intrinsic size #5574

Merged
merged 16 commits into from
Jul 8, 2021

Conversation

noamr
Copy link
Contributor

@noamr noamr commented May 24, 2020

…size.

See https://discourse.wicg.io/t/proposal-exif-image-resolution-auto-and-from-image/4326/24

This proposal allows using EXIF or other metadata to determine an image's default resolution, to be applied regardless of the resolution specified by srcset / image-set. This resolution is written with image-resolution in mind, with image-resolution: from-image representing this EXIF image resolution, similar to how image-orientation: from-image represents EXIF orientation.

The use-case is, for example, allowing serving low-density placeholder in poor network conditions without affecting layout.

The change to the HTML spec defines how resolution is expressed in EXIF, and it would take more work to define it in other image metadata formats such as XMP.

To mitigate backwards compatibility issues, the spec change requires five different metadata attributes to be present and valid - horizontal resolution, vertical resolution, preferred width, preferred height, and resolution unit. It was confirmed using Chrome histogram that this would affect a very small percentage of websites (see conversation on PR).

(See WHATWG Working Mode: Changes for more details.)


/acknowledgements.html ( diff )
/images.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )

@noamr noamr force-pushed the intrinsic-density-correction branch from 2ace940 to 238836b Compare May 25, 2020 05:43
@annevk
Copy link
Member

annevk commented May 26, 2020

Where would the proposal be standardized?

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: img labels May 26, 2020
@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

Where would the proposal be standardized?

I don't understand the question. Isn't here where proposals are standardized?

@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

(Filing this after a conversation with @tabatkins and with @yoavweiss, where Tab had proposed to file this as an HTML PR).
After incubation on WICG (https://discourse.wicg.io/t/proposal-exif-image-resolution-auto-and-from-image/4326/24)
Working in parallel on Chromium/WebKit implementations and on WPT, but wanted to see how people feel about the proposal first.

@annevk
Copy link
Member

annevk commented May 26, 2020

Yes, HTML is such a place, but you're not adding the specifics here, right? HTML cannot forever depend on something only defined in a proposal. Some standard needs to eventually own that EXIF data definition.

@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

Yes, HTML is such a place, but you're not adding the specifics here, right? HTML cannot forever depend on something only defined in a proposal. Some standard needs to eventually own that EXIF data definition.

Agreed. The EXIF data is already standardized (resolution/size). The use of these properties in practice is specific to HTML, same as for EXIF orientation. I'd be happy to put that definition in the HTML spec or in any other place - probably wherever image-orientation EXIF should reside. Any suggestions?

@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

The EXIF standard is here: http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2019-E. However it's not really a living standard. It contains the definitions of size/resolution etc, but as said before, not the choice to abide to them only if they match, which is HTML specific.

@annevk
Copy link
Member

annevk commented May 26, 2020

I guess at some point HTML should own it or we should split out a lot of the image infrastructure into its own standard. CSS and everyone else using images are also expected to use HTML's algorithms to obtain images. (See #4474.)

@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

So should I add that in as an algorithm to HTML as part of this patch? What would be the way forward?

@annevk
Copy link
Member

annevk commented May 26, 2020

Yeah, that seems preferable if that's all there is to it.

@noamr noamr force-pushed the intrinsic-density-correction branch from 238836b to 0f651cb Compare May 26, 2020 13:10
@noamr
Copy link
Contributor Author

noamr commented May 26, 2020

Yeah, that seems preferable if that's all there is to it.

Available in new PR changes.

@noamr noamr force-pushed the intrinsic-density-correction branch 6 times, most recently from d5179c1 to 75383fa Compare May 27, 2020 08:14
@noamr
Copy link
Contributor Author

noamr commented May 27, 2020

@annevk: updated the PR with detailed EXIF algorithm, as per our IRC discussion

@noamr noamr force-pushed the intrinsic-density-correction branch 4 times, most recently from 5dc5560 to 6a6a6d8 Compare May 27, 2020 10:12
noamr added a commit to web-platform-tests/wpt that referenced this pull request May 27, 2020
These test include images that have EXIF values that specify density-corrected size
And they assert that the intrinsic size for those images is corrected by that EXIF.

WhatWG PR: whatwg/html#5574
noamr added a commit to web-platform-tests/wpt that referenced this pull request May 27, 2020
These test include images that have EXIF values that specify density-corrected size
And they assert that the intrinsic size for those images is corrected by that EXIF.

WhatWG PR: whatwg/html#5574
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@noamr noamr force-pushed the intrinsic-density-correction branch from 7fe14ad to da9f571 Compare June 27, 2021 04:33
zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Jun 29, 2021
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 30, 2021
…rigin images, a=testonly

Automatic update from web-platform-tests
Test density size correction for cross-origin images

See whatwg/html#5574

--

wpt-commits: 2c19d6ee62676ac90146e7311ba13d8eaeb241fd
wpt-pr: 29411
@yoavweiss
Copy link
Contributor

@domenic - any other blockers here?

@domenic domenic merged commit 355a0ca into whatwg:main Jul 8, 2021
@domenic
Copy link
Member

domenic commented Jul 8, 2021

Are there tests that only EXIF is supported, and other formats like XMP (which aren't in the spec) don't work?

@eeeps
Copy link
Contributor

eeeps commented Jul 8, 2021

@domenic no, but I'm not sure ① how one would create such a file, ② if there are any other metadata formats besides XMP that contain fields which map to the EXIF ResolutionUnit, XResolution, YResolution, PixelXDimension, and PixelYDimension fields, used here. I've asked the exiftool folks.

@noamr
Copy link
Contributor Author

noamr commented Jul 11, 2021

@domenic no, but I'm not sure ① how one would create such a file, ② if there are any other metadata formats besides XMP that contain fields which map to the EXIF ResolutionUnit, XResolution, YResolution, PixelXDimension, and PixelYDimension fields, used here. I've asked the exiftool folks.

Arguably, Exif embedded in XMP is still Exif. The spec doesn't limit to Binary EXIF, and I'm not sure it should go further than it does into image-codec specifics at this point in time. The line "Let exifTagMap be the EXIF tags obtained from req's image data, as defined by the relevant codec." leaves it to the codec to decide whether EXIF is binary or XMP-embedded.

The Mac/WebKit implementation supports Exif embedded in XMP in PNG images, as the CF image decoders (Mac OS APIs) do that by default. I think we should add tests for that, which would currently pass on Mac, rather than add tests to assert that Exif embedded in XMP doesn't work.

@lrosenthol
Copy link

Arguably, Exif embedded in XMP is still Exif. The spec doesn't limit to Binary EXIF, and I'm not sure it should go further than it does into image-codec specifics at this point in time.

Correct. That was the agreement early on in this thread/discussion - that how & where a processor locates the EXIF was undefined. One reason for doing so is that the location of EXIF data varies from file format to file format as noted by @noamr above.

jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 16, 2021
…rigin images, a=testonly

Automatic update from web-platform-tests
Test density size correction for cross-origin images

See whatwg/html#5574

--

wpt-commits: 2c19d6ee62676ac90146e7311ba13d8eaeb241fd
wpt-pr: 29411
@svgeesus
Copy link

One reason for doing so is that the location of EXIF data varies from file format to file format as noted by @noamr above.

Note that PNG used not to have a standard way to embed EXIF (hence people putting it in XMP) but then eXIfwas added to the extension chunks register and now, there is an issue to add it directly to the PNG spec.

As part of that, it would be good to point to the HTML LS on how browsers handle the EXIF data.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
See whatwg/html#5574

Currently applies to JPEG only.

Use the EXIF resolution/size information we read from the
JPEG image to determine a density-corrected preferred size,
which overrides the intrinsic size of the image.

We only apply this intrinsic size when an image has both
resolution and size information in its exif.

When asking an Image for display size, the preferred size
is returned, and the source rect for reading from the image
is scaled when applicable.

Added many WPT reference tests.

Bug: 1069755
Change-Id: I01a38c72bb46974a0c112175934cd3c2c48607a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292219
Commit-Queue: Stephen Chenney <[email protected]>
Reviewed-by: Stephen Chenney <[email protected]>
Cr-Commit-Position: refs/heads/master@{#817600}
GitOrigin-RevId: 86ae1272b4a60b42439bc115777bb916fa3a6720
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Using wpt-import in Chromium 1ca167279b1e149a0fa86adc0102e8f3be8ae343.
With Chromium commits locally applied on WPT:
7c0edf3443 "Add test for crbug.com/1138917"
86ae1272b4 "Read image resolution from EXIF, apply to intrinsic size See whatwg/html#5574"

Note to sheriffs: This CL imports external tests and adds
expectations for those tests; if this CL is large and causes
a few new failures, please fix the failures by adding new
lines to TestExpectations rather than reverting. See:
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md

Directory owners for changes in this CL:
[email protected]:
  external/wpt/resources

NOAUTOREVERT=true
[email protected]

No-Export: true
Cq-Include-Trybots: luci.chromium.try:linux-wpt-identity-fyi-rel,linux-wpt-payments-fyi-rel
Change-Id: Iaf79b62b9fcf5f5ceaeb388b2bb3f741a16ccca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2477373
Reviewed-by: WPT Autoroller <[email protected]>
Commit-Queue: WPT Autoroller <[email protected]>
Cr-Commit-Position: refs/heads/master@{#817734}
GitOrigin-RevId: 63931a18b27df4e94ebec891d27cc34b7fe46194
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: img
Development

Successfully merging this pull request may close these issues.