Bug 227226 - BabylonJS Under water demo is slower than it should be on Intel
Summary: BabylonJS Under water demo is slower than it should be on Intel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Local Build
Hardware: Mac (Intel) macOS 10.15
: P2 Normal
Assignee: Kyle Piddington
URL: https://playground.babylonjs.com/#LPTLZM
Keywords: InRadar
: 226841 (view as bug list)
Depends on:
Blocks: webgl2perfproblem 227596
  Show dependency treegraph
 
Reported: 2021-06-21 11:05 PDT by Kimmo Kinnunen
Modified: 2021-07-07 14:10 PDT (History)
9 users (show)

See Also:


Attachments
Patch (3.04 KB, patch)
2021-06-29 16:22 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch (2.36 KB, patch)
2021-06-29 18:00 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff
Patch for landing (2.40 KB, patch)
2021-07-01 09:28 PDT, Kyle Piddington
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2021-06-21 11:05:20 PDT
BabylonJS Under water demo is slower than it should be on Intel
https://playground.babylonjs.com/#LPTLZM

MacBookPro19,1 Intel / AMD 

WebGL on Metal on 10fps
WebGL on Metal off 30fps

Should be 60fps.
Comment 1 Kimmo Kinnunen 2021-06-21 11:07:18 PDT
Related to rdar://79216506 but not strictly the same
Comment 2 Radar WebKit Bug Importer 2021-06-28 11:06:17 PDT
<rdar://problem/79872484>
Comment 3 Kyle Piddington 2021-06-29 16:22:08 PDT
Created attachment 432552 [details]
Patch
Comment 4 EWS Watchlist 2021-06-29 16:23:55 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 5 Kenneth Russell 2021-06-29 16:29:33 PDT
Comment on attachment 432552 [details]
Patch

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

> Source/ThirdParty/ANGLE/ChangeLog:6
> +	Remove fastMath restriction on Intel, restore explicit [[invariant]] tag to position, fragcoord 

Please fix indentation in ChangeLog.

> Source/ThirdParty/ANGLE/src/compiler/translator/TranslatorMetalDirect/EmitMetal.cpp:977
> +    (decl.isField() ? mInvariants.contains(decl.field()) : mInvariants.contains(decl.variable())) || (qualifier == TQualifier::EvqPosition || qualifier == TQualifier::EvqFragCoord);

Making gl_Position and gl_FragCoord implicitly invariant seems like a far-reaching change. If fastMath is re-enabled on Intel, do more applications than just this one need this implicit invariant qualifier on these built-in variables?
Comment 6 Kyle Piddington 2021-06-29 18:00:58 PDT
Created attachment 432563 [details]
Patch
Comment 7 Kenneth Russell 2021-06-29 21:09:08 PDT
Comment on attachment 432563 [details]
Patch

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

Looks good to me. A few minor comments about the commit message. r+

> Source/ThirdParty/ANGLE/ChangeLog:7
> +        Removing fastmath in all scenarios leads to unnacceptable performance on integrated graphics

Consider adding periods at end of sentences.

> Source/ThirdParty/ANGLE/ChangeLog:8
> +        Currently, webgl conformance tests and the safrai tests don't have any invariance tests that show issue. 

safrai -> safari

show issue -> "show issues" or "show the issue"
Comment 8 Kyle Piddington 2021-07-01 09:28:01 PDT
Created attachment 432704 [details]
Patch for landing
Comment 9 EWS 2021-07-01 10:03:25 PDT
Committed r279466 (239322@main): <https://commits.webkit.org/239322@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432704 [details].
Comment 10 Truitt Savell 2021-07-01 15:02:27 PDT
It looks like the changes in https://trac.webkit.org/changeset/279466/webkit

broke 4 Webgl tests

tracking in https://bugs.webkit.org/show_bug.cgi?id=227596
Comment 11 Kyle Piddington 2021-07-07 14:10:18 PDT
*** Bug 226841 has been marked as a duplicate of this bug. ***