1880996 - [gpu-canvas] Low performance and screen flashes while rendering a puzzle game

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Found in

• Firefox 124.0b1

Affected versions

• Firefox 124.0b1
• Firefox 125.0a1

Tested platforms

• Affected platforms: macOS 13.6.4 Intel / 14.3.1 ARM, Windows 11, Ubuntu 23.10

Steps to reproduce

1. Access this puzzle game

Expected result

• The performance rendering of the game is good

Actual result

• Performance goes down to 10-12 fps and the screen randomly flashes

Regression range

• Regression was introduced in Firefox 112.0a1 (2023-03-08).
• Here's the Pushlog
  Mozregression pinpoints to:
  Bug 1820988 - Rename ProcessOneChar to ProcessSimpleRun, and allow it to also handle purely-LTR strings. r=emilio

This is the significant change here: extend the single-character fast-path that skips
bidi processing to also handle strings that are confirmed to have no bidi content.
In such cases, we don't need to pass the text to a Bidi engine for analysis at all.

Depends on D171988

Additional notes

• The issue can be seen in the following attachment
• In comparison, Chrome has a constant 60-120 fps (depending on the device's screen frequency)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:csasca, could you try to find a regression range using for example mozregression?

Comment 2

[Mayank Bansal]

Can also repro on Windows with gpu-canvas enabled.

gpu-canvas: https://share.firefox.dev/3OOyu80
d2d-canvas: https://share.firefox.dev/3I7tbwu

Edit: I got the same regression range for Windows.
Bug 1820988 - Rename ProcessOneChar to ProcessSimpleRun, and allow it to also handle purely-LTR strings. r=emilio
Differential Revision: https://phabricator.services.mozilla.com/D171989

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1820988

Comment 4

[Jonathan Kew [:jfkthame]]

This is surprising; the regressing bug was aiming to let us use a simpler codepath in more cases, so it ought to be an improvement, not a regression. Somehow in this case, the "simplified" codepath seems to be ending up a lot more expensive. But I can confirm I see the poor performance, just as reported; something isn't behaving as expected here.

Comment 5

[Jonathan Kew [:jfkthame]]

Weird: this reproduces for me in both release Firefox and Nightly, across any platform; but in my local build on Windows (where I was going to try and do some debugging), it doesn't happen; I get a nice steady 60fps, and no excessive CPU use. Even a DEBUG, non-opt build still gets 60fps. Guess I'll try investigating on a different platform....

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1880996 - Check for zero-length text in nsBidiPresUtils::ProcessSimpleRun. r=#layout-reviewers

In the example here, failing to check for an empty string was resulting
in lots of extra work to set up for drawing a shadow, etc., even though
nothing ends up being rendered. Just bail out early if there's no text.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad3e0a193f66
Check for zero-length text in nsBidiPresUtils::ProcessSimpleRun. r=dholbert

Comment 8

[Serban Stanca [:SerbanS]]

Backed out for causing canvas failures in 2d.text.measure.width.empty.html.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /html/canvas/element/text/2d.text.measure.width.empty.html | The empty string has zero width - assert_equals: ctx.measureText("").width === 0 (got -23860930[number], expected 0[number]) expected 0 but got -23860930

L.E. There are also these reftests failures.

• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/text-overflow/marker-string.html == layout/reftests/text-overflow/marker-string-ref.html | image comparison, max difference: 255, number of differing pixels: 26

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f571738470f9
Check for zero-length text in nsBidiPresUtils::ProcessSimpleRun. r=dholbert

Comment 10

[Cristian Tuns]

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

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox124 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1880996 - Check for zero-length text in nsBidiPresUtils::ProcessSimpleRun.

In the example here, failing to check for an empty string was resulting
in lots of extra work to set up for drawing a shadow, etc., even though
nothing ends up being rendered. Just bail out early if there's no text.

Original Revision: https://phabricator.services.mozilla.com/D202630

Comment 13

[Phabricator Automation]

Uplift Approval Request

• String changes made/needed: none
• Is Android affected?: yes
• Needs manual QE test: no
• Code covered by automated testing: yes
• Steps to reproduce for manual QE testing: n/a
• Fix verified in Nightly: yes
• Risk associated with taking this patch: minimal
• Explanation of risk level: just adds an early-return in the case of empty string
• User impact if declined: Potential for bad perf in canvas as a result of fillText("")

Comment 14

[Jonathan Kew [:jfkthame]]

Although it's a fairly obscure edge case, the perf impact is large enough and the patch trivial enough that I think we should consider uplift here.

Comment 15

[Mayank Bansal]

This is fixed on Nightly : https://share.firefox.dev/49K8P8r

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/b308fc8aa944

Comment 18

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Attachment: 20240301_125203(1)(1).mp4

Hello! We verified this today using steps from comment 0 and here are the results:

macOS 12 Intel:

• Firefox 123.0 (affected): 11 FPS and no flashes
• Firefox 125.0a1 (2024-02-28) and Firefox 124.0b5: ~60 fps with intermittent flashes.
  Note that flashes are displayed and highly intermittent sometimes but they seem to occur when leaving Firefox open with the game loaded for ~1/2 minutes

Windows 11 VM:

• affected build: 11 fps with no flashes
• Firefox 125.0a1 (2024-02-28) and Firefox 124.0b5: ~45-50 fps with constant flashes
  Note: on first load and refresh there is a stutter

Windows 11 station:

• affected build: ~50 fps with no flashes
• Firefox 125.0a1 (2024-02-28) and Firefox 124.0b5: ~60 fps with constant flashes
  Note: on first load and refresh there is a stutter

Windows 10x64 station:

• affected 125.0a1 (2023-02-20): ~45-50 fps with no flashes
• Firefox 125.0a1 (2024-02-28) and Firefox 124.0b5: ~60 fps with no flashes
  Note: on first load or refresh there is a stutter

Ubuntu 23.10:

• affected: ~10-20 fps with multiple flashes
• Firefox 125.0a1 (2024-02-29): ~60 fps with flashes (way less frequent than on 124.0b5)
• Firefox 124.0b5: ~60 fps with multiple flashes > Sometimes loading the puzzle page will only show flashes intermittently, but sometimes the flashes are displayed continuously. See the attached screen recording.

macOS 14.3.1 ARM:

• affected 125.0a: ~10-12 fps with flashes
• Firefox 125.0a1 (2024-02-29) and Firefox 124.0b5: ~120 fps with no flashes

We've additionally conducted smoke tests by running random critical tests from our graphics suite on macOS 12, Ubuntu 23, and Windows 10x64.

From the above results, It seems that the fps issue is fixed but there is still a page flash issue that can be seen sometimes on macOS 12, Windows 11 VM, and especially Ubuntu 23.10 with the link from comment 0.

Should we file another issue for the flashes? They are sometimes pretty visible, especially on my Ubuntu 23.10.
Furthermore, I'm uncertain whether it's expected for certain machines to encounter FPS drop when loading or refreshing the page. Please let me know if we should file an issue for this as well.

If more information is needed please let us know! Thank you!

Comment 19

[Jonathan Kew [:jfkthame]]

Huh, that's interesting. Could you please compare with Firefox 111 (i.e. a version from before bug 1820988 landed), and see if the flashing is present there? Also, please try getting a profile with one of the versions where flashes are happening; that should help us see whether it's a related issue or something different. Thanks!

Comment 20

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to Jonathan Kew [:jfkthame] from comment #19)

Huh, that's interesting. Could you please compare with Firefox 111 (i.e. a version from before bug 1820988 landed), and see if the flashing is present there? Also, please try getting a profile with one of the versions where flashes are happening; that should help us see whether it's a related issue or something different. Thanks!

I can't seem to reproduce the issue with Firefox 111 on macOS 12 or Ubuntu 23.10 where I see the flashing with the fixed builds. I have registered two profiles for each station one with Firefox settings and one with graphics settings:
Ubuntu 23.10: Graphics / Firefox
macOS 12: Graphics / Firefox

I have also uploaded a screen recording here from macOS 12. If more information is needed please let us know. Thank you!