Closed Bug 1453935 Opened 7 years ago Closed 6 years ago

Make layout/reftests/bugs/{1291528.html,289480}.html pass again (repeat snapping)

Categories

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

Product:
Core
Component:
Graphics: WebRender
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: nical, Assigned: aosmond)

References

Details

(Whiteboard: gfx-noted)

Attachments

(2 files, 2 obsolete files)

Bug 1453935 - Ensure that we snap within the visible rect with WebRender.
(deleted), text/x-phabricator-request
Details
2019-02-11_13h44_34.mp4
(deleted), video/mp4
Details
A lot of this is being addressed on the webrender side with work separated into a number of pull requests: - https://github.com/servo/webrender/pull/2621 - https://github.com/servo/webrender/pull/2644 - https://github.com/servo/webrender/pull/2572 This work is affected by how we handle the pixel snapping of repeated primitives. The current plan is to simplify the problem described here: https://github.com/servo/webrender/pull/2644#issuecomment-380807766 by snapping both the primitive rect and the repeated rect. This is a fairly big change for repeated backgrounds with fractional repeated sizes.
cf. https://github.com/servo/webrender/pull/2644 There is also a bit of fuzziness to add to the ACID reftest: > REFTEST TEST-UNEXPECTED-FAIL | http://localhost:46135/1523467925296/136/289480.html#top == http://localhost:46135/1523467925296/136/289480-ref.html | image comparison, max difference: 12, number of differing pixels: 1534 but I haven't found where this is specified yet.
Comment on attachment 8967699 [details] [diff] [review] Reftests annotations for the webrender update containing PR #2644 Review of attachment 8967699 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/reftests/css-gradients/reftest.list @@ +46,5 @@ > fails-if(gtkWidget&&/x86_64-/.test(xulRuntime.XPCOMABI)) fuzzy(1,1622) fuzzy-if(cocoaWidget,2,41281) fuzzy-if(Android,8,1091) fuzzy-if(skiaContent,2,500) == radial-shape-farthest-corner-1b.html radial-shape-farthest-corner-1-ref.html > fuzzy-if(Android,17,13320) == radial-shape-farthest-side-1a.html radial-shape-farthest-side-1-ref.html > fuzzy-if(Android,17,13320) == radial-shape-farthest-side-1b.html radial-shape-farthest-side-1-ref.html > +fuzzy-if(webrender, 1, 4) == radial-size-1a.html radial-size-1-ref.html > +fuzzy-if(webrender, 1, 4) == radial-size-1b.html radial-size-1-ref.html (a) spaces are not allowed in annotations (b) please use tight ranges like 1-1,4-4
(In reply to Nicolas Silva [:nical] from comment #1) > > REFTEST TEST-UNEXPECTED-FAIL | http://localhost:46135/1523467925296/136/289480.html#top == http://localhost:46135/1523467925296/136/289480-ref.html | image comparison, max difference: 12, number of differing pixels: 1534 > > but I haven't found where this is specified yet. https://searchfox.org/mozilla-central/rev/9f3da81290054c5b8955bb67ff98cae66676f745/layout/reftests/bugs/reftest.list#308
Updated patch. I ran it locally to make sure the manifest is parsed properly.
Attachment #8967699 - Attachment is obsolete: true
Comment on attachment 8967708 [details] [diff] [review] Reftests annotations for the webrender update containing PR #2644 I'll be landing this patch as part of bug 1457241 which will contain #2644, so I'll obsolete the patch here. One thing for future reference, the change you made for 1291528.html doesn't actually work because the last clause that evaluates to true is the one that is applied, so the fuzzy(8,1900) overrides the fails-if(webrender). The webrender clause needs to be after the fuzzy to take effect. I'll make that change locally. Also, I'll repurpose this bug to track the fix that will remove the fails-if.
Attachment #8967708 - Attachment is obsolete: true
Summary: Support repeated primitives with many repetitions. → Make layout/reftests/bugs/1291528.html pass again
Summary: Make layout/reftests/bugs/1291528.html pass again → Make layout/reftests/bugs/1291528.html and layout/reftests/bugs/289480.html pass again
Expanding this bug to also cover the tests that I'm disabling in bug 1416772, since they also started failing with high frequency from servo/webrender#2644.
Summary: Make layout/reftests/bugs/1291528.html and layout/reftests/bugs/289480.html pass again → Make layout/reftests/bugs/{1291528.html,289480}.html, testing/web-platform/meta/css/CSS2/backgrounds/background-root-01{0,6}.xht pass again
Do you think this is important enough to fix for the MVP? Are we going to miss regressions with these tests disabled?
Flags: needinfo?(nical.bugzilla)
As far as getting these tests to pass, I think that we have a lot of more important items for the MVP. There's one thing that I think is worth fixing before going to release is snapping repeated sizes before repeating, to make sure we avoid some cases where a line or a column of pixels get wrapped on the wrong side with certain border styles and non-integer sizes.
Flags: needinfo?(nical.bugzilla)
(In reply to Nicolas Silva [:nical] from comment #8) > As far as getting these tests to pass, I think that we have a lot of more > important items for the MVP. > There's one thing that I think is worth fixing before going to release is > snapping repeated sizes before repeating, to make sure we avoid some cases > where a line or a column of pixels get wrapped on the wrong side with > certain border styles and non-integer sizes. Thanks, Nical. I agree with you. Updating this bug to block riding to Release, not Nightly or Beta.
Priority: P1 → P2
Summary: Make layout/reftests/bugs/{1291528.html,289480}.html, testing/web-platform/meta/css/CSS2/backgrounds/background-root-01{0,6}.xht pass again → Make layout/reftests/bugs/{1291528.html,289480}.html, testing/web-platform/meta/css/CSS2/backgrounds/background-root-01{0,6}.xht pass again (repeat snapping)
Assignee: nical.bugzilla → nobody
Priority: P2 → P3
Assignee: nobody → aosmond
At this moment, I think only layout/reftests/bug/1291528.html is failing. layout/reftests/bugs/289480.html can be re-enabled with some fuzz. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2990526f571cefcc8ed443b2864b36688d6789aa&selectedJob=220009794
Summary: Make layout/reftests/bugs/{1291528.html,289480}.html, testing/web-platform/meta/css/CSS2/backgrounds/background-root-01{0,6}.xht pass again (repeat snapping) → Make layout/reftests/bugs/{1291528.html,289480}.html pass again (repeat snapping)

Added CPU side changes to mirror what is done on the GPU side (it was also incorrect using the pic clip rect instead of the prim rect, but lining them up didn't solve my original snapping problems, so... ;)).

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=81f218b4fb36fcef6763233ee73fc27cd4c6bcf8

Oddly enough, it still fails one of the tests, despite it passing locally for me (with and without my usual 200% screen scaling).

try (linux): https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddeb1ee67e9d27e7c9d762e4fa7c0b17d8bfa66a
try (windows, osx): https://treeherder.mozilla.org/#/jobs?repo=try&revision=56f011077f56cb4f780f9800642caf3626f5f980

Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/87ad4b91e29f Ensure that we snap within the visible rect with WebRender. r=kvark
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e32dd08ec5db Ensure that we snap within the visible rect with WebRender. r=kvark
Flags: needinfo?(aosmond)

https://hg.mozilla.org/mozilla-central/rev/e32dd08ec5db

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Depends on: 1526756

Hi, This issue seems to still occur on https://www.samsung.com/ro/ in Nightly 67.0a1 (2019-02-10), unlike before this does not happen when you move the mouse cursor on top of the watches thumbnail so they come into view, but rather when you move the mouse cursor away from the thumbnail and the watches are going back to their initial state, thats when the images are shaking a bit, this issue does not occur with webrender off.

Please check the attachment for more info.

Attached video 2019-02-11_13h44_34.mp4 (deleted) —

Please note that this issue occurs on this website as well https://news.err.ee/ as stated in the last comment of Bug 1526298

Depends on: 1527829
Depends on: 1528418

Rares, can you file a separate bug for the issues you're still seeing.

Flags: needinfo?(rares.doghi)

Hi Jeff, I already added a separate issue Bug 1526298 but it's been marked as Duplicate for this bug.

Flags: needinfo?(rares.doghi)
Depends on: 1533292
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: