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

VERIFIED FIXED in Firefox 50

Status

()

Core
Graphics
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: arai, Assigned: nical)

Tracking

(Blocks: 1 bug, {regression})

50 Branch
mozilla50
x86_64
Mac OS X
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50+ verified)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments)

(Reporter)

Description

a year ago
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
:nical, can you comment to the bug? Bug 1167235 seems to be related to this bug.
Flags: needinfo?(nical.bugzilla)
Hi Fujisawa-san, thanks for reporting the bug. Can you paste about:support graphics section?
Flags: needinfo?(arai.unmht)
(Reporter)

Comment 3

a year ago
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
Flags: needinfo?(arai.unmht)
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
(Reporter)

Comment 4

a year ago
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
(Assignee)

Updated

a year ago
Assignee: nobody → nical.bugzilla
Flags: needinfo?(nical.bugzilla)
(Assignee)

Comment 5

a year ago
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.
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/
Whiteboard: [gfx-noted]
Version: Trunk → 50 Branch
[Tracking Requested - why for this release]: Don't know how deep the Canvas problem goes, but it could be a lot of things affected.
tracking-firefox50: --- → ?
Tracking 50+ for this regression and potential other issues related to Canvas that are yet undiscovered.
tracking-firefox50: ? → +
(Reporter)

Updated

a year ago
See Also: → bug 1284856
(Assignee)

Updated

a year ago
Blocks: 1285271
(Assignee)

Comment 9

a year ago
Created attachment 8768824 [details] [diff] [review]
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.
Attachment #8768824 - Flags: review?(bas)
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.
(Assignee)

Comment 11

a year ago
(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.
(Assignee)

Comment 12

a year ago
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.
(Assignee)

Updated

a year ago
Keywords: leave-open
(Assignee)

Updated

a year ago
Attachment #8768824 - Flags: review?(bas) → review?(lsalzman)
(Assignee)

Comment 13

a year ago
Created attachment 8769233 [details] [diff] [review]
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.
Attachment #8769233 - Flags: review?(lsalzman)
Attachment #8768824 - Flags: review?(lsalzman) → review+
Attachment #8769233 - Flags: review?(lsalzman) → review+

Comment 14

a year ago
Pushed by nsilva@mozilla.com:
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

a year ago
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b966ababca8d
Partially backout some changes that got incidentally folded into the wrong patch. r=me
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
Flags: needinfo?(nical.bugzilla)

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fceb3f145f91
https://hg.mozilla.org/mozilla-central/rev/75f6c7e15d76

Comment 18

a year ago
Pushed by nsilva@mozilla.com:
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
(Assignee)

Updated

a year ago
Flags: needinfo?(nical.bugzilla)

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/721b7837762d
https://hg.mozilla.org/mozilla-central/rev/3a276d3d8d6b
(Assignee)

Comment 20

a year ago
This should be fixed now. Please reopen if the problem reproduces again.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
(Reporter)

Comment 21

a year ago
yes, it works smoothly :)

Updated

a year ago
Depends on: 1287707
status-firefox50: affected → fixed
Keywords: leave-open
Target Milestone: --- → mozilla50
Flags: qe-verify+

Comment 22

a year ago
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.
Thank you, Carmen!
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.