search

markusfisch / ShaderEditor

Android app to create GLSL shaders and use them as live wallpaper
https://play.google.com/store/apps/details?id=de.markusfisch.android.shadereditor
MIT License
924 stars 137 forks source link

Refactor to CameraX library #201

Closed AntonPieper closed 3 months ago

AntonPieper commented 3 months ago

CameraX is the recommended API by the android project. I first started with the Camera2 API, however I saw that some methods were already deprecated since API level 30 and would need to be refactored again when the target API level is raised further.

markusfisch commented 3 months ago

Great work! 💪 CameraX is the way to go 👍

I just have two minor issues, one of which is the change in MainActivity I have commented on (see the review above). Should be trivial to resolve.

The other one is that the app does crash immediately after granting camera permission. Reproduce with a fresh install, "Load sample", pick "Back camera", and grant permission. Now you should see something like this in logcat:

java.lang.IllegalArgumentException: No supported surface combination is found for camera device - Id : 0.  May be attempting to bind too many use cases. Existing surfaces: [AttachedSurfaceInfo{surfaceConfig=SurfaceConfig{configType=PRIV, configSize=PREVIEW, streamUseCase=0}, imageFormat=34, size=720x480, dynamicRange=DynamicRange@5d654b6{encoding=SDR, bitDepth=8}, captureTypes=[PREVIEW], implementationOptions=androidx.camera.camera2.impl.Camera2ImplConfig@a491a43, targetFrameRate=null}, AttachedSurfaceInfo{surfaceConfig=SurfaceConfig{configType=PRIV, configSize=PREVIEW, streamUseCase=0}, imageFormat=34, size=720x480,dynamicRange=DynamicRange@5d654b6{encoding=SDR, bitDepth=8}, captureTypes=[PREVIEW], implementationOptions=androidx.camera.camera2.impl.Camera2ImplConfig@b5bea49, targetFrameRate=null}] New configs: [androidx.camera.core.impl.PreviewConfig@bb1186f]                 at androidx.camera.lifecycle.LifecycleCameraRepository.bindToLifecycleCamera(LifecycleCameraRepository.java:302)       at androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle(ProcessCameraProvider.java:652)
    at androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle(ProcessCameraProvider.java:387)
    at de.markusfisch.android.shadereditor.hardware.CameraListener.lambda$register$2$de-markusfisch-android-shadereditor-hardware-CameraListener(CameraListener.java:88)
AntonPieper commented 3 months ago

I will look into the bindToLifecycle issue later👍 Thanks for the review! Though, I think your comment on the MainActivity did not get through, I can't see a comment🤔 I did change the cursor to use a try with resources, because it crashed due to an unclosed cursor, but I see that the cursor could outlive the scope, so the real Dix would be to actually find the missing close inside the called method

markusfisch commented 3 months ago

Ah, sorry, I missed starting the review process. Now you should see my comment.

AntonPieper commented 3 months ago

It should be fixed now, but I also see that setTargetResolution is deprecated as of the newest CameraX library version... I tried using the replacement (setResolutionSelector), but the aspect ratio seems to not be respected correctly.

markusfisch commented 3 months ago

Thanks again! 👍

About the deprecated setTargetResolution, maybe there's a way to use ResolutionSelector for the same result, but I'll have to check that.

AntonPieper commented 3 months ago

The best way would be to add a mat3 transformation matrix that does not only rotate, flip and translate the camera, but also correct the aspect ratio. This would eventually be a breaking change though. It could be added as a new uniform and the other two could have a deprecation warning in their description until they are removed.