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

DefaultTimeBar ignores touch transformations #7303

Closed
AChep opened this issue Apr 28, 2020 · 4 comments
Closed

DefaultTimeBar ignores touch transformations #7303

AChep opened this issue Apr 28, 2020 · 4 comments
Assignees
Labels

Comments

@AChep
Copy link
Contributor

AChep commented Apr 28, 2020

private Point resolveRelativeTouchPosition(MotionEvent motionEvent) {

Steps to reproduce:

  1. Create PlayerView with a default time bar.
  2. Create a container that transforms every touch event by translating it by 100 px.
  3. DefaultTimeBar ignores the translation.

Steps to fix:
I believe that simply replacing the:

  private Point resolveRelativeTouchPosition(MotionEvent motionEvent) {
    getLocationOnScreen(locationOnScreen);
    touchPosition.set(
        ((int) motionEvent.getRawX()) - locationOnScreen[0],
        ((int) motionEvent.getRawY()) - locationOnScreen[1]);
    return touchPosition;
  }

with

  private Point resolveRelativeTouchPosition(MotionEvent motionEvent) {
    touchPosition.set(
        (int) motionEvent.getX(),
        (int) motionEvent.getY());
    return touchPosition;
  }

should do the trick, but I don't know what was the reasoning behind obtaining the touch position manually.

@AChep AChep closed this as completed Apr 28, 2020
@AChep AChep reopened this Apr 28, 2020
AChep added a commit to AChep/ExoPlayer that referenced this issue Apr 29, 2020
@ojw28 ojw28 added bug and removed needs triage labels Apr 30, 2020
@ojw28
Copy link
Contributor

ojw28 commented Apr 30, 2020

I'm not sure what the reason is either. It's existed since the code was originally written, and wasn't commented on during internal code review. I assume you tested your pull request and it works, in which case I'm happy to merge.

@AChep
Copy link
Contributor Author

AChep commented Apr 30, 2020

@ojw28 let me double check it again.

@AChep
Copy link
Contributor Author

AChep commented Apr 30, 2020

@ojw28 everything seem to work fine. Also removed no longer being used locationOnScreen variable.

@ojw28
Copy link
Contributor

ojw28 commented May 3, 2020

#7304 was merged.

@ojw28 ojw28 closed this as completed May 3, 2020
@google google locked and limited conversation to collaborators Jul 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants