Bug 216377

Summary: Disable hardware JPEG decoding on x86 Mac
Product: WebKit Reporter: Ben Nham <nham>
Component: ImagesAssignee: Ben Nham <nham>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, benjamin, cdumez, cmarcelo, eric.carlson, ews-watchlist, ggaren, glenn, gyuyoung.kim, jer.noble, nham, philipj, ryuan.choi, sabouhallawa, sergio, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=224526
https://bugs.webkit.org/show_bug.cgi?id=226869
Attachments:
Description Flags
Patch
none
Patch none

Ben Nham
Reported 2020-09-10 13:13:43 PDT
Disable hardware JPEG decoding on x86 Mac
Attachments
Patch (14.72 KB, patch)
2020-09-10 13:27 PDT, Ben Nham 		no flags
DetailsFormatted DiffDiff
Patch (14.66 KB, patch)
2020-09-10 15:11 PDT, Ben Nham 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ben Nham
Comment 1 2020-09-10 13:27:00 PDT
Created attachment 408472 [details] Patch
Radar WebKit Bug Importer
Comment 2 2020-09-10 13:27:33 PDT
<rdar://problem/68661412>
Geoffrey Garen
Comment 3 2020-09-10 13:42:31 PDT
Comment on attachment 408472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408472&action=review r=me > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:341 > + if (PAL::isMediaToolboxFrameworkAvailable() && PAL::canLoad_MediaToolbox_FigPhotoDecompressionSetHardwareCutoff()) If we just need to know whether the framework is available, isn't weak linking the more efficient option?
Said Abou-Hallawa
Comment 4 2020-09-10 14:21:08 PDT
Comment on attachment 408472 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408472&action=review > Source/WTF/wtf/PlatformEnableCocoa.h:237 > +#if !defined(ENABLE_HARDWARE_JPEG) > +#if PLATFORM(MAC) && CPU(X86_64) > +#define ENABLE_HARDWARE_JPEG 0 > +#else > +#define ENABLE_HARDWARE_JPEG 1 > +#endif > +#endif Why do we need to define ENABLE_HARDWARE_JPEG when its value is 0? Can't the definition be like the rest in this file? #if !defined(ENABLE_HARDWARE_JPEG) && !(PLATFORM(MAC) && CPU(X86_64)) #define ENABLE_HARDWARE_JPEG 1 #endif
Ben Nham
Comment 5 2020-09-10 14:24:30 PDT
Weak linking is definitely preferable to soft linking for cases like this. (From asking around it sounds like we are soft linking MediaToolbox not because of a circular dependency but because it doesn't exist in BaseSystem.) But that would be a larger and riskier patch so I'll probably do the work of changing this from a soft link to a weak link in a separate patch. I'll address Said's comment in the patch for landing.
Geoffrey Garen
Comment 6 2020-09-10 14:31:18 PDT
Sounds good.
Ben Nham
Comment 7 2020-09-10 15:11:42 PDT
Created attachment 408483 [details] Patch
EWS
Comment 8 2020-09-10 16:52:15 PDT
Committed r266898: <https://trac.webkit.org/changeset/266898> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408483 [details].
Note You need to log in before you can comment on or make changes to this bug.