Initial implementation for JXL
Categories
(Core :: Graphics: ImageLib, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox90 | --- | fixed |
People
(Reporter: saschanaz, Assigned: saschanaz)
References
(Blocks 1 open bug)
Details
(Keywords: perf-alert)
Attachments
(5 files)
Assignee | ||
Comment 1•2 years ago
|
||
Assignee | ||
Comment 2•2 years ago
|
||
Depends on D113358
Assignee | ||
Comment 3•2 years ago
|
||
Depends on D113359
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 4•2 years ago
|
||
Depends on D113360
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/01bb0e75cb82 Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/40a3f43a2306 Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/3e76ff83ebe2 Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/f3f40c6fb8e2 Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Comment 6•2 years ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f591c3da4311 Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/a3f23a20b532 Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/4b69f99caf69 Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/4ccdb31e1c5a Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Assignee | ||
Updated•2 years ago
|
Comment 8•2 years ago
|
||
Backed out 4 changesets (bug 1707590) for worker/checkouts/gecko/config/rules.mk. CLOSED TREE
Log:
https://treeherder.mozilla.org/logviewer?job_id=338698659&repo=autoland&lineNumber=83749
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=4ccdb31e1c5a4944cbf02b2e613d676c2c69dfbf
Backout:
https://hg.mozilla.org/integration/autoland/rev/dca9a0ba9cbc552051c5264c7a0aed9c483915e1
Assignee | ||
Comment 9•2 years ago
|
||
Blocked by clang 5 compatibility: https://gitlab.com/wg1/jpeg-xl/-/issues/227
Comment 10•2 years ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9ab607973f20 Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/9dfd0d516062 Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/3a7e5c98dd13 Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/fec7b65cc3d7 Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Comment 11•2 years ago
|
||
Backed out for causing gtest crashes.
backout: https://hg.mozilla.org/integration/autoland/rev/9f83edbeb83f7dbee267aed95452041fcdc73003
failure logs:
- PROCESS-CRASH | gtest | application crashed [@ mozilla::Maybe<mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> >::operator*() const &]
- PROCESS-CRASH | gtest | application crashed [@ mozilla::image::SourceBuffer::ExpectLength(unsigned long)]
- REFTEST TEST-UNEXPECTED-FAIL | image/test/reftest/jxl/jxl-size-33x33.jxl == image/test/reftest/jxl/jxl-size-33x33.png | image comparison, max difference: 207, number of differing pixels: 6931
Assignee | ||
Comment 12•2 years ago
|
||
Oh no, https://treeherder.mozilla.org/jobs?repo=try&revision=35798c94cfa7a6f1141d61c561bb305514e5d282 was all green, has there been another change before my patches? 🤔
Assignee | ||
Comment 13•2 years ago
|
||
It's still green https://treeherder.mozilla.org/jobs?repo=try&revision=28f5078c8e7068e88fab3c73f7b24fba616f48a4 and also locally has no problem, so I'll give it another try.
Comment 14•2 years ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25da391baa22 Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/9c7204c3f03c Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/e5d2d6824408 Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/6edab66fe2c5 Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Assignee | ||
Comment 15•2 years ago
|
||
It's failing again, not sure why it fails on autoland but not on try/local?
Comment 16•2 years ago
|
||
Backed out 4 changesets (Bug 1707590) for causing gtest crashes CLOSED TREE
Log: https://treeherder.mozilla.org/logviewer?job_id=338871044&repo=autoland&lineNumber=1739
https://treeherder.mozilla.org/logviewer?job_id=338870932&repo=autoland&lineNumber=5243
Backout: https://hg.mozilla.org/integration/autoland/rev/56d36c0ea666d8e31477f60d72986b87240c6da5
Assignee | ||
Comment 17•2 years ago
|
||
gtest is failing even before creating the decoder, I think LoadFile()
fails to read my added jxl files and is just returning zero bytes?
Comment 18•2 years ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3201d860afef Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/21f41a9df048 Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/8cc3eaf728b6 Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/3bd322893127 Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Assignee | ||
Comment 19•2 years ago
|
||
Canceled the CI jobs, hoping it backs out early...
Comment 20•2 years ago
|
||
Backed out 4 changesets (Bug 1707590) for causing gtest crashes CLOSED TREE
Log: https://treeherder.mozilla.org/logviewer?job_id=338889882&repo=autoland&lineNumber=5245
and there are also some TV failures which need attention: https://treeherder.mozilla.org/logviewer?job_id=338890911&repo=autoland&lineNumber=2234
Backout: https://hg.mozilla.org/integration/autoland/rev/c100827b4f4acf98cf34ab52de6911b2576c7f8a
Comment 21•2 years ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4d806a39de22 Part 1: Add vendored libjxl source r=aosmond https://hg.mozilla.org/integration/autoland/rev/c4a3f9962009 Part 2: Implement nsJXLDecoder r=tnikkel https://hg.mozilla.org/integration/autoland/rev/b9cbeba9c10b Part 3: Add feature gate for JXL r=tnikkel,fluent-reviewers,preferences-reviewers https://hg.mozilla.org/integration/autoland/rev/09d918e34a59 Part 4: Add image/jxl to Accept header and DownloadsViewableInternally r=necko-reviewers,dragana
Assignee | ||
Comment 22•2 years ago
|
||
I hope it finally land this time 🙏
Assignee | ||
Comment 23•2 years ago
|
||
Just to note, the empty file issue is bug 1709608.
Comment 24•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4d806a39de22
https://hg.mozilla.org/mozilla-central/rev/c4a3f9962009
https://hg.mozilla.org/mozilla-central/rev/b9cbeba9c10b
https://hg.mozilla.org/mozilla-central/rev/09d918e34a59
Comment 25•2 years ago
•
|
||
This broke the build on !tier 1 builds, such as ppc64 linux, s390x linux, mips64el linux, etc.
Hitting this: https://hg.mozilla.org/mozilla-central/file/tip/third_party/highway/hwy/base.h#l254
Comment 26•2 years ago
|
||
For record. Solaris SPARC is also affected:
third_party/highway/hwy/targets.h:378:14: error: ?HWY_HIGHEST_TARGET_BIT? was not declared in this scope; did you mean ?HWY_HIGHEST_TARGET_BIT_RVV??
Assignee | ||
Comment 27•2 years ago
|
||
Oops. Reported it to https://github.com/google/highway/issues/183
Comment 28•2 years ago
|
||
Thank you for reporting this. We've fixed it in Highway, JPEG XL will still need to update the submodule.
Assignee | ||
Comment 29•2 years ago
•
|
||
This is fixed in upstream. Is it urgent or can we wait for libjxl to update its dependency? If it's urgent I guess we can try updating highway only.
Comment 30•2 years ago
|
||
(In reply to Pulsebot from comment #21)
https://hg.mozilla.org/integration/autoland/rev/09d918e34a59
Part 4: Add image/jxl to Accept header and DownloadsViewableInternally
r=necko-reviewers,dragana
== Change summary for alert #29985 (as of Thu, 06 May 2021 15:09:52 GMT) ==
Regressions:
Ratio | Suite | Test | Platform | Options | Absolute values (old vs new) |
---|---|---|---|---|---|
0.43% | installer size | osx-shippable | instrumented | 115,258,681.46 -> 115,750,972.67 | |
0.34% | installer size | osx-shippable | nightly | 85,778,752.88 -> 86,073,304.83 | |
0.30% | installer size | osx-aarch64-shippable | aarch64 nightly | 82,078,135.54 -> 82,322,820.75 |
For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=29985
Comment 31•2 years ago
|
||
(In reply to Kagami :saschanaz [away until 2021-05-17] from comment #29)
This is fixed in upstream. Is it urgent or can we wait for libjxl to update its dependency? If it's urgent I guess we can try updating highway only.
I would prefer an early fix by updating highway. Having builds broken for longer time makes our CI for ppc/s390x/... less useful as we can't catch possibly newly introduced issues.
Comment 32•2 years ago
|
||
(In reply to Dan Horák from comment #31)
(In reply to Kagami :saschanaz [away until 2021-05-17] from comment #29)
This is fixed in upstream. Is it urgent or can we wait for libjxl to update its dependency? If it's urgent I guess we can try updating highway only.
I would prefer an early fix by updating highway. Having builds broken for longer time makes our CI for ppc/s390x/... less useful as we can't catch possibly newly introduced issues.
The same for me for Solaris SPARC. Thank you!
Comment 33•2 years ago
|
||
also broken on armv7a-linux:
19:53.10 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-90.0/work/firefox-90.0/third_party/highway/hwy/ops/arm_neon-inl.h:876:326: error: no matching function for call to 'hwy::N_NEON::Mask128<float, 4>::Mask128(uint32x4_t)'
would be great if the upstream fix can be backported into the beta phase of firefox-90.0, so that beta testers can provide feedback if the fix actually works.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 34•1 year ago
|
||
the jxl code is not listed in toolkit/content/license.html
Assignee | ||
Comment 35•1 year ago
|
||
Assignee | ||
Updated•1 year ago
|
Comment 36•1 year ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6cf0774534a5 Part 4: Add references to libjxl and highway in about:license r=mhoye
Comment 37•1 year ago
|
||
bugherder |
Comment 38•1 year ago
|
||
A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)
Description
•