Bug 227226

Summary: BabylonJS Under water demo is slower than it should be on Intel
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kyle Piddington <kpiddington>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, floooh, kbr, kkinnunen, kondapallykalyan, kpiddington, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Mac (Intel)   
OS: macOS 10.15   
URL: https://playground.babylonjs.com/#LPTLZM
Bug Depends on:    
Bug Blocks: 223434, 227596    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Kimmo Kinnunen
Reported 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.
Attachments
Patch (3.04 KB, patch)
2021-06-29 16:22 PDT, Kyle Piddington 		no flags
DetailsFormatted DiffDiff
Patch (2.36 KB, patch)
2021-06-29 18:00 PDT, Kyle Piddington 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.40 KB, patch)
2021-07-01 09:28 PDT, Kyle Piddington 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Kimmo Kinnunen
Comment 1 2021-06-21 11:07:18 PDT
Related to rdar://79216506 but not strictly the same
Radar WebKit Bug Importer
Comment 2 2021-06-28 11:06:17 PDT
<rdar://problem/79872484>
Kyle Piddington
Comment 3 2021-06-29 16:22:08 PDT
Created attachment 432552 [details] Patch
EWS Watchlist
Comment 4 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
Kenneth Russell
Comment 5 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?
Kyle Piddington
Comment 6 2021-06-29 18:00:58 PDT
Created attachment 432563 [details] Patch
Kenneth Russell
Comment 7 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"
Kyle Piddington
Comment 8 2021-07-01 09:28:01 PDT
Created attachment 432704 [details] Patch for landing
EWS
Comment 9 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].
Truitt Savell
Comment 10 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
Kyle Piddington
Comment 11 2021-07-07 14:10:18 PDT
*** Bug 226841 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.