Bug 216377 – Disable hardware JPEG decoding on x86 Mac

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Ben Nham]

Disable hardware JPEG decoding on x86 Mac

[Ben Nham]

Created attachment 408472 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/68661412>

[Geoffrey Garen]

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 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]

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]

Sounds good.

[Ben Nham]

Created attachment 408483 [details]
Patch

[EWS]

Committed r266898: <https://trac.webkit.org/changeset/266898>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408483 [details].