Closed Bug 1707590 Opened 3 years ago Closed 3 years ago

Initial implementation for JXL

Categories

(Core :: Graphics: ImageLib, task)

task

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: saschanaz, Assigned: saschanaz)

References

(Blocks 1 open bug)

Details

(Keywords: perf-alert)

Attachments

(5 files)

No description provided.
Attachment #9218349 - Attachment description: Bug 1707590 - Part 1: Add vendored libjxl source r=tnikkel → Bug 1707590 - Part 1: Add vendored libjxl source r=aosmond
Depends on: 1707681
See Also: 1707681
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

Backed out for causing build bustages.

Push with failures

Failure log

Backout link

Flags: needinfo?(krosylight)
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
Flags: needinfo?(krosylight)

Blocked by clang 5 compatibility: https://gitlab.com/wg1/jpeg-xl/-/issues/227

Flags: needinfo?(krosylight)
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

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.

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

It's failing again, not sure why it fails on autoland but not on try/local?

gtest is failing even before creating the decoder, I think LoadFile() fails to read my added jxl files and is just returning zero bytes?

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

Canceled the CI jobs, hoping it backs out early...

Flags: needinfo?(krosylight)
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

I hope it finally land this time 🙏

Just to note, the empty file issue is bug 1709608.

Regressions: 1709808

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

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??

Thank you for reporting this. We've fixed it in Highway, JPEG XL will still need to update the submodule.

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.

Flags: needinfo?(mh+mozilla)
Regressions: 1710038

(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

(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.

(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!

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.

Regressions: 1711366
Flags: needinfo?(mh+mozilla)

the jxl code is not listed in toolkit/content/license.html

Flags: needinfo?(krosylight)
Flags: needinfo?(krosylight)
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

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)

You need to log in before you can comment on or make changes to this bug.