| Summary: | Disable hardware JPEG decoding on x86 Mac | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ben Nham <nham> | ||||||
| Component: | Images | Assignee: | 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
Ben Nham
2020-09-10 13:13:43 PDT
Created attachment 408472 [details]
Patch
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? 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 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. Sounds good. Created attachment 408483 [details]
Patch
Committed r266898: <https://trac.webkit.org/changeset/266898> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408483 [details]. |