1726388 - Line artifact present on background gradient with transform:skew

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[rubenwardy]

Attachment: Screenshot_2021-08-18_16-45-58.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Firefox/90.0

Steps to reproduce:

Open the attached gradient_glitch_mve.html file in your browser. It contains a single element, styled with a background element and a transform: skew property.

Reproduced on Firefox 90 and 91 on Linux and Windows.

Jörg Sommer on matrix used mozreg and found this faulty changeset (21749038, f733b37f) https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2174903818ae542d463979cd70134789e4ffdc80&tochange=f733b37f9626548ae83564a3fd43ddfa972977d9

Actual results:

A line appears running though the gradient

Expected results:

There should be no line

Comment 1

[rubenwardy]

Attachment: gradient_glitch_mve.html

Minimal reproducible example

Comment 2

[rubenwardy]

One interesting thing to note is that the line appears to appear where the gradient percentage is set to. If you set the percentage to 0%, the line no longer appears

Comment 3

[rubenwardy]

To clarify, I mean like so:

background: linear-gradient(#007DB8 0%, #093167);

Comment 4

[Marco Castelluccio [:marco]]

Set release status flags based on info from the regressing bug 1702228

Comment 5

[Nicolas Silva [:nical]]

This one's going to have to wait for a bit longer as I'll be away for a couple of weeks, but here is the analysis:

• I think the problem is this loop: https://searchfox.org/mozilla-central/rev/d34f9713ae128a3138c2b70d8041a535f1049d19/gfx/wr/webrender/src/prim_store/gradient/linear.rs#253
• We decompose some gradients into simpler ones but float precision can cause the gradient segments to not be exactly at the same spot and the non-axis aligned transform makes it more likely to see the crack between the segments.

The fix will be likely something along the lines of using the segment end of the previous iteration as the segment start of the following iteration instead of recomputing them in a way that can round off differently.

Comment 6

[Nicolas Silva [:nical]]

Actually the problem is caused by antialiasing between the gradient segments.

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 1726388 - Make it possible to not anti-alias at the bounds of the primitive. r=#gfx-reviewers

The Antialiasing code sets up the bounds of a rect to anti-alias in the vertex shader for the fragment shader to compute the AA via a distance approximation.
The modified mix call selects between the edges of the current instance (could be a primitive segment) and the edges of the primitive based on the anti-aliasing edge flags. If an edge does not have the AA flag, then the aa will be computed based on the distance to the primitive's edge. This usually works because we tend to want AA around the primitive but not along the internal edges of potential segments of a primitive. However it also means that if we take the anti-aliasing code path (for example under a non-axis-aligned transform) then the edges of the primitive are always anti-aliased. In some cases we want to disable the anti-aliasing along the primtive, though, for example when something is split into multiple primitive like the linear gradient optimization.

This patch changes this mix call so that it selects between the segment bounds and some very large rect. If the primitive is not segmented, then the segment bounds are the bounds of the primitive. The effect is that AA is only applied to edges that have the flag set rather than segment edges with the flag set as well as any edge at the border of the primitive.

Comment 8

[Nicolas Silva [:nical]]

Attachment: Bug 1726388 - Allow primitive to mask out the antialiasing of their edges. r=#gfx-reviewers

This patch adds a edge aa mask field in the common primitive data so that primitives can turn off anti-aliasing at their edges. There was some padding in the structure so it didn't change the size.
This will be use by a followup patch to turn off anti-aliasing between pieces of linear gradients when they are decomposed into simpler primitives.

Depends on D145935

Comment 9

[Nicolas Silva [:nical]]

Attachment: Bug 1726388 - Disable AA between parts of a decomposed linear gradient. r=#gfx-reviewers

Depends on D145936

Comment 10

[Nicolas Silva [:nical]]

Attachment: Bug 1726388 - Adjust reftest expectations. r=#gfx-reviewers

This patch series gave picture primitives the full anti-aliasing instead of just the inward half, some reftest references have to be changed accordingly.

Depends on D145937

Comment 11

[Nicolas Silva [:nical]]

Attachment: Bug 1726388 - Add a test. r=#gfx-reviewers

Depends on D146199

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/622db270fc75
Make it possible to not anti-alias at the bounds of the primitive. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/a4db203ad05b
Allow primitive to mask out the antialiasing of their edges. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/ad98b0908fcc
Disable AA between parts of a decomposed linear gradient. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/e00338ba954b
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/053f952f253b
Add a test. r=gfx-reviewers,lsalzman

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1ce411a0689
Backed out 5 changesets for causing wrench bustages on linear-transformed.yaml CLOSED TREE

Comment 14

[bszekely]

Backed out for causing wrench bustages on linear-transformed.yaml

Push with failures

Failure log for REFTEST TEST-UNEXPECTED-FAIL | reftests/gradient/linear-transformed.yaml == reftests/gradient/linear-transformed.png | image comparison, max difference: 220, number of differing pixels: 183054 | 183054 num_differences > 0 and <= 255 (allowed 0);

Failure log for java.lang.SecurityException: Package org.mozilla.wrench has not requested permission android.permission.WRITE_EXTERNAL_STORAGE

Backout link

Comment 15

[Nicolas Silva [:nical]]

Hm, odd I was sure I had regenerated the reference image from a try run before landing.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d42c0e759ba
Make it possible to not anti-alias at the bounds of the primitive. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/23f46cf380e8
Allow primitive to mask out the antialiasing of their edges. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/db71d8cb9f63
Disable AA between parts of a decomposed linear gradient. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/0b40f0a0fd75
Adjust reftest expectations. r=gfx-reviewers,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/aadd7a2b9e43
Add a test. r=gfx-reviewers,lsalzman

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for wrench failures on reftest.rs

• backout: https://hg.mozilla.org/integration/autoland/rev/46fce4bc8cd488edfcf904febadba61be749708f
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=I-XDbn5pTY6CgH_FSckbFw.0&revision=aadd7a2b9e437a3208d7acaca38541a726749dcb
• failure log: https://treeherder.mozilla.org/logviewer?job_id=378232247&repo=autoland&lineNumber=40109

[task 2022-05-16T16:19:30.059Z] REFTEST reftests/filters/blend-clipped.yaml == reftests/filters/blend-clipped.png
[task 2022-05-16T16:19:30.059Z] thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Decoding(DecodingError { format: Exact(Png), message: Some("unexpected EOF"), underlying: None })', wrench/src/reftest.rs:936:55
[task 2022-05-16T16:19:30.059Z] stack backtrace:
[task 2022-05-16T16:19:30.076Z]    0:     0x55a38f156eed - std::backtrace_rs::backtrace::libunwind::trace::hee598835bc88d35b
[task 2022-05-16T16:19:30.076Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
[task 2022-05-16T16:19:30.076Z]    1:     0x55a38f156eed - std::backtrace_rs::backtrace::trace_unsynchronized::h9cdc730ba5cf5d72
[task 2022-05-16T16:19:30.076Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
[task 2022-05-16T16:19:30.076Z]    2:     0x55a38f156eed - std::sys_common::backtrace::_print_fmt::h75aeaf7ed30e43fa
[task 2022-05-16T16:19:30.076Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:66:5
[task 2022-05-16T16:19:30.076Z]    3:     0x55a38f156eed - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h606862f787600875
[task 2022-05-16T16:19:30.076Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:45:22
[task 2022-05-16T16:19:30.078Z]    4:     0x55a38f17b7ec - core::fmt::write::he803f0f418caf762
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/fmt/mod.rs:1190:17
[task 2022-05-16T16:19:30.078Z]    5:     0x55a38f152198 - std::io::Write::write_fmt::h70bc45872f37e7bb
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/io/mod.rs:1657:15
[task 2022-05-16T16:19:30.078Z]    6:     0x55a38f158ee7 - std::sys_common::backtrace::_print::h64d038cf8ac3e13e
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:48:5
[task 2022-05-16T16:19:30.078Z]    7:     0x55a38f158ee7 - std::sys_common::backtrace::print::h359300b4a7fccf65
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:35:9
[task 2022-05-16T16:19:30.078Z]    8:     0x55a38f158ee7 - std::panicking::default_hook::{{closure}}::hf51be35e2f510149
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:295:22
[task 2022-05-16T16:19:30.078Z]    9:     0x55a38f158bb0 - std::panicking::default_hook::h03ca0f22e1d2d25e
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:314:9
[task 2022-05-16T16:19:30.078Z]   10:     0x55a38f159639 - std::panicking::rust_panic_with_hook::h3b7380e99b825b63
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:698:17
[task 2022-05-16T16:19:30.078Z]   11:     0x55a38f159327 - std::panicking::begin_panic_handler::{{closure}}::h8e849d0710154ce0
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:588:13
[task 2022-05-16T16:19:30.078Z]   12:     0x55a38f1573b4 - std::sys_common::backtrace::__rust_end_short_backtrace::hedcdaddbd4c46cc5
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:138:18
[task 2022-05-16T16:19:30.078Z]   13:     0x55a38f159039 - rust_begin_unwind
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:584:5
[task 2022-05-16T16:19:30.078Z]   14:     0x55a38e9ed763 - core::panicking::panic_fmt::he1bbc7336d49a357
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/panicking.rs:143:14
[task 2022-05-16T16:19:30.078Z]   15:     0x55a38e9ed853 - core::result::unwrap_failed::h719f957ed74f93fd
[task 2022-05-16T16:19:30.078Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/result.rs:1749:5
[task 2022-05-16T16:19:30.080Z]   16:     0x55a38ea795e7 - wrench::reftest::ReftestHarness::load_image::hf78cd0d99194e7bb
[task 2022-05-16T16:19:30.080Z]   17:     0x55a38ea795e7 - wrench::reftest::ReftestHarness::run_reftest::he5da1c11b4789d0c
[task 2022-05-16T16:19:30.080Z]                                at /builds/worker/checkouts/gecko/gfx/wr/wrench/src/reftest.rs:767:27
[task 2022-05-16T16:19:30.080Z]   18:     0x55a38ea795e7 - wrench::reftest::ReftestHarness::run::hc5d8dbf07b912719
[task 2022-05-16T16:19:30.080Z]                                at /builds/worker/checkouts/gecko/gfx/wr/wrench/src/reftest.rs:708:16
[task 2022-05-16T16:19:30.083Z]   19:     0x55a38ea3f731 - wrench::reftest::hf7880374173ff051
[task 2022-05-16T16:19:30.083Z]                                at /builds/worker/checkouts/gecko/gfx/wr/wrench/src/main.rs:509:24
[task 2022-05-16T16:19:30.083Z]   20:     0x55a38ea44996 - wrench::main::hf2ec65b9304c17c8
[task 2022-05-16T16:19:30.084Z]                                at /builds/worker/checkouts/gecko/gfx/wr/wrench/src/main.rs:769:23
[task 2022-05-16T16:19:30.085Z]   21:     0x55a38ea9a453 - core::ops::function::FnOnce::call_once::hcf88501a774cb483
[task 2022-05-16T16:19:30.085Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:227:5
[task 2022-05-16T16:19:30.085Z]   22:     0x55a38ea9a453 - std::sys_common::backtrace::__rust_begin_short_backtrace::hefd449b5a7c74575
[task 2022-05-16T16:19:30.085Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/sys_common/backtrace.rs:122:18
[task 2022-05-16T16:19:30.087Z]   23:     0x55a38ea28f69 - std::rt::lang_start::{{closure}}::hcb4ac4056ce01d62
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/rt.rs:145:18
[task 2022-05-16T16:19:30.087Z]   24:     0x55a38f1565d1 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hb7014f43484a8b4e
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/core/src/ops/function.rs:259:13
[task 2022-05-16T16:19:30.087Z]   25:     0x55a38f1565d1 - std::panicking::try::do_call::h7bc9dc436daeb8c7
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:492:40
[task 2022-05-16T16:19:30.087Z]   26:     0x55a38f1565d1 - std::panicking::try::h653d68a27ff5f175
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:456:19
[task 2022-05-16T16:19:30.087Z]   27:     0x55a38f1565d1 - std::panic::catch_unwind::h9d739f9f59895e68
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panic.rs:137:14
[task 2022-05-16T16:19:30.087Z]   28:     0x55a38f1565d1 - std::rt::lang_start_internal::{{closure}}::hf006f2bc7ce22bbe
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/rt.rs:128:48
[task 2022-05-16T16:19:30.087Z]   29:     0x55a38f1565d1 - std::panicking::try::do_call::hfb39d6df61a2e69f
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:492:40
[task 2022-05-16T16:19:30.087Z]   30:     0x55a38f1565d1 - std::panicking::try::h13e2d225134958ac
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panicking.rs:456:19
[task 2022-05-16T16:19:30.087Z]   31:     0x55a38f1565d1 - std::panic::catch_unwind::h3bd49b5a5dfb1a50
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/panic.rs:137:14
[task 2022-05-16T16:19:30.087Z]   32:     0x55a38f1565d1 - std::rt::lang_start_internal::h2ba92edce36c035e
[task 2022-05-16T16:19:30.087Z]                                at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/std/src/rt.rs:128:20
[task 2022-05-16T16:19:30.087Z]   33:     0x55a38ea473a8 - main
[task 2022-05-16T16:19:30.088Z]   34:     0x7f32f109209b - __libc_start_main
[task 2022-05-16T16:19:30.088Z]   35:     0x55a38e9edf9a - _start
[task 2022-05-16T16:19:30.088Z]   36:                0x0 - <unknown>
[task 2022-05-16T16:19:30.099Z] ../target/release/build/osmesa-src-fa18da3eaa788eb8/out/mesa/src/gallium/targets/osmesa
[task 2022-05-16T16:19:30.099Z] Running: `../target/release/wrench --no-scissor --headless reftest`
[task 2022-05-16T16:19:30.099Z] Traceback (most recent call last):
[task 2022-05-16T16:19:30.099Z]   File "script/headless.py", line 152, in <module>
[task 2022-05-16T16:19:30.099Z]     subprocess.check_call(cmd, stderr=subprocess.STDOUT)
[task 2022-05-16T16:19:30.099Z]   File "/usr/lib/python2.7/subprocess.py", line 190, in check_call
[task 2022-05-16T16:19:30.099Z]     raise CalledProcessError(retcode, cmd)
[task 2022-05-16T16:19:30.099Z] subprocess.CalledProcessError: Command '['../target/release/wrench', '--no-scissor', '--headless', 'reftest']' returned non-zero exit status -6
[taskcluster 2022-05-16 16:19:30.696Z] === Task Finished ===
[taskcluster 2022-05-16 16:19:30.696Z] Unsuccessful task run with exit code: 1 completed in 459.197 seconds

Comment 18

[Nicolas Silva [:nical]]

https://treeherder.mozilla.org/jobs?repo=try&revision=fcd9c4ab17aceb74644e1708510cf8dc57ca5106

Looks like HG or something in the CI had a cold, just pushing the same revs to try doesn't show the corrupted image issue. However the test I added renders slightly differently in Linux+Opt and Windows+Opt compared to other configurations, including the debug configuration of the same platforms.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8de9ab7c1bb5
Make it possible to not anti-alias at the bounds of the primitive. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/e6ff50f388c3
Allow primitive to mask out the antialiasing of their edges. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/5aabf7271c8b
Disable AA between parts of a decomposed linear gradient. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/7ec7ac35e55b
Adjust reftest expectations. r=gfx-reviewers,jrmuizel

Comment 20

[Nicolas Silva [:nical]]

Relanded without the fuzzy test.

Comment 21

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8de9ab7c1bb5
https://hg.mozilla.org/mozilla-central/rev/e6ff50f388c3
https://hg.mozilla.org/mozilla-central/rev/5aabf7271c8b
https://hg.mozilla.org/mozilla-central/rev/7ec7ac35e55b