Bug 187840 - Animation stops with object-fit:contain on an animated 2d canvas
Summary: Animation stops with object-fit:contain on an animated 2d canvas
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: Safari Technology Preview
Hardware: iPhone / iPad Other
: P2 Major
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
: 188004 188007 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-07-19 18:58 PDT by andj2223
Modified: 2018-09-10 09:21 PDT (History)
9 users (show)

See Also:


Attachments
Patch (10.28 KB, patch)
2018-07-24 18:59 PDT, Simon Fraser (smfr)
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description andj2223 2018-07-19 18:58:39 PDT
http://jsfiddle.net/n7nebbL8/17/

Expected behavior upon clicking 'Run':
Diagonal line should be slowly drawn from upper-left corner to lower right corner -- one pixel at a time per frame. There should be no gaps, lag, or stoppages when the line is being drawn until it reaches the lower right corner. The performance should be quite smooth.

Actual behavior:
The line drawing stops for long periods of time (500ms or more) part way through, stops entirely, or is missing many segments, resulting in a dotted line.

I've tested this on Safari with the latest iOS 12 beta as of this writing (16A5327f) and the problem still exists.
I don't have a desktop Safari to test with, but I wouldn't be surprised to see the same issue there -- it's worth trying out.

Chrome has the same bug.

Firefox has the expected behavior, so that can be used as a guide for how this should behave.
Comment 1 Radar WebKit Bug Importer 2018-07-20 19:27:04 PDT
<rdar://problem/42454524>
Comment 2 Simon Fraser (smfr) 2018-07-24 18:04:47 PDT
This is pretty funny. Turn on repaint rects in web inspector and you can see that we fail to map the repaints through the object-fit transform.
Comment 3 Simon Fraser (smfr) 2018-07-24 18:59:06 PDT
Created attachment 345735 [details]
Patch
Comment 4 zalan 2018-07-24 20:43:03 PDT
Comment on attachment 345735 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345735&action=review

> Source/WebCore/html/HTMLCanvasElement.cpp:511
> +    if (is<RenderReplaced>(renderBox())) {

The canvas renderer is not necessarily a RenderReplaced.
Comment 5 zalan 2018-07-24 20:45:39 PDT
(In reply to zalan from comment #4)
> Comment on attachment 345735 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=345735&action=review
> 
> > Source/WebCore/html/HTMLCanvasElement.cpp:511
> > +    if (is<RenderReplaced>(renderBox())) {
> 
> The canvas renderer is not necessarily a RenderReplaced.
And the object-fit is specific to replaced elements.
Comment 7 Dawei Fenton (:realdawei) 2018-07-25 10:10:16 PDT
Reverted r234187 for reason:

the layout test added with this change is flaky on the bots

Committed r234203: <https://trac.webkit.org/changeset/234203>
Comment 8 Dawei Fenton (:realdawei) 2018-07-25 10:18:44 PDT
(In reply to David Fenton (:realdawei) from comment #7)
> Reverted r234187 for reason:
> 
> the layout test added with this change is flaky on the bots
> 
> Committed r234203: <https://trac.webkit.org/changeset/234203>

This test was 
A) Failing on High Sierra WK2 and Sierra WK 2, 
B) Flaky on High Sierra Release WK1, High Sierra (Leaks) and 
C) Failing on Windows Release.

So I made the decision to roll it out for now.

Sample Diff:
https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r234195%20(5827)/fast/repaint/canvas-object-fit-diff.txt

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/repaint/canvas-object-fit-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/repaint/canvas-object-fit-actual.txt
@@ -10,9 +10,6 @@
           (position 8.00 8.00)
           (bounds 202.00 102.00)
           (drawsContent 1)
-          (repaint rects
-            (rect 70.00 20.00 7.00 7.00)
-          )
         )
       )
     )


Flakiness Dashboard:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Frepaint%2Fcanvas-object-fit.html
Comment 9 Dawei Fenton (:realdawei) 2018-07-25 10:19:58 PDT
*** Bug 188007 has been marked as a duplicate of this bug. ***
Comment 10 Ryan Haddad 2018-07-25 12:35:18 PDT
*** Bug 188004 has been marked as a duplicate of this bug. ***
Comment 11 Simon Fraser (smfr) 2018-07-27 21:39:16 PDT
When the test flakes, there's no layer flush between the first drawing and the drawing with tracked repaints, so we've never cleared the dirty rect from the first fill, and the second draw doesn't add a new dirty rect.

In the good case we flush between the load event and the zero-delay timer.
Comment 12 Simon Fraser (smfr) 2018-07-27 22:22:39 PDT
Re-landed in https://trac.webkit.org/changeset/234343/webkit with a fixed test.
Comment 14 Truitt Savell 2018-09-10 09:21:20 PDT
Looks like fast/repaint/canvas-object-fit.html has been flakey sense being reintroduced in:
https://trac.webkit.org/changeset/234343/webkit

Test History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Frepaint%2Fcanvas-object-fit.html

Reproduced with:
run-webkit-tests --root testbuild-234343 fast/repaint/canvas-object-fit.html --iterations 500 -f -1

Diff:
--- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/fast/repaint/canvas-object-fit-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/fast/repaint/canvas-object-fit-actual.txt
@@ -1,21 +1 @@
- (GraphicsLayer
-  (anchor 0.00 0.00)
-  (bounds 800.00 600.00)
-  (children 1
-    (GraphicsLayer
-      (bounds 800.00 600.00)
-      (contentsOpaque 1)
-      (children 1
-        (GraphicsLayer
-          (position 8.00 8.00)
-          (bounds 202.00 102.00)
-          (drawsContent 1)
-          (repaint rects
-            (rect 70.00 20.00 7.00 7.00)
-          )
-        )
-      )
-    )
-  )
-)