diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/Issue514Workaround.java b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/Issue514Workaround.java index c794bb1c..13745800 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/Issue514Workaround.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/Issue514Workaround.java @@ -12,16 +12,14 @@ import com.otaliastudios.cameraview.preview.RendererThread; /** * Fixes an issue for some devices with snapshot picture and video recording. - * This is so dirty and totally unclear that I wanted to have a separate class. + * This is so unclear that I wanted to have a separate class holding code and comments. * - * WHY IT FIXES THAT ISSUE - * I have no answer, this is pure magic to me. + * WHEN TO USE THIS CLASS * There is actually no need of this class in some cases: * - when we don't have overlays, everything works * - on the majority of devices, everything works * But some devices will show the issue #514 and so they need this class to fix it. - * I believe that this has no performance impact but it could be measured and the use of this - * class could be restricted to those specific devices that need it. + * We will use this always since it should have close to none performance impact. * * SNAPSHOT PROCEDURE * The issue is about picture and video snapshots with overlays. In both cases, we: @@ -75,12 +73,15 @@ import com.otaliastudios.cameraview.preview.RendererThread; * THE MAGIC * Hard to say why, but using this class fixes the described issue. * It seems that when the {@link SurfaceTexture#updateTexImage()} method for the overlay surface - * is called - the one that updates the overlayTextureId - we must ensure that the currently - * bound texture is the cameraTextureId. + * is called - the one that updates the overlayTextureId - we must ensure that the CURRENTLY + * BOUND TEXTURE ID IS NOT 0. The id we choose to apply might be cameraTextureId, or overlayTextureId, + * or probably whatever other valid id, and should be passed to {@link #Issue514Workaround(int)}. + * [Tested with cameraTextureId and overlayTextureId: both do work.] + * [Tested with invalid id like 9999. This won't work.] * - * This makes no sense, since overlaySurfaceTexture.updateTexImage() is setting it to overlayTextureId, - * but it fixes the issue. Specifically, after any draw operation with {@link EglViewport}, the bound - * texture is reset to 0 so this must be undone here. We offer: + * This makes no sense, since overlaySurfaceTexture.updateTexImage() is setting it to overlayTextureId + * anyway, but it fixes the issue. Specifically, after any draw operation with {@link EglViewport}, + * the bound texture is reset to 0 so this must be undone here. We offer: * * - {@link #beforeOverlayUpdateTexImage()} to be called before the {@link SurfaceTexture#updateTexImage()} call * - {@link #end()} to release and bring things back to normal state @@ -90,22 +91,22 @@ import com.otaliastudios.cameraview.preview.RendererThread; * finally the {@link EglViewport} drawing operations should be synchronized with a lock. * * REFERENCES + * https://github.com/natario1/CameraView/issues/514 * https://android.googlesource.com/platform/frameworks/native/+/5c1139f/libs/gui/SurfaceTexture.cpp + * I can see here that SurfaceTexture does indeed call glBindTexture with the same parameters whenever + * updateTexImage is called, but it also does other gl stuff first. This other gl stuff might be + * breaking when we don't have a bound texture on some specific hardware implementation. */ public class Issue514Workaround { - private final int cameraTextureId; + private final int textureId; - public Issue514Workaround(int cameraTextureId) { - this.cameraTextureId = cameraTextureId; + public Issue514Workaround(int textureId) { + this.textureId = textureId; } public void beforeOverlayUpdateTexImage() { - bindTexture(cameraTextureId); - } - - public void afterOverlayGlDrawn() { - bindTexture(cameraTextureId); + bindTexture(textureId); } public void end() { diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/overlay/OverlayDrawer.java b/cameraview/src/main/java/com/otaliastudios/cameraview/overlay/OverlayDrawer.java index 16b23aed..317bb749 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/overlay/OverlayDrawer.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/overlay/OverlayDrawer.java @@ -47,17 +47,22 @@ public class OverlayDrawer { private Issue514Workaround mIssue514Workaround; private final Object mIssue514WorkaroundLock = new Object(); - public OverlayDrawer(@NonNull Overlay overlay, @NonNull Size size, int cameraTextureId) { + public OverlayDrawer(@NonNull Overlay overlay, @NonNull Size size) { mOverlay = overlay; mViewport = new EglViewport(); mTextureId = mViewport.createTexture(); mSurfaceTexture = new SurfaceTexture(mTextureId); mSurfaceTexture.setDefaultBufferSize(size.getWidth(), size.getHeight()); mSurface = new Surface(mSurfaceTexture); - mIssue514Workaround = new Issue514Workaround(cameraTextureId); + mIssue514Workaround = new Issue514Workaround(mTextureId); } - + /** + * Should be called to draw the {@link Overlay} on the given {@link Overlay.Target}. + * This will provide a working {@link Canvas} to the overlay and also update the + * drawn contents to a GLES texture. + * @param target the target + */ public void draw(@NonNull Overlay.Target target) { try { final Canvas surfaceCanvas = mSurface.lockCanvas(null); @@ -74,20 +79,34 @@ public class OverlayDrawer { mSurfaceTexture.getTransformMatrix(mTransform); } + /** + * Returns the transform that should be used to render the drawn content. + * This should be called after {@link #draw(Overlay.Target)} and can be modified. + * @return the transform matrix + */ public float[] getTransform() { return mTransform; } + /** + * Renders the drawn content in the current EGL surface, assuming there is one. + * Should be called after {@link #draw(Overlay.Target)} and any {@link #getTransform()} + * modification. + */ public void render() { synchronized (mIssue514WorkaroundLock) { mViewport.drawFrame(mTextureId, mTransform); } - // mIssue514Workaround.afterOverlayGlDrawn(); - // mIssue514Workaround.afterOverlayGlDrawn(); } + /** + * Releases resources. + */ public void release() { - mIssue514Workaround.end(); + if (mIssue514Workaround != null) { + mIssue514Workaround.end(); + mIssue514Workaround = null; + } if (mSurfaceTexture != null) { mSurfaceTexture.release(); mSurfaceTexture = null; diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/picture/SnapshotGlPictureRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/picture/SnapshotGlPictureRecorder.java index c4c63045..b1e286af 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/picture/SnapshotGlPictureRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/picture/SnapshotGlPictureRecorder.java @@ -119,7 +119,7 @@ public class SnapshotGlPictureRecorder extends PictureRecorder { Matrix.setIdentityM(mTransform, 0); if (mHasOverlay) { - mOverlayDrawer = new OverlayDrawer(mOverlay, mResult.size, textureId); + mOverlayDrawer = new OverlayDrawer(mOverlay, mResult.size); } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java index 37b7d1cb..e1eaba7a 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java @@ -93,7 +93,7 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram public void onRendererTextureCreated(int textureId) { mTextureId = textureId; if (mHasOverlay) { - mOverlayDrawer = new OverlayDrawer(mOverlay, mResult.size, textureId); + mOverlayDrawer = new OverlayDrawer(mOverlay, mResult.size); } }