Skip to content

Decode WebP images directly into a buffer Core Animation likes#600

Merged
garrettmoon merged 5 commits intopinterest:masterfrom
Adlai-Holler:faster_webp
Jun 2, 2021
Merged

Decode WebP images directly into a buffer Core Animation likes#600
garrettmoon merged 5 commits intopinterest:masterfrom
Adlai-Holler:faster_webp

Conversation

@Adlai-Holler
Copy link
Copy Markdown
Contributor

@Adlai-Holler Adlai-Holler commented May 28, 2021

Without this change, the images we output aren't compatible with the GPU and so CA has to copy them all on the main thread at commit time, which means stutters. Plus memory bloat.

The difference here is so substantial that you may seriously want to consider re-running a WebP experiment if you did in the past and bailed on it.

CA::copy_image before:
Screen Shot 2021-05-28 at 5 11 00 PM

After:
Screen Shot 2021-05-28 at 5 11 24 PM

Yes I am too lazy to figure out how to do this on macOS, and for that I am sorry. Desktops are fast anyway!

Without this change, the images we output aren't compatible with the GPU and so CA has to copy them all on the main thread at commit time, which means stutters. Plus memory bloat.

The difference here is so substantial that you may seriously want to consider re-running a WebP experiment if you did in the past and bailed on it.
@Adlai-Holler
Copy link
Copy Markdown
Contributor Author

Also updated the PR to attempt to use CGImageSource – which supports WebP at least as of iOS 14 – before trying our own manual WebP decoding.

@Adlai-Holler
Copy link
Copy Markdown
Contributor Author

Looks like the CI failure is a config issue unrelated to the CL.

WDYT about the change @garrettmoon ? Also hi! Long time no code.

@garrettmoon
Copy link
Copy Markdown
Collaborator

Long time no code!

I think the CI issue is real?
- WARN | [PINRemoteImage/WebP] xcodebuild: /Users/runner/work/PINRemoteImage/PINRemoteImage/Source/Classes/Categories/PINImage+WebP.m:15:13: warning: unused function 'releaseData' [-Wunused-function]

I think the releaseData method needs to be ifdef'd out for iOS?

My only concern about this PR is the bit rot that will happen on macOS… Any chance you can try and unify some of the code for macOS in a follow up?

@Adlai-Holler Adlai-Holler changed the title On iOS, decode WebP images directly into a buffer Core Animation likes Decode WebP images directly into a buffer Core Animation likes Jun 2, 2021
@Adlai-Holler
Copy link
Copy Markdown
Contributor Author

OK updated – lmk

Copy link
Copy Markdown
Collaborator

@garrettmoon garrettmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you @Adlai-Holler !

You… uh… think you can apply this to the animated webp code path next? :D

@garrettmoon garrettmoon merged commit 019b71b into pinterest:master Jun 2, 2021
Adlai-Holler added a commit to Adlai-Holler/odysee-ios that referenced this pull request Jun 2, 2021
akinwale added a commit to OdyseeTeam/odysee-ios that referenced this pull request Jun 3, 2021
annomusa added a commit to annomusa/PINRemoteImage that referenced this pull request Dec 17, 2021
* master-pinremoteimage:
  Disable asserts in release builds when using Swift Package Manager
  unarchiveObjectWithData: and archivedDataWithRootObject: are deprecated (pinterest#610)
  update comment
  Set cachePolicy accordingly when PINRemoteImageManagerDownloadOptionsIgnoreCache is provided
  Decode WebP images directly into a buffer Core Animation likes (pinterest#600)
  Xcode 12.5 / SPM - Add missing headers to fix build errors with SPM and Xcode 12.5 (pinterest#597)
  Fixed spm  integration on regular Xcode project (pinterest#586)
  Fix PINRemoteImageManager.h Close Comment Formatting (pinterest#594)
  Fix typo in PINRemoteImageManager.html (pinterest#593)
  Carthage is broken on Xcode 12 (pinterest#584)
  Update podspec to match release
  Hopefully fix publish for real this time...

# Conflicts:
#	Package.swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants