Closed Bug 1317967 Opened 3 years ago Closed 3 years ago

Unity WebGL stops to render when key press Arrow key

Categories

(Core :: JavaScript Engine: JIT, defect, P1)

52 Branch
x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + fixed
firefox53 - fixed

People

(Reporter: alice0775, Assigned: luke)

References

(Depends on 1 open bug)

Details

(Keywords: hang, regression)

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1317960 +++

When I test Bug 1317960,
Unity WebGL stops to render when key press Arrow key

Steps to reproduce:
1. open URL http://beta.unity3d.com/jonas/AngryBots/ 
2. Key press Arrow key to move a player

Actual Results:
Unity WebGL stops to render

Expected Results:
Not stop.

Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=add9dada238ed99b4f93c027b535423f067d3781&tochange=1196bf3032e1bce1fb07a01fd9082a767426c5fb
Keywords: hang, regression
Summary: Unity WebGL stops to render when key press Arrow] key → Unity WebGL stops to render when key press Arrow key
[Tracking Requested - why for this release]:

Regression window(m-i):
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9d1b72a21a395e36cbfd9982243054e64a955bb3&tochange=eafea39ebf3977beefda0a7cdcef8c8266060ca0

Regressed by: eafea39ebf39	Luke Wagner — Bug 1313180 - Baldr: fix accidental disabling of asm.js when wasm is disabled (r=sunfish)
Blocks: 1313180
Component: Untriaged → JavaScript Engine: JIT
Flags: needinfo?(luke)
Product: Firefox → Core
[Tracking Requested - why for this release]:
(can't repro under latest nightly 64 bits, linux x64)
See Also: → 1317986
Attached file about_suppot
To easy reproduce the problem, 
You may need to play on a low spec PC. 
you need to key down and hold Arrow key immediately after start game(within 0-2 seconds as soon as possible).

But, It seems to depend on spec of CPU/GPU.

Steps to reproduce:
1. open URL http://beta.unity3d.com/jonas/AngryBots/ 
2. Key down and hold Arrow key to move a player immediately after start game(within 0-2 seconds as soon as possible)

Actual Results:
Unity WebGL stops to render.

Expected Results:
Not stop.
(In reply to Alice0775 White from comment #6)
Thanks for taking the time to bisect!  Since this is performance sensitive, I expect the reason your bisection stopped on bug 1313180 is because it re-enabled asm.js for default profiles (when javascript.options.wasm=false) which significantly changes performance.  Could you try bisecting further with javascript.options.wasm=true?
Flags: needinfo?(luke)
(In reply to Luke Wagner [:luke] from comment #7)
> (In reply to Alice0775 White from comment #6)
> Thanks for taking the time to bisect!  Since this is performance sensitive,
> I expect the reason your bisection stopped on bug 1313180 is because it
> re-enabled asm.js for default profiles (when javascript.options.wasm=false)
> which significantly changes performance.  Could you try bisecting further
> with javascript.options.wasm=true?

Regression window(with javascript.options.wasm=true):
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c69a0c17a17d4948c66a1e3dae73a959836be628&tochange=6dae0685c9d21b30fbe39ebb31254c733fd43900

Triggered by: 6dae0685c9d2	Luke Wagner — Bug 1313180 - Baldr: only allocate 4 bytes for float32 on the stack (r=bbouvier)
I cannot reproduce on Nightly53.0a1 x64.
So, this problem seems to be affected to 32bit build only.
Ah, this bug does seem to be a likely culprit; I'll take a look, thanks.

By any chance, does it reproduce on Linux 32-bit?
(In reply to Luke Wagner [:luke] from comment #10)
> Ah, this bug does seem to be a likely culprit; I'll take a look, thanks.
> 
> By any chance, does it reproduce on Linux 32-bit?


I can reproduce the problem on Nightly53.0a1 32bit Ubuntu16.04 32bit.

And It works without problem on Nightly53.0a1 64bit Ubuntu16.04 64bit.
Yay, thanks!
Attached patch fix-float32Splinter Review
I found one potential culprit: the codegen of WasmStackArg was still using a storeDouble.  Because arguments are stored in order this mostly wasn't a problem unless the very last argument is a float and this clobbers a value in the spill space.  I was able to reproduce the problem with the attached test case and confirm the fix.  Note: since ARM32 puts the single/double in the ABIArg, the ma_vstr() in ARM32's codegen does the right thing already.

I still need to do a 32-bit browser build to confirm the problem and fix.
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8811440 - Flags: review?(bbouvier)
Version: 53 Branch → 52 Branch
Priority: -- → P1
Comment on attachment 8811440 [details] [diff] [review]
fix-float32

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

Great catch!

::: js/src/jit-test/tests/wasm/wasm-abi.js
@@ +35,5 @@
> +                   getLocals +
> +                   adds +
> +                   `    )\n` +
> +                   `)`;
> +        assertEq(wasmEvalText(code).exports.run(100), numLocals * 100 + sum);

Mind to make this a wasmFullPass test, please?
Attachment #8811440 - Flags: review?(bbouvier) → review+
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/234ca79c0f2e
Baldr: Fix x86 float32 stack argument passing (r=bbouvier)
Comment on attachment 8811440 [details] [diff] [review]
fix-float32

Approval Request Comment
[Feature/regressing bug #]: 1313180
[User impact if declined]: totally wrong behavior in large wasm apps
[Describe test coverage new/current, TreeHerder]: tests added in m-c
[Risks and why]: low-risk: small change, with test
[String/UUID change made/needed]: none
Attachment #8811440 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/234ca79c0f2e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Tracking 53- as this is now fixed.
Alice0775 White, can you verify the fix in nightly?  Thanks.
Flags: needinfo?(alice0775)
Comment on attachment 8811440 [details] [diff] [review]
fix-float32

fix float argument passing on 32bit wasm, take in aurora52
Attachment #8811440 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced the problem on 2016-Nov17 build[1].
Verified to fix the problem on 2016-Nov-20 build[2].

[1]https://hg.mozilla.org/mozilla-central/rev/13f49da109ea460665ad27c8497cb1489548450c
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 ID:20161117030212

[2]https://hg.mozilla.org/mozilla-central/rev/57a8cde3f08ca9d60bcd8bdd698ceec687f0aed2
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 ID:20161120030205
Flags: needinfo?(alice0775)
You need to log in before you can comment on or make changes to this bug.