Add JPEG XL interop-2026 coverage and decoder tests [chromium/src : main]
Helmut Januschka (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Philip Jägenstedt
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Jägenstedt (Gerrit)
Philip Jägenstedt added 1 comment![]( "Open in Gerrit")
8x8_noise.jxl \How was this and other files here created? Mistakes are always possible, so I'm trying to figure out how we'd notice if some of these files are actually more plain JXL files than the filenames suggest by accident.
Related details
- Helmut Januschka
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Jägenstedt (Gerrit)
Philip Jägenstedt added 6 comments![]( "Open in Gerrit")
I've taken a high level look. My main reaction is that we'll also need more reftests, for example a reftest of the 10- and 12-bit images to ensure they're not just successfully decoded, but result in the expected 8-bit screenshot.
<title>JPEG XL interop: alpha with modular fixture</title>No need to put the "in interop" bit in the titles:
```suggestion
<title>JPEG XL: alpha with modular fixture</title>
```
<title>JPEG XL interop: animation basics via ImageDecoder</title>Can you rename this test so that it's listed alongside the other ImageDecoder test? Maybe imagedecoder-animation.https.html
assert_true('ImageDecoder' in self, 'ImageDecoder should exist');This kind of precondition (dependency on another feature) can use a special assert:
```suggestion
assert_implements('ImageDecoder' in self, 'ImageDecoder should exist');
```
<title>JPEG XL integration: ImageDecoder API</title>All of the tests in this directory are for JPEG XL, can you remove "-jpegxl" from the filename?
Content-Type: image/jxlIs this only needed for this .jxl file but not the others?
What Content-Type header does wptserve use by default? Is it possible to just update wptserve to serve .jxl as image/jxl by default?
Helmut Januschka (Gerrit)
Helmut Januschka added 7 comments![]( "Open in Gerrit")
I've taken a high level look. My main reaction is that we'll also need more reftests, for example a reftest of the 10- and 12-bit images to ensure they're not just successfully decoded, but result in the expected 8-bit screenshot.
Done. Added reftests for every unique JXL resource:
- bitdepth-10bpc, bitdepth-12bpc, bitdepth-16bpc
- jxl-art-basic, layers-basic, progressive-dc-observable
- vardct-large-blocks, vardct-small-blocks, icc-uncommon-profile
- basic-decode, orientation-identity, orientation-rotate-90-ccw
PNG references generated with jxl-rs `jxl_cli --data-type u8` to match the Chromium decoder output. The 12bpc reftest uses `fuzzy maxDifference=0-1; totalPixels=0-100000` for off-by-one alpha compositing rounding, all others are pixel-exact.
While adding these I found that `issue648_palette0.jxl` failed to decode because we did not yet release https://github.com/libjxl/jxl-rs/pull/666
<title>JPEG XL interop: alpha with modular fixture</title>No need to put the "in interop" bit in the titles:
```suggestion
<title>JPEG XL: alpha with modular fixture</title>
```
Done
<title>JPEG XL interop: animation basics via ImageDecoder</title>Can you rename this test so that it's listed alongside the other ImageDecoder test? Maybe imagedecoder-animation.https.html
Done
assert_true('ImageDecoder' in self, 'ImageDecoder should exist');This kind of precondition (dependency on another feature) can use a special assert:
```suggestion
assert_implements('ImageDecoder' in self, 'ImageDecoder should exist');
```
Done
All of the tests in this directory are for JPEG XL, can you remove "-jpegxl" from the filename?
Done
Is this only needed for this .jxl file but not the others?
What Content-Type header does wptserve use by default? Is it possible to just update wptserve to serve .jxl as image/jxl by default?
Done
How was this and other files here created? Mistakes are always possible, so I'm trying to figure out how we'd notice if some of these files are actually more plain JXL files than the filenames suggest by accident.
- the script generates the 3x3 fixtures from scratch using `convert`/`cjxl`/`djxl`
- copies from a local ~/jxl-rs checkout`jxl/resources/test/` has curated test images named.
- The `conformance_test_images/` files are from the official JPEG XL conformance test suite used by all implementations
Related details
- Philip Jägenstedt
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Honey B (Gerrit)
Honey B added 1 comment![]( "Open in Gerrit")
I would suggest adding some edge cases like crbug.com/484214291 and crbug.com/484171917.
Related details
- Helmut Januschka
- Philip Jägenstedt
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Helmut Januschka (Gerrit)
Helmut Januschka added 2 comments![]( "Open in Gerrit")
I would suggest adding some edge cases like crbug.com/484214291 and crbug.com/484171917.
Done. Added edge case coverage for both bugs:
- crbug.com/484214291: C++ unit test + WPT with reftest for the smallest valid 12-byte JXL bitstream.
- crbug.com/484171917: C++ unit test verifying the decoder correctly rejects the corrupt 14-byte bitstream at the decoder level.
Didn't add a WPT for this one because making the browser fire onerror (instead of silently loading a transparent image) requires invasive changes across the core image pipeline.
note for my self: once review is satisified, pre-land a CL with decoder changes, to keep this CL WPT-only
Related details
- Honey B
- Philip Jägenstedt
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |