Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control #3559

dreampiggy · 2023-07-09T12:05:04Z

New Pull Request Checklist

I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (pod lib lint)

This merge request fixes / refers to the following issues: ...

Pull Request Description

This close #3417

Reason

This commit: ff6b3b9 cause to not force decode on Non-ImageIO created CGImage. In theory it's correct because like libwebp-created CGImage has already bitmap buffer in RAM.

However, this change has bad effect on final RAM usage. Because of https://github.com/path/FastImageCache#byte-alignment

When some bad coder (The SDWebImageWebPCoder in that example), does not handle the byte alignment correctly, CoreAnimation will try to copy the bitmap buffer when rendering, cause 2 * bitmap buffer usage, instead of 1.

You can use Xcode's Color Copied Image to find this behavior.
See:

Solution

SD itself provide the helper function to easily handle the bitmap info and byte alignment, using runtime detection (instead of hardcode). For example, previous hardcode BRRA8888/BGRX8888 is not correct.
WebPCoder need update its codebase to byte alignment the CGImage
(new) Users can control whether always force-decode, never, or use the automatic smart check provided by us

See: SDWebImage/SDWebImage#3559

dreampiggy · 2023-07-12T11:23:13Z

The SDWebImageAvoidDecodeImage is deprecated, using the new policy to control the behavior, which has 3 values instead of 2 values BOOL

dreampiggy · 2023-07-12T11:24:04Z

Added new SDImageForceDecodePolicy, which seems match user's feedback about details control, like only some of URLs, like webp URL, always force-decode, other use the automatic checking

dreampiggy · 2023-07-12T11:28:24Z

SDWebImage/Private/SDDeviceHelper.h

@@ -14,5 +14,6 @@

 + (NSUInteger)totalMemory;
 + (NSUInteger)freeMemory;
+ (size_t)cacheLineSize;


Seems useless.

…ck to handle the decode output bitmap info Include: colorspace, byte alignment, bitmap info Remove the exists hardcode on BGRA8888

Now it's will mark as false for not byte-aligned CGImage

The sd_isDecoded default value only check once

…iggerCACopyImage`

…orce-decode using automatic check, always or never Deprecate the `SDWebImageAvoidDecodeImage` This can help for some user who don't use ImageIO and need to ensure the memory footprint

…ed image and vector image to not break them

Should use device RGB, like Color LCD

…e and cause copy Fix the NSColor should still convert to sRGB for print...

… support

dreampiggy added the performance label Jul 9, 2023

dreampiggy added a commit to SDWebImage/SDWebImageWebPCoder that referenced this pull request Jul 9, 2023

Avoid force-decode by apply the byte alignment for static WebP images

8196947

See: SDWebImage/SDWebImage#3559

This was referenced Jul 9, 2023

Avoid force-decode by apply the byte alignment for static WebP images, using runtime detection for bitmap info SDWebImage/SDWebImageWebPCoder#77

Merged

Memory usage increases when webp is used after 5.13.0 #3417

Closed

dreampiggy force-pushed the performance/default_bitmap_and_force_decode branch from 71c17a3 to 6938175 Compare July 10, 2023 15:27

dreampiggy changed the title ~~Refactory the logic to handle non-ImageIO CGImage's force decode logic to avoid CA copy frame buffer~~ Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control Jul 12, 2023

dreampiggy added the Behavior Changes label Jul 12, 2023

dreampiggy commented Jul 12, 2023

View reviewed changes

dreampiggy force-pushed the performance/default_bitmap_and_force_decode branch 2 times, most recently from 42979e6 to 99873fc Compare July 12, 2023 17:32

dreampiggy added 10 commits July 13, 2023 22:58

Provide the API for custom coders (like WebPCoder) to use runtime che…

b455999

…ck to handle the decode output bitmap info Include: colorspace, byte alignment, bitmap info Remove the exists hardcode on BGRA8888

Update the default force decode value, partial revert ff6b3b9

06589db

Now it's will mark as false for not byte-aligned CGImage

Update the correct byte alignment and check with 8 * 4 bytes

f6ca661

The sd_isDecoded default value only check once

Added test case test28ThatNotTriggerCACopyImage and `test28ThatDoTr…

69c98c5

…iggerCACopyImage`

Introduce SDImageForceDecodePolicy, which can controls whether to f…

e1c01fe

…orce-decode using automatic check, always or never Deprecate the `SDWebImageAvoidDecodeImage` This can help for some user who don't use ImageIO and need to ensure the memory footprint

Update test case for SDImageForceDecodePolicyAutomatic

1a61c8d

Removed the unused cache line size helper method

b4af7cc

Fix the handle to context option and add test case

3e382b2

Fix the handle of ForceDecodePolicyAlways, still need to check animat…

cc89e14

…ed image and vector image to not break them

Fix the colorspace of hardware supported on macOS

67da434

Should use device RGB, like Color LCD

dreampiggy force-pushed the performance/default_bitmap_and_force_decode branch from c771362 to ed1a44b Compare July 13, 2023 14:58

dreampiggy added this to the 5.17.0 milestone Jul 13, 2023

Fix the macOS force decode colorspace does not match screen colorspac…

45b497c

…e and cause copy Fix the NSColor should still convert to sRGB for print...

dreampiggy force-pushed the performance/default_bitmap_and_force_decode branch 2 times, most recently from e3b6900 to 1fc9d4b Compare July 13, 2023 15:35

Rename to use struct to return the alignment, preserve for future HDR…

808cedc

… support

dreampiggy force-pushed the performance/default_bitmap_and_force_decode branch from 1fc9d4b to 808cedc Compare July 13, 2023 15:41

dreampiggy merged commit 8f16a63 into SDWebImage:master Jul 13, 2023
3 of 4 checks passed

dreampiggy deleted the performance/default_bitmap_and_force_decode branch July 13, 2023 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control #3559

Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control #3559

dreampiggy commented Jul 9, 2023 •

edited

dreampiggy commented Jul 12, 2023

dreampiggy commented Jul 12, 2023 •

edited

dreampiggy Jul 12, 2023

Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control #3559

Refactory the logic to handle force decode logic to avoid CA copy frame buffer, introduce SDImageForceDecodePolicy detailed control #3559

Conversation

dreampiggy commented Jul 9, 2023 • edited

New Pull Request Checklist

Pull Request Description

Reason

Solution

dreampiggy commented Jul 12, 2023

dreampiggy commented Jul 12, 2023 • edited

dreampiggy Jul 12, 2023

Choose a reason for hiding this comment

dreampiggy commented Jul 9, 2023 •

edited

dreampiggy commented Jul 12, 2023 •

edited