Skip to content

Making first frame render synchronous #2989

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ sk_sp<SkSurface> OpenGLWindowContext::getSurface() {
return _skSurface;
}

void OpenGLWindowContext::present() {
void OpenGLWindowContext::present(bool flush) {
_glContext->makeCurrent(_glSurface.get());
_directContext->flushAndSubmit();
_directContext->flushAndSubmit(flush ? GrSyncCpu::kYes : GrSyncCpu::kNo);
_glSurface->present();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class OpenGLWindowContext : public WindowContext {

sk_sp<SkSurface> getSurface() override;

void present() override;
void present(bool flush) override;

int getWidth() override { return ANativeWindow_getWidth(_window); };

Expand Down
4 changes: 2 additions & 2 deletions packages/skia/android/cpp/rnskia-android/RNSkAndroidView.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class RNSkAndroidView : public T, public RNSkBaseAndroidView {
bool opaque) override {
std::static_pointer_cast<RNSkOpenGLCanvasProvider>(T::getCanvasProvider())
->surfaceAvailable(surface, width, height, opaque);
RNSkView::redraw();
RNSkView::redraw(true);
}

void surfaceDestroyed() override {
Expand All @@ -52,7 +52,7 @@ class RNSkAndroidView : public T, public RNSkBaseAndroidView {
->surfaceSizeChanged(surface, width, height, opaque);
// This is only need for the first time to frame, this renderImmediate call
// will invoke updateTexImage for the previous frame
RNSkView::redraw();
RNSkView::redraw(true);
}

float getPixelDensity() override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ int RNSkOpenGLCanvasProvider::getScaledHeight() {
}

bool RNSkOpenGLCanvasProvider::renderToCanvas(
const std::function<void(SkCanvas *)> &cb) {
const std::function<void(SkCanvas *)> &cb, bool flush) {
if (_surfaceHolder != nullptr && cb != nullptr) {
// Get the surface
auto surface = _surfaceHolder->getSurface();
Expand All @@ -66,7 +66,7 @@ bool RNSkOpenGLCanvasProvider::renderToCanvas(
// Draw into canvas using callback
cb(surface->getCanvas());
// Swap buffers and show on screen
_surfaceHolder->present();
_surfaceHolder->present(flush);
return true;
} else {
// the render context did not provide a surface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class RNSkOpenGLCanvasProvider

int getScaledHeight() override;

bool renderToCanvas(const std::function<void(SkCanvas *)> &cb) override;
bool renderToCanvas(const std::function<void(SkCanvas *)> &cb, bool flush) override;

void surfaceAvailable(jobject surface, int width, int height, bool opaque);

Expand Down
2 changes: 1 addition & 1 deletion packages/skia/cpp/api/JsiSkiaContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class JsiSkiaContext : public JsiSkWrappingSharedPtrHostObject<WindowContext> {
}

JSI_HOST_FUNCTION(present) {
getObject()->present();
getObject()->present(true);
return jsi::Value::undefined();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/skia/cpp/rnskia/DawnWindowContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace RNSkia {

void DawnWindowContext::present() {
void DawnWindowContext::present(bool flush) {
auto recording = _recorder->snap();
if (!recording) {
throw std::runtime_error("Failed to create graphite recording");
Expand Down
2 changes: 1 addition & 1 deletion packages/skia/cpp/rnskia/DawnWindowContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class DawnWindowContext : public WindowContext {
return surface;
}

void present() override;
void present(bool flush) override;

void resize(int width, int height) override {
throw std::runtime_error("resize not implemented yet");
Expand Down
8 changes: 4 additions & 4 deletions packages/skia/cpp/rnskia/RNSkPictureView.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class RNSkPictureRenderer
_platformContext(std::move(context)) {}

void
renderImmediate(std::shared_ptr<RNSkCanvasProvider> canvasProvider) override {
performDraw(canvasProvider);
renderImmediate(std::shared_ptr<RNSkCanvasProvider> canvasProvider, bool flush) override {
performDraw(canvasProvider, flush);
}

void setPicture(sk_sp<SkPicture> picture) {
Expand All @@ -54,7 +54,7 @@ class RNSkPictureRenderer
}

private:
bool performDraw(std::shared_ptr<RNSkCanvasProvider> canvasProvider) {
bool performDraw(std::shared_ptr<RNSkCanvasProvider> canvasProvider, bool flush) {
return canvasProvider->renderToCanvas([=](SkCanvas *canvas) {
// Make sure to scale correctly
auto pd = _platformContext->getPixelDensity();
Expand All @@ -65,7 +65,7 @@ class RNSkPictureRenderer
canvas->drawPicture(_picture);
}
canvas->restore();
});
}, flush);
}

std::shared_ptr<RNSkPlatformContext> _platformContext;
Expand Down
14 changes: 7 additions & 7 deletions packages/skia/cpp/rnskia/RNSkView.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class RNSkCanvasProvider {
/**
Render to a canvas
*/
virtual bool renderToCanvas(const std::function<void(SkCanvas *)> &) = 0;
virtual bool renderToCanvas(const std::function<void(SkCanvas *)> &, bool flush) = 0;

protected:
std::function<void()> _requestRedraw;
Expand All @@ -55,7 +55,7 @@ class RNSkRenderer {
: _requestRedraw(std::move(requestRedraw)), _showDebugOverlays(false) {}

virtual void
renderImmediate(std::shared_ptr<RNSkCanvasProvider> canvasProvider) = 0;
renderImmediate(std::shared_ptr<RNSkCanvasProvider> canvasProvider, bool flush) = 0;

void setShowDebugOverlays(bool showDebugOverlays) {
_showDebugOverlays = showDebugOverlays;
Expand Down Expand Up @@ -114,7 +114,7 @@ class RNSkOffscreenCanvasProvider : public RNSkCanvasProvider {
/**
Render to a canvas
*/
bool renderToCanvas(const std::function<void(SkCanvas *)> &cb) override {
bool renderToCanvas(const std::function<void(SkCanvas *)> &cb, bool flush) override {
cb(_surface->getCanvas());
return true;
};
Expand Down Expand Up @@ -157,16 +157,16 @@ class RNSkView : public std::enable_shared_from_this<RNSkView> {
if (auto strongThis = weakThis.lock()) {
// Only proceed if the object still exists
if (strongThis->_renderer && strongThis->_redrawRequested) {
strongThis->_renderer->renderImmediate(strongThis->_canvasProvider);
strongThis->_renderer->renderImmediate(strongThis->_canvasProvider, false);
strongThis->_redrawRequested = false;
}
}
});
}
}

void redraw() {
_renderer->renderImmediate(_canvasProvider);
void redraw(bool flush) {
_renderer->renderImmediate(_canvasProvider, flush);
_redrawRequested = false;
}

Expand Down Expand Up @@ -197,7 +197,7 @@ class RNSkView : public std::enable_shared_from_this<RNSkView> {
getPlatformContext(), std::bind(&RNSkView::requestRedraw, this),
_canvasProvider->getScaledWidth(), _canvasProvider->getScaledHeight());

_renderer->renderImmediate(provider);
_renderer->renderImmediate(provider, false);
return provider->makeSnapshot(bounds);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/skia/cpp/rnskia/WindowContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class WindowContext {
public:
virtual ~WindowContext() = default;
virtual sk_sp<SkSurface> getSurface() = 0;
virtual void present() = 0;
virtual void present(bool flush) = 0;
virtual void resize(int width, int height) = 0;
virtual int getWidth() = 0;
virtual int getHeight() = 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/skia/ios/RNSkia-iOS/MetalWindowContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class MetalWindowContext : public RNSkia::WindowContext {

sk_sp<SkSurface> getSurface() override;

void present() override;
void present(bool flush) override;

int getWidth() override {
return _layer.frame.size.width * _layer.contentsScale;
Expand Down
7 changes: 6 additions & 1 deletion packages/skia/ios/RNSkia-iOS/MetalWindowContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,18 @@
return _skSurface;
}

void MetalWindowContext::present() {
void MetalWindowContext::present(bool flush) {
if (auto dContext = GrAsDirectContext(_skSurface->recordingContext())) {
dContext->flushAndSubmit();
}

id<MTLCommandBuffer> commandBuffer([_commandQueue commandBuffer]);
[commandBuffer presentDrawable:_currentDrawable];
[commandBuffer commit];

if (flush) {
[commandBuffer waitUntilCompleted];
}

_skSurface = nullptr;
}
2 changes: 1 addition & 1 deletion packages/skia/ios/RNSkia-iOS/RNSkMetalCanvasProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class RNSkMetalCanvasProvider : public RNSkia::RNSkCanvasProvider {
int getScaledWidth() override;
int getScaledHeight() override;

bool renderToCanvas(const std::function<void(SkCanvas *)> &cb) override;
bool renderToCanvas(const std::function<void(SkCanvas *)> &cb, bool flush) override;

void setSize(int width, int height);
CALayer *getLayer();
Expand Down
4 changes: 2 additions & 2 deletions packages/skia/ios/RNSkia-iOS/RNSkMetalCanvasProvider.mm
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
Render to a canvas
*/
bool RNSkMetalCanvasProvider::renderToCanvas(
const std::function<void(SkCanvas *)> &cb) {
const std::function<void(SkCanvas *)> &cb, bool flush) {
if (!_ctx) {
return false;
}
Expand Down Expand Up @@ -82,7 +82,7 @@
}
auto canvas = surface->getCanvas();
cb(canvas);
_ctx->present();
_ctx->present(flush);
}
return true;
};
Expand Down
3 changes: 2 additions & 1 deletion packages/skia/ios/RNSkia-iOS/SkiaUIView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ - (void)willMoveToSuperview:(UIView *)newWindow {
_manager->setSkiaView(_nativeId, _impl->getDrawView());
}
_impl->getDrawView()->setShowDebugOverlays(_debugMode);
[self setNeedsDisplay];
}
}
}
Expand Down Expand Up @@ -125,7 +126,7 @@ - (void)drawRect:(CGRect)rect {
// We override drawRect to ensure we to direct rendering when the
// underlying OS view needs to render:
if (_impl != nullptr) {
_impl->getDrawView()->redraw();
_impl->getDrawView()->redraw(true);
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/skia/src/renderer/Canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ export const Canvas = forwardRef(
}, []);

// Root
const root = useMemo(() => new SkiaSGRoot(Skia, nativeId), [nativeId]);
const root = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised this change helps, do you have examples I can try where I could see a difference there?

Copy link
Author

Choose a reason for hiding this comment

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

Funnily enough, removing either set of changes (this one or the synchronous presentation) makes the bug return (i.e. the first frame renders empty).

Copy link
Author

Choose a reason for hiding this comment

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

Happy to provide an example, but not sure what would be the best form? Ideally I would prepare a unit test demonstrating the issue, but I didn't find a suitable example to start off with – I would need to render a component, preferrably with frame-by-frame control, would appreciate hints on how to do it.

const result = new SkiaSGRoot(Skia, nativeId);
result.render(children);

return result;
}, [nativeId]);

// Render effects
useEffect(() => {
Expand Down