1284721 - Graphics is not updated correctly in http://slither.io/

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Tooru Fujisawa [:arai]]

Steps to reproduce:
  1. Open http://slither.io/
  2. Click "Play" button

Actual result:
  either following, depending on timing or something:
    A. entire frame rate drops down to unplayable level, like 1-2 FPS
    B. player characters (worms) are blinking (rendered only on some frames)

Expected result:
  player characters are rendered every frame, and frame rate keeps playable level

Regression range:
  https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=57cf7cae92f1b234b763240a33f030fd68fc2166&tochange=56715a33b016d05afc2541e19fd7e0c907258971

Comment 1

[Sotaro Ikeda [:sotaro]]

:nical, can you comment to the bug? Bug 1167235 seems to be related to this bug.

Comment 2

[Sotaro Ikeda [:sotaro]]

Hi Fujisawa-san, thanks for reporting the bug. Can you paste about:support graphics section?

Comment 3

[Tooru Fujisawa [:arai]]

Here's about:support data.
I'm on iMac Late 2013 with OS X 10.11.4.
will check on other machines.

Graphics
--------

Features
Compositing: OpenGL
Asynchronous Pan/Zoom: none
WebGL Renderer: NVIDIA Corporation -- NVIDIA GeForce GT 750M OpenGL Engine
Hardware H264 Decoding: Yes
GPU #1
Active: Yes
Vendor ID: 0x10de
Device ID: 0x0fe9

Diagnostics
AzureCanvasAccelerated: 1
AzureCanvasBackend: skia
AzureContentBackend: skia
AzureFallbackCanvasBackend: none

Comment 4

[Tooru Fujisawa [:arai]]

So far, here's some investigation:

  1. The issue disappears when I change "gfx.canvas.azure.backends" pref from "skia" to "cg" (not sure if this pref is correct tho)

  2. The issue changes when I change "layers.acceleration.disabled" pref from false to true
     With the pref, entire web page blinks


Also, similar issue happens on debian 64bit.
entire web page starts blinking with same regression range.
but this time changing "gfx.canvas.azure.backends" pref from "skia" to "cairo" doesn't fix the issue.

Graphics
--------

Features
Compositing: Basic
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled
WebGL Renderer: Intel Open Source Technology Center -- Mesa DRI Intel(R) Ironlake Desktop
Hardware H264 Decoding: No
GPU #1
Active: Yes
Description: Intel Open Source Technology Center -- Mesa DRI Intel(R) Ironlake Desktop
Vendor ID: Intel Open Source Technology Center
Device ID: Mesa DRI Intel(R) Ironlake Desktop
Driver Version: 2.1 Mesa 10.3.2

Diagnostics
AzureCanvasAccelerated: 0
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: none
CairoUseXRender: 0
Decision Log
HW_COMPOSITING:
blocked by default: Acceleration blocked by platform
OPENGL_COMPOSITING:
unavailable by default: Hardware compositing is disabled

Comment 5

[Nicolas Silva [:nical]]

Apparently the canvas work from bug 1167235 is interacting poorly with skia's gl backend used on mac (I can reproduce it when enabling skia-gl on linux). I'll disable the shared BufferProvider when skia-gl is enabled to have a quick fix and get some more time to investigate the issue.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

If we're not backing this out, then let's keep working to understand what's going on, even after the quick fix.  We don't want to leave it as "disabled shared buffer provider, no clue why we had to" state, and do it in 50.

'Cause the time has stopped with this regression: http://randomibis.com/coolclock/

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

[Tracking Requested - why for this release]: Don't know how deep the Canvas problem goes, but it could be a lot of things affected.

Comment 8

[Marcia Knous [:marcia]]

Tracking 50+ for this regression and potential other issues related to Canvas that are yet undiscovered.

Comment 9

[Nicolas Silva [:nical]]

Attachment: Preff off copy-on-write canvas (on all platforms) for now.

This works around the issue by disabling the "optimization" that's causing the skiagl regression. Since there are other regressions with the same cause on other platforms I decided to turn it off on all platforms and take the time to figure these bugs out one by one.

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8768824 [details] [diff] [review]
Preff off copy-on-write canvas (on all platforms) for now.

Review of attachment 8768824 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think this patch actually fixes the (comment 6) problem for me, but I like being able to put this behind the pref, and I like testing for failure even when the pref is on.  In other words, I like the patch, but it doesn't seem to be fixing the problem for me.

Comment 11

[Nicolas Silva [:nical]]

(In reply to Milan Sreckovic [:milan] from comment #10)
> I don't think this patch actually fixes the (comment 6) problem for me

Hm, Odd. It does fix the skia-gl issues I am able to reproduce on linux (including slither.io, ie fishtank, coolclocks and whatnot). Another bug is preventing me from enabling accelerated layers though, so I am not testing in the exact same configuration which would explain it (most likely the forced read-back from the skia-gl canvas is hiding some other synchronization issues). I'll hack my way around that and do some more testing.

Comment 12

[Nicolas Silva [:nical]]

Ok, I can now reproduce some more badness happening with skiagl + gl cmpositing. The preblem goes away if I call Flush on the canvas's DrawTarget before sending canvas updates to the compositor, so a bit of synchronization has been lost somewhere. Should not be too hard to track down.

Comment 13

[Nicolas Silva [:nical]]

Attachment: Flush the DrawTarget before giving it back to the BufferProvider

Thanks to Lee's pointers during the gfx meeting, I found where I messed up the sync. we used to flush in the PreTransactionCallbak which is called, well, just before the next transaction if any. Returning the DrawTarget was moved to the StableState callback which happens a bit earlier and clears the DrawTarget which meant we would not flush later in the PreTransaction callback.

With the other patch we always have a buffer provider (a basic one in the case of skiagl), which means we can move the flush to PersistentBufferProviderBasic::ReturnTarget. The BufferProvuderShared version will always flush when unlocking the TextureClient so there is no need to flush another time.

It's probable that we don't need the PreTransaction callback anymore now, but I'll look into that later. the whole canvas/layers glue is screaming for a cleanup.

With this patch I verified that coolclocks are cool again and slither is also fixed.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fceb3f145f91
Pref off PersistentBufferProviderShared due to various regressions. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/75f6c7e15d76
Flush CanvasRenderingContext2D's DrawTarget before handing it off to the BufferProvider. r=lsalzman

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b966ababca8d
Partially backout some changes that got incidentally folded into the wrong patch. r=me

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

All backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/84b0f9eb63d0 for bustage:

https://treeherder.mozilla.org/logviewer.html#?job_id=31451108&repo=mozilla-inbound

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/fceb3f145f91
https://hg.mozilla.org/mozilla-central/rev/75f6c7e15d76

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/721b7837762d
Pref off PersistentBufferProviderShared due to various regressions. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a276d3d8d6b
Flush CanvasRenderingContext2D's DrawTarget when returning it to the BufferProvider. r=lsalzman

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/721b7837762d
https://hg.mozilla.org/mozilla-central/rev/3a276d3d8d6b

Comment 20

[Nicolas Silva [:nical]]

This should be fixed now. Please reopen if the problem reproduces again.

Comment 21

[Tooru Fujisawa [:arai]]

yes, it works smoothly :)

Comment 22

[Carmen]

I have managed to reproduce the issue using the affected build Nightly 50.0a1 (2016-07-05) on Mac OS Yosemite 10.10.4

The fix is verified on Beta 50.0b3 Build 1, using  Mac OS Yosemite 10.10.4.

Comment 23

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you, Carmen!