Photo editor module refactor #11
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tion-photo-editor-refactor
mzorz
changed the base branch from
app/foundation-photo-editor-lib-module
to
app/foundation
…ecycle and manage camera / video player fragments switching
…d, and video player modes
7 tasks
aforcier
reviewed
Aug 2, 2019
| @@ -39,7 +39,10 @@ internal class TextureRenderer { | |||
|
|
|||
| fun init() { | |||
| // Create program | |||
| mProgram = GLToolbox.createProgram(VERTEX_SHADER, FRAGMENT_SHADER) | |||
| mProgram = GLToolbox.createProgram( | |||
There was a problem hiding this comment.
Noticed there are a few m prefixes around the photoeditor lib, opened an issue to track cleaning this up (not necessary to do in this PR): #17.
aforcier
reviewed
Aug 2, 2019
| @@ -130,8 +137,12 @@ class PhotoEditorView : RelativeLayout { | |||
| ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.WRAP_CONTENT | |||
| ) | |||
| brushParam.addRule(RelativeLayout.CENTER_IN_PARENT, RelativeLayout.TRUE) | |||
| brushParam.addRule(RelativeLayout.ALIGN_TOP, imgSrcId) | |||
| brushParam.addRule(RelativeLayout.ALIGN_BOTTOM, imgSrcId) | |||
| brushParam.addRule(RelativeLayout.ALIGN_TOP, | |||
|
Looks good @mzorz ! Things feel a lot neater with this structure 💯 Nice use of the |
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #7
This refactor organizes things in a better way by doing some renames and bringing a new package structure:
The
com.automattic.photoeditor.viewspackage now contains all related files for:PhotoEditorView. This is the main view, where all the action happens..views.brush. This one contains all classes related to free form drawing.views.filter. This package contains all classes that are needed to apply filters on top of the background image. Filters are only applicable to static background images.views.background. Background can be eitherfixed(static image implemented byBackgroundImageViewor static image with filters applied on top withFilterImageView) orvideo(either pre-recorded video or camera preview mode). The latter is implemented by aTextureView..views.addedcontain the helper classes that hold information about all other Views the user is able to add (TextViewwhich is used for Text and emoji), andImageViewfor animated / static stickers).This PR also introduces a new class
BackgroundSurfaceManager. The state changes are consumed by this specialized class which is a LifecycleObserver https://developer.android.com/topic/libraries/architecture/lifecycle (also see https://github.com/googlesamples/android-architecture-components for samples)I think this new way makes things clearer, and the new class encapsulates the switching of the background view from static / video playing / camera preview modes in an efficient way.
To test:
For testing, I've added 3 options on the overflow menu:
The first one will request permissions as needed, and will render the camera preview as a background. You should still be able to use the normal added views (brush, emoji, text) normally, while having the camera preview as a background.
The second one switches back to the static background (orange background for now).
The third one makes the background SurfaceTexture visible, ready for video to be played.
Also note that the static background can't be changed (it's just the orange shapes for now), and the third option only makes the right surface available, but no videos are played at the moment, as this PR cares only about being able to manage and switch background surfaces as per #7. What this means is, if you use the camera preview first, then switching to "Video player" mode will show the last buffered still image from the camera preview (that's what the underlying TextureSurface contains). If you try it before testing camera preview, a blank canvas will be shown.
Notes:
getLifecycle()on AppCompatActivityjava.lang.NoSuchMethodError: No super method getLifecycle()Landroidx/lifecycle/Lifecycle;)