Skip to content
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

Improve the photos list animations (600$ Bounty) (aka gallery pinch/zoom) #83

Closed
3 of 4 tasks
ghorbani-m opened this issue Jan 24, 2022 · 9 comments
Closed
3 of 4 tasks
Assignees
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed High Priority High priority. Only one open and unassigned high priority is allowed at each time.
Milestone

Comments

@ghorbani-m
Copy link
Collaborator

ghorbani-m commented Jan 24, 2022

What issue do we need to solve?

We are going to improve the photos list animations when pinching and zooming (in/out) the list to change the number of columns (View Type).
Since the Photos App is going to load a large list of images we need to use a RecyclerView to manage a large list, for this purpose we used the RecyclerListView open-source project to handle this issue.

It works as expected for a large list, but there are some challenges to handling the image's animations when we need to change the view type.

Actual Behavior

The animation starts after a while client pinch the view and the whole of the animation will be done.

photos.mp4

Expected Behavior?

The client has to be able to control the animation in the middle of the way.

20220116_135942_edited.mov

Steps to Reproduce

  1. Clone the Master branch
  2. Build the project
  3. Pinch the list

Platform:

Where is this issue occurring?

  • iOS
  • Android

Tasks

  • The image flickering issue on changing the view is fixed.
  • There is a bug when the user scroll to the end of list and try to change the gridview columns
@ehsan6sha ehsan6sha changed the title Improve the photos list animations Improve the photos list animations (600$ Bounty) Feb 1, 2022
@amanforindia
Copy link

amanforindia commented Feb 10, 2022

Hi team, @ghorbani-m:

  1. The pinch shown in the video (actual behavior) doesn't work in the latest master branch. I can get the number of columns to change by clicking on the button at the top though. Was the pinch done in an earlier commit that I can checkout to?
  2. Currently, each asset is a separate section for recyclerlistview. Instead my idea is to have the day's images which are all combined in 1 section into a common section. Now, we make numCols an animated.value (or useSharedValue of reanimated but I haven't worked with reanimated too much yet), give canChangeSize={true} to recyclerlistview and in recyclerlistview's height/width computation for the section, we only calculate the combined section's height (width being 100%). For achieving this we may need to use externalScrollView or onSizeChanged callback or forceNonDeterministicRendering but I need to investigate more about that. Inside the specific day's section, we can implement a custom grid which is in sync with the numCols animated variable.

Let me know if this approach sounds good!

@ehsan6sha to answer your question about "Are we still using one recyclerlistview for 2,3,4 and 5 column modes?", yes we would still use that but the column number becomes an animated value with possible floating point values rather than integers.

@ghorbani-m
Copy link
Collaborator Author

ghorbani-m commented Feb 11, 2022

@amanforindia Thanks for your proposal,

  1. I have checked it out one more time, and it works for me, I have tested it on the android platform (samsung phone), could you please make sure you got the latest changes from the Master branch.
  2. The drawback which I could see is using a bunch of images in a section, the recyclelistview's idea is recycling the components (section) instead of creating them to prevent performance issue on scrolling, but in your approach you need to recreate a list of images inside each section when the section is recycling, I believe it causes performance issue.

@skydevht
Copy link

A solution would be to precalculate the image dimensions for k columns, k ranging from 1 to n. Then, we implement a custom grid layout manager, rearranging the views around when k changes. The difference with the actual behavior, is to control the animation part directly with our animation control value. I guess the current layout manager calculates the new layout when k changes and does the animation. The trick is to precalculate all the possible layouts, then let it evolve based on the current control value.

@amanforindia
Copy link

Hi @skydevht, @ghorbani-m for a smooth experience I think the height of the container which has the current day's images needs to scale linearly and gradually as the pinch goes from 1 column to 2 columns (accounting for fractional values). Also, the images themselves have to scale fractionally and not just get rearranged. Maybe I'm missing something here but I'm not exactly clear yet how I would achieve this without any tradeoff in performance.

@ghorbani-m
Copy link
Collaborator Author

ghorbani-m commented Feb 12, 2022

@skydevht Thanks for your description
You are right, the solution is to control the transition's animation while pinching the screen, but the question is how can we accomplish that? at the Layout Manager, you just define the section's dimensions, and there is no way to determine the section's position. somehow if you can manage the image's position from the outside of "Recyclelistview" the task is almost done.

@amanforindia You know, the matter is not the type of section's animation, the matter is how we can control the animation will pinching the screen,

A workaround that I am thinking of is if each section (image) knows about the next position and dimension based on the pinch type (Pinch in/out) then we might manage the next transition's animation based on pinch value.

Another way is to change the animation's type which was implemented before by @ehsan-git-dev , like this:

https://github.com/functionland/photos/blob/main/assets/demo/BOX_Photos_pinchzoom_to_switch_between_modes.gif)

The above animation was implemented by 3 Recycleviewlist with different colNums at the same time, Also it is acceptable if we could manage it only with one Recycleviewlist.

@skydevht
Copy link

A possible implementation would be to use the pinch-zoom event value as our control. The possible state would be the current number of columns and the next amount (lower or greater) of columns. When the interaction end, we can select the closest state to do a finishing animation. As the layout of the items will change the number of rows, we need to control the position of the additional elements that are presents (date header). If we need one interaction to switch between a greater number of columns, we can add states in between.

This solution requires direct access to the position of the items. This will likely require us to write a custom manager instead of relying on the one provided by the recycler list view library. The photos sections are the only one that need their scale to be animated. I don't know yet if we can hijack that part of the layout calculation. The library has a VirtualRenderer that manage the visible views, but I hope we only need to provide their position, and not manage the creation and destruction too.

@skydevht
Copy link

@ghorbani-m you need to overwrite whatever is controlling the section position. If it can automatically adjust that based on the section's dimension, you need to redefine what is a section and go from there.

@ghorbani-m
Copy link
Collaborator Author

@skydevht The current implementation uses the pinch-zoom value and column number state (numCols) and then decides based on pinch scale value to increase or decrease the numCols' value and finally in the Layout Provider calculates the section dimension based on numCols' value.

Regarding your solution "This solution requires direct access to the position of the items", actually we have access to each section's position by "onItemLayout" event, and we are able to animate the sections but finally, somehow we need to tell the RecyclerListView that the sections positions are changed and recalculate the sections or recreate the new sections that now they are in the ViewPort.

As you mentioned we need to controll the section position but there is not any interface in the recycleviewlist to force reclaculate the layouy smoothly.

It seems if we want to use RecycleViewList with layout animation we need to add some features in the Recycleviewlist repo.

@gitaaron gitaaron added this to the Photos v0.1 milestone Mar 10, 2022
@gitaaron gitaaron changed the title Improve the photos list animations (600$ Bounty) Improve the photos list animations (600$ Bounty) (aka gallery pinch/zoom) Mar 10, 2022
@emadbaqeri emadbaqeri added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed High Priority High priority. Only one open and unassigned high priority is allowed at each time. labels Mar 12, 2022
@gitaaron
Copy link

Also related - #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed High Priority High priority. Only one open and unassigned high priority is allowed at each time.
Projects
Status: Done-previous-sprints
Development

No branches or pull requests

5 participants