Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Create Image.MakeImageFromPlatformBuffer(..) #2344

Merged
merged 18 commits into from
Apr 10, 2024

Conversation

mrousavy
Copy link
Contributor

@mrousavy mrousavy commented Apr 5, 2024

Creates a new API: Skia.Image.MakeImageFromPlatformBuffer(..).

This API can create an SkImage instance from a "platform buffer".

A "platform buffer" is a platform-native GPU/CPU buffer instance:

  • On iOS: This is a CMSampleBuffer.
  • On Android: This is a AHardwareBuffer*

This is a preparation for VisionCamera Skia Frame Processors, where a VisionCamera Frame can be converted to an SkImage and later be used for drawing operations.

const surface = useSharedValue(null)
const frameProcessor = useFrameProcessor((frame) => {
  'worklet'

  // 1. prepare Surface
  if (surface.value == null) {
    surface.value = Skia.Surface.MakeOffscreen(frame.width, frame.height)
  }
  const canvas = surface.value.getCanvas()

  // 2. convert Frame to SkImage
  const platformBuffer = frame.getPlatformBuffer()
  const image = Skia.Image.MakeFromPlatformBuffer(platformBuffer.pointer)

  // 3. draw it (and any other drawing operations)
  canvas.drawImage(image, 0, 0)

  // 4. flush and clean
  surface.value.flush()
  platformBuffer.delete()
}, [])

Demo of a VisionCamera Frame Processor drawing a blur shader directly onto the Camera Frame in realtime at 60 FPS:

RPReplay_Final1712748323.mov

@@ -666,4 +666,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: b6119b41acd610ac50d2967dabb3239ee5bbb4ec

COCOAPODS: 1.15.2
COCOAPODS: 1.14.3
Copy link
Contributor

Choose a reason for hiding this comment

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

to revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted!

@wcandillon
Copy link
Contributor

looks great 👍

* @throws Throws an error if the Image could not be created, for example when the given
* platform buffer is invalid.
*/
MakeImageFromPlatformBuffer: (platformBuffer: bigint) => SkImage
Copy link
Contributor

Choose a reason for hiding this comment

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

let's run a little lint --fix here, sorry we don't have the same eslint rules

Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be named MakeImageFromGPUBuffer or MakeImageFromBuffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • MakeImageFromBuffer no because a buffer is normally something like a uint8_t*. This is not a normal buffer, it is a special platform native buffer that is shared across GPU and CPU.
  • MakeImageFromGPUBuffer, hmm, a bit better, but still not entirely accurate since CMSampleBuffer and AHardwareBuffer are GPU / CPU memory, shared access. That's their whole point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Linted 👍

CMSampleBufferRef sampleBuffer) {
if (!SkiaMetalSurfaceFactory::createSkiaDirectContextIfNecessary(
&ThreadContextHolder::ThreadSkiaMetalContext)) {
throw std::runtime_error("Failed to create Skia Context for this Thread!");
Copy link
Contributor

Choose a reason for hiding this comment

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

if we are not able to avoid the cpu memory copy on iOS for now and we want to ship this feature. This is where the CPU memory copy should happen (SkImages::MakeRaster)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, for now I am doing this in JS and it works fine - on my iPhone 15 Pro it still runs at 120 FPS :)

So from my side we can ship. Maybe it is a bug in Skia or something we are doing wrong that makes the GPU Texture SkImage invalid, hence we need to do a non-texture-image copy explicitly. Fine for now, better to fix later

@wcandillon wcandillon merged commit 933a0e2 into Shopify:main Apr 10, 2024
1 check passed
Copy link
Contributor

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants