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

[DON'T MERGE] fontSize/scale handler switch management (scaling refactor part 6 - final) #214

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Nov 28, 2019

This PR adds a ScaleGestureDetector and a mechanism to switch between handling pinch-to-zoom with scale or view's size (width / height) when each of their maximum / minimum are reached.

  • if user is zooming in (and size is smaller than maximum), first we augment the view's size. Once it reaches the fontSize maximum, we switch to augmenting the view's scale.

  • if user is zooming out (and scale is greater than minimum), we first reduce the scale. Once scale reaches its minimum (1.0) we switch to resizing the view (the fontSize will be adapted correspondingly by the AutoResizeTextView).

  • most of the switching logic done in 76f245f

Important drawbacks and considerations:

  • had to get the LAYER_TYPE_SOFTWARE back in in order to avoid having the view disappear and fail rendering 😞 8489100
  • the change from one handler to the other could use some ironing to make it smoother, there's a small "vibrating" glitch when we switch from one state to the other, especially the first time you zoom into an emoji (the subsequent times seems to work better), and also sometimes when the emoji is left on the edge (like, reaching maximum fontSize) and start a gesture from there it feels like not responding to pinch to zoom gesture (the emoji does move and rotate but is not zoomed).
  • checking develop current state of things on a Samsung J2 shows this doesn't really add much more image quality - it is kept a bit better as we expand the emoji up to it's best maximum fontSize available, but then switching to the scale handler happens quickly enough as to not notice much gain there. Videos here https://cloudup.com/ckH_e_69vAL (you can see develop is there first on the left, and this branch on the right).

Leaving the PR open for further studying / checks but overall I think it's worth spending time on other areas of the app, given this approach after all doesn't seem to make things much better.

@peril-automattic
Copy link

peril-automattic bot commented Nov 28, 2019

You can test the changes on this Pull Request by downloading the APK here.

@mzorz mzorz changed the title WIP - fontSize/scale handler switch management fontSize/scale handler switch management Nov 29, 2019
@mzorz mzorz changed the title fontSize/scale handler switch management fontSize/scale handler switch management (scaling refactor part 6) Nov 29, 2019
@mzorz mzorz changed the title fontSize/scale handler switch management (scaling refactor part 6) fontSize/scale handler switch management (scaling refactor part 6 - final) Nov 29, 2019
@mzorz mzorz changed the title fontSize/scale handler switch management (scaling refactor part 6 - final) [DON'T MERGE] fontSize/scale handler switch management (scaling refactor part 6 - final) Dec 2, 2019
@mzorz mzorz closed this Dec 31, 2019
@mzorz mzorz deleted the issue/207-fontsize-scaling-callback-scale-switcher branch July 29, 2020 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant