Skip to content

Commit d314745

Browse files
wcandillonwcandillon
authored
fix(🤖): bug fixes in OpenGL renderer (#2759)
This fixes a couple of issues where the OpenGL renderer could get in an inconsistent state.
1 parent 516bdb5 commit d314745

File tree

16 files changed

+85
-117
lines changed

16 files changed

+85
-117
lines changed

‎apps/paper/src/Examples/API/Data.tsx

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import React from "react";
2-
import { useWindowDimensions } from "react-native";
32
import {
43
AlphaType,
54
Canvas,
@@ -28,12 +27,19 @@ const img = Skia.Image.MakeImage(
2827
256 * 4
2928
)!;
3029

30+
const surface = Skia.Surface.MakeOffscreen(256, 256)!;
31+
const canvas = surface.getCanvas();
32+
canvas.drawColor(Skia.Color("cyan"));
33+
const paint = Skia.Paint();
34+
paint.setColor(Skia.Color("magenta"));
35+
canvas.drawCircle(128, 128, 128, paint);
36+
const img1 = surface.makeImageSnapshot().makeNonTextureImage();
37+
3138
export const Data = () => {
32-
const { width } = useWindowDimensions();
33-
const SIZE = width;
3439
return (
35-
<Canvas style={{ width: SIZE, height: SIZE }}>
40+
<Canvas style={{ width: 256, height: 512 }}>
3641
<Image image={img} x={0} y={0} width={256} height={256} fit="cover" />
42+
<Image image={img1} x={0} y={256} width={256} height={256} fit="cover" />
3743
</Canvas>
3844
);
3945
};

‎packages/skia/android/cpp/jni/include/JniSkiaBaseView.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ class JniSkiaBaseView {
3333
_skiaAndroidView->surfaceAvailable(surface, width, height);
3434
}
3535

36-
virtual void surfaceSizeChanged(int width, int height) {
37-
_skiaAndroidView->surfaceSizeChanged(width, height);
36+
virtual void surfaceSizeChanged(jobject surface, int width, int height) {
37+
_skiaAndroidView->surfaceSizeChanged(surface, width, height);
3838
}
3939

4040
virtual void surfaceDestroyed() { _skiaAndroidView->surfaceDestroyed(); }

‎packages/skia/android/cpp/jni/include/JniSkiaDomView.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ class JniSkiaDomView : public jni::HybridClass<JniSkiaDomView>,
5151
JniSkiaBaseView::surfaceAvailable(surface, width, height);
5252
}
5353

54-
void surfaceSizeChanged(int width, int height) override {
55-
JniSkiaBaseView::surfaceSizeChanged(width, height);
54+
void surfaceSizeChanged(jobject surface, int width, int height) override {
55+
JniSkiaBaseView::surfaceSizeChanged(surface, width, height);
5656
}
5757

5858
void surfaceDestroyed() override { JniSkiaBaseView::surfaceDestroyed(); }

‎packages/skia/android/cpp/jni/include/JniSkiaPictureView.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ class JniSkiaPictureView : public jni::HybridClass<JniSkiaPictureView>,
5353
JniSkiaBaseView::surfaceAvailable(surface, width, height);
5454
}
5555

56-
void surfaceSizeChanged(int width, int height) override {
57-
JniSkiaBaseView::surfaceSizeChanged(width, height);
56+
void surfaceSizeChanged(jobject surface, int width, int height) override {
57+
JniSkiaBaseView::surfaceSizeChanged(surface, width, height);
5858
}
5959

6060
void surfaceDestroyed() override { JniSkiaBaseView::surfaceDestroyed(); }

‎packages/skia/android/cpp/rnskia-android/OpenGLContext.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,11 @@ class OpenGLContext {
125125
#endif
126126
}
127127

128+
// TODO: remove width, height
128129
std::unique_ptr<WindowContext> MakeWindow(ANativeWindow *window, int width,
129130
int height) {
130-
return std::make_unique<OpenGLWindowContext>(this, window, width, height);
131+
return std::make_unique<OpenGLWindowContext>(
132+
_directContext, _glDisplay.get(), _glContext.get(), window);
131133
}
132134

133135
private:

‎packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.cpp

+20-49
Original file line numberDiff line numberDiff line change
@@ -16,74 +16,45 @@ namespace RNSkia {
1616

1717
sk_sp<SkSurface> OpenGLWindowContext::getSurface() {
1818
if (_skSurface == nullptr) {
19-
20-
struct ReleaseContext {
21-
std::unique_ptr<gl::Surface> surface = nullptr;
22-
};
23-
24-
if (!_window) {
25-
throw std::runtime_error("No native window provided");
26-
}
27-
auto releaseCtx = new ReleaseContext();
28-
releaseCtx->surface =
29-
_context->_glDisplay->makeWindowSurface(_context->_glConfig, _window);
30-
if (!releaseCtx->surface) {
31-
throw std::runtime_error("Failed to create window surface");
32-
}
33-
_glSurface = releaseCtx->surface.get();
34-
35-
// Now make this one current
36-
auto success = _context->_glContext->makeCurrent(releaseCtx->surface.get());
37-
if (!success) {
38-
throw std::runtime_error("Failed to make window surface current");
39-
}
40-
41-
// Set up parameters for the render target so that it
42-
// matches the underlying OpenGL context.
43-
GrGLFramebufferInfo fboInfo;
44-
45-
// We pass 0 as the framebuffer id, since the
46-
// underlying Skia GrGlGpu will read this when wrapping the context in the
47-
// render target and the GrGlGpu object.
48-
fboInfo.fFBOID = 0;
49-
fboInfo.fFormat = 0x8058; // GL_RGBA8
50-
19+
_glContext->makeCurrent(_glSurface.get());
5120
GLint stencil;
5221
glGetIntegerv(GL_STENCIL_BITS, &stencil);
5322

5423
GLint samples;
5524
glGetIntegerv(GL_SAMPLES, &samples);
5625

57-
auto colorType = kN32_SkColorType;
26+
auto colorType = kRGBA_8888_SkColorType;
5827

5928
auto maxSamples =
60-
_context->_directContext->maxSurfaceSampleCountForColorType(colorType);
29+
_directContext->maxSurfaceSampleCountForColorType(colorType);
6130

6231
if (samples > maxSamples) {
6332
samples = maxSamples;
6433
}
6534

66-
auto renderTarget = GrBackendRenderTargets::MakeGL(_width, _height, samples,
67-
stencil, fboInfo);
68-
69-
SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
70-
71-
// Create surface object
35+
GrGLFramebufferInfo fbInfo;
36+
fbInfo.fFBOID = 0;
37+
fbInfo.fFormat = GR_GL_RGBA8;
38+
// fbInfo.fProtected =
39+
// skgpu::Protected(fDisplayParams.fCreateProtectedNativeBackend);
40+
41+
auto width = ANativeWindow_getWidth(_window);
42+
auto height = ANativeWindow_getHeight(_window);
43+
auto backendRT =
44+
GrBackendRenderTargets::MakeGL(width, height, samples, stencil, fbInfo);
45+
sk_sp<SkColorSpace> colorSpace(nullptr);
46+
SkSurfaceProps surfaceProps(0, kRGB_H_SkPixelGeometry);
7247
_skSurface = SkSurfaces::WrapBackendRenderTarget(
73-
_context->_directContext.get(), renderTarget,
74-
kBottomLeft_GrSurfaceOrigin, colorType, nullptr, &props,
75-
[](void *addr) {
76-
auto releaseCtx = reinterpret_cast<ReleaseContext *>(addr);
77-
delete releaseCtx;
78-
},
79-
reinterpret_cast<void *>(releaseCtx));
48+
_directContext.get(), backendRT, kBottomLeft_GrSurfaceOrigin,
49+
kRGBA_8888_SkColorType, colorSpace, &surfaceProps);
8050
}
8151
return _skSurface;
8252
}
8353

8454
void OpenGLWindowContext::present() {
85-
_context->_glContext->makeCurrent(_glSurface);
86-
_context->_directContext->flushAndSubmit();
55+
_glContext->makeCurrent(_glSurface.get());
56+
// TODO: is flushAndSubmit needed here?
57+
_directContext->flushAndSubmit();
8758
_glSurface->present();
8859
}
8960

‎packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.h

+14-16
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,16 @@
3131

3232
namespace RNSkia {
3333

34-
class OpenGLContext;
35-
3634
class OpenGLWindowContext : public WindowContext {
3735
public:
38-
OpenGLWindowContext(OpenGLContext *context, ANativeWindow *window, int width,
39-
int height)
40-
: _context(context), _window(window), _width(width), _height(height) {
36+
OpenGLWindowContext(sk_sp<GrDirectContext> directContext,
37+
gl::Display *display, gl::Context *glContext,
38+
ANativeWindow *window)
39+
: _directContext(directContext), _display(display), _glContext(glContext),
40+
_window(window) {
4141
ANativeWindow_acquire(_window);
42+
auto config = display->chooseConfig();
43+
_glSurface = display->makeWindowSurface(config, _window);
4244
}
4345

4446
~OpenGLWindowContext() {
@@ -51,23 +53,19 @@ class OpenGLWindowContext : public WindowContext {
5153

5254
void present() override;
5355

54-
void resize(int width, int height) override {
55-
_skSurface = nullptr;
56-
_width = width;
57-
_height = height;
58-
}
56+
int getWidth() override { return ANativeWindow_getWidth(_window); };
5957

60-
int getWidth() override { return _width; };
58+
int getHeight() override { return ANativeWindow_getHeight(_window); };
6159

62-
int getHeight() override { return _height; };
60+
void resize(int width, int height) override { _skSurface = nullptr; }
6361

6462
private:
65-
OpenGLContext *_context;
63+
sk_sp<GrDirectContext> _directContext;
64+
gl::Display *_display;
6665
ANativeWindow *_window;
6766
sk_sp<SkSurface> _skSurface = nullptr;
68-
gl::Surface *_glSurface = nullptr;
69-
int _width = 0;
70-
int _height = 0;
67+
gl::Context *_glContext = nullptr;
68+
std::unique_ptr<gl::Surface> _glSurface = nullptr;
7169
};
7270

7371
} // namespace RNSkia

‎packages/skia/android/cpp/rnskia-android/RNSkAndroidView.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class RNSkBaseAndroidView {
1515

1616
virtual void surfaceDestroyed() = 0;
1717

18-
virtual void surfaceSizeChanged(int width, int height) = 0;
18+
virtual void surfaceSizeChanged(jobject surface, int width, int height) = 0;
1919

2020
virtual float getPixelDensity() = 0;
2121

@@ -50,9 +50,9 @@ class RNSkAndroidView : public T, public RNSkBaseAndroidView {
5050
->surfaceDestroyed();
5151
}
5252

53-
void surfaceSizeChanged(int width, int height) override {
53+
void surfaceSizeChanged(jobject surface, int width, int height) override {
5454
std::static_pointer_cast<RNSkOpenGLCanvasProvider>(T::getCanvasProvider())
55-
->surfaceSizeChanged(width, height);
55+
->surfaceSizeChanged(surface, width, height);
5656
// This is only need for the first time to frame, this renderImmediate call
5757
// will invoke updateTexImage for the previous frame
5858
RNSkView::renderImmediate();

‎packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp

+13-4
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ bool RNSkOpenGLCanvasProvider::renderToCanvas(
7676

7777
void RNSkOpenGLCanvasProvider::surfaceAvailable(jobject jSurfaceTexture,
7878
int width, int height) {
79+
// If the surface is 0, we can skip it
80+
if (width == 0 && height == 0) {
81+
return;
82+
}
7983
// Create renderer!
8084
JNIEnv *env = facebook::jni::Environment::current();
8185

@@ -118,17 +122,22 @@ void RNSkOpenGLCanvasProvider::surfaceDestroyed() {
118122
}
119123
}
120124

121-
void RNSkOpenGLCanvasProvider::surfaceSizeChanged(int width, int height) {
125+
void RNSkOpenGLCanvasProvider::surfaceSizeChanged(jobject jSurfaceTexture,
126+
int width, int height) {
122127
if (width == 0 && height == 0) {
123128
// Setting width/height to zero is nothing we need to care about when
124129
// it comes to invalidating the surface.
125130
return;
126131
}
127132

128-
// Recreate RenderContext surface based on size change???
129-
_surfaceHolder->resize(width, height);
133+
if (_surfaceHolder == nullptr) {
134+
_surfaceHolder = nullptr;
135+
surfaceAvailable(jSurfaceTexture, width, height);
136+
} else {
137+
_surfaceHolder->resize(width, height);
138+
}
130139

131140
// Redraw after size change
132141
_requestRedraw();
133142
}
134-
} // namespace RNSkia
143+
} // namespace RNSkia

‎packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class RNSkOpenGLCanvasProvider
3131

3232
void surfaceDestroyed();
3333

34-
void surfaceSizeChanged(int width, int height);
34+
void surfaceSizeChanged(jobject jSurface, int width, int height);
3535

3636
private:
3737
std::unique_ptr<WindowContext> _surfaceHolder = nullptr;

‎packages/skia/android/cpp/rnskia-android/gl/Display.h

+4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ class Display {
3535

3636
bool isValid() const { return _display != EGL_NO_DISPLAY; }
3737

38+
void clearContext() {
39+
eglMakeCurrent(_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
40+
}
41+
3842
EGLConfig chooseConfig() {
3943

4044
EGLint att[] = {EGL_RENDERABLE_TYPE,

‎packages/skia/android/src/main/java/com/shopify/reactnative/skia/SkiaBaseView.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void onSurfaceTextureSizeChanged(SurfaceTexture surface, int width, int h
6464
return;
6565
}
6666
Log.i(tag, "onSurfaceTextureSizeChanged " + width + "/" + height);
67-
surfaceSizeChanged(width, height);
67+
surfaceSizeChanged(surface, width, height);
6868
}
6969

7070
@Override
@@ -82,18 +82,18 @@ public boolean onSurfaceTextureDestroyed(SurfaceTexture surface) {
8282
return false;
8383
}
8484

85-
//private long _prevTimestamp = 0;
85+
private long _prevTimestamp = 0;
8686
@Override
8787
public void onSurfaceTextureUpdated(SurfaceTexture surface) {
88-
// long timestamp = surface.getTimestamp();
89-
// long frameDuration = (timestamp - _prevTimestamp)/1000000;
90-
// Log.i(tag, "onSurfaceTextureUpdated "+frameDuration+"ms");
91-
// _prevTimestamp = timestamp;
88+
long timestamp = surface.getTimestamp();
89+
long frameDuration = (timestamp - _prevTimestamp)/1000000;
90+
Log.i(tag, "onSurfaceTextureUpdated "+frameDuration+"ms");
91+
_prevTimestamp = timestamp;
9292
}
9393

9494
protected abstract void surfaceAvailable(Object surface, int width, int height);
9595

96-
protected abstract void surfaceSizeChanged(int width, int height);
96+
protected abstract void surfaceSizeChanged(Object surface, int width, int height);
9797

9898
protected abstract void surfaceDestroyed();
9999

‎packages/skia/android/src/main/java/com/shopify/reactnative/skia/SkiaDomView.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protected void finalize() throws Throwable {
2727

2828
protected native void surfaceAvailable(Object surface, int width, int height);
2929

30-
protected native void surfaceSizeChanged(int width, int height);
30+
protected native void surfaceSizeChanged(Object surface, int width, int height);
3131

3232
protected native void surfaceDestroyed();
3333

@@ -37,8 +37,6 @@ protected void finalize() throws Throwable {
3737

3838
protected native void setDebugMode(boolean show);
3939

40-
protected native void updateTouchPoints(double[] points);
41-
4240
protected native void registerView(int nativeId);
4341

4442
protected native void unregisterView();

‎packages/skia/android/src/main/java/com/shopify/reactnative/skia/SkiaPictureView.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ protected void finalize() throws Throwable {
2626

2727
protected native void surfaceAvailable(Object surface, int width, int height);
2828

29-
protected native void surfaceSizeChanged(int width, int height);
29+
protected native void surfaceSizeChanged(Object surface, int width, int height);
3030

3131
protected native void surfaceDestroyed();
3232

@@ -36,8 +36,6 @@ protected void finalize() throws Throwable {
3636

3737
protected native void setDebugMode(boolean show);
3838

39-
protected native void updateTouchPoints(double[] points);
40-
4139
protected native void registerView(int nativeId);
4240

4341
protected native void unregisterView();

‎packages/skia/android/src/paper/java/com/facebook/react/viewmanagers/SkiaDrawViewManagerInterface.java

-18
This file was deleted.

0 commit comments

Comments
 (0)