Android drag ImageView very laggy

Question

I am trying to implement a touch-draggable ImageViewin an Android app. There are loads of sample codes for this on SO and elsewhere, and all seem to be along the lines of this one.

I tried something similar to this, but it was very laggy and I kept getting the warning

Choreographer﹕ Skipped XX frames!  The application may be doing too much work on its main thread.

I followed the Android documentation advice on this and tried to do the work in a separate thread, but this hasn't solved the issue.

Basically I register an OnTouchListener to my view, then in the onTouch() method I start a new thread to try and do the work (the final update of the layout params is sent back to the UI thread for processing using View.post()).

I can't see how to achieve this by doing any less work on the main thread. Is there some way to fix my code? Or is there perhaps a better approach altogether?

The code:

    private void setTouchListener() {
        final ImageView inSituArt = (ImageView)findViewById(R.id.inSitu2Art);
        inSituArt.setOnTouchListener(new View.OnTouchListener() {

            @Override
            public boolean onTouch(View v, MotionEvent event) {

                final MotionEvent event2 = event;

                new Thread(new Runnable() {
                    @Override
                    public void run() {
                        FrameLayout.LayoutParams layoutParams = (FrameLayout.LayoutParams) inSituArt.getLayoutParams();
                        switch (event2.getAction()) {
                            case MotionEvent.ACTION_DOWN: {
                                int x_cord = (int) event2.getRawX();
                                int y_cord = (int) event2.getRawY();
                                // Remember where we started
                                mLastTouchY = y_cord;
                                mLastTouchX = x_cord;
                            }
                            break;
                            case MotionEvent.ACTION_MOVE: {
                                int x_cord = (int) event2.getRawX();
                                int y_cord = (int) event2.getRawY();
                                float dx = x_cord - mLastTouchX;
                                float dy = y_cord - mLastTouchY;

                                layoutParams.leftMargin += dx;
                                layoutParams.topMargin += dy;
                                layoutParams.rightMargin -= dx;


                                final FrameLayout.LayoutParams finalLayoutParams = layoutParams;
                                inSituArt.post(new Runnable() {
                                    @Override
                                    public void run() {
                                        inSituArt.setLayoutParams(finalLayoutParams);
                                    }
                                });


                                mLastTouchX = x_cord;
                                mLastTouchY = y_cord;
                            }
                            break;
                            default:
                                break;
                        }

                    }
                }).start();
                return true;
            }
        });
    }

Answer 1

I found a real solution after a long time.

You should use a Drawable instead of a Bitmap.

override fun onDraw(canvas: Canvas?) {
        canvas?.save()
        canvas?.concat(myCustomMatrix)
        mDrawable?.draw(canvas!!)
        canvas?.restore()
    }

---

move image (dx, dy)

myCustomMatrix.postTranslate(dx, dy)
invalidate()

Answer 2

with the new Thread you just added more choppiness to the drag movement. As mentioned by @JohnWhite onTouch is called lots of times in a row, and creating and firing new threads like this is REALLY, REALLY bad.

The general reason why you were encountering those issues to begin with is because the example code that u were following, event though it works, it's a bad unoptimised code.

every time you're calling setLayoutParams the system have to execute a new complete layout pass, and that is a lot of stuff to be doing continuously.

I won't completely write the code for you, but I'll point you to possible alternatives.

First remove all the thread logic you used. You're not really executing any complex calculation, everything can run on the UI-thread.

1. use offsetTopAndBottom and offsetLeftAndRight to move your ImageView on the screen.
2. use ViewTransformations with setTranslationX (and Y)

3. change your ImageView to match_parent and create a custom ImageView. In this custom view you'll override the onDraw method to move the image right before you draw it. Like this:

    @Override
    public void onDraw(Canvas canvas) {
      int saveCount = canvas.save();
      canvas.translate(dx, dy); // those are the values calculated by your OnTouchListener
      super.onDraw(canvas); // let the image view do the normal draw
      canvas.restoreToCount(saveCount); // restore the canvas position
    }
   

this last option is very interesting as it's not changing the layouts in absolutely no way. It's only making the ImageView take care of the whole area and moving the drawing to the correct position. Very efficient way of doing.

on some of those options you'll need to call "invalidate" on the view, so the draw can be called again.

Answer 3

onTouch is an event that fires repeatedly, and you are asking the view to redraw itself in a new location every time onTouch is fired. Draw is a very expensive operation for your application - it should never be asked to do this repeatedly in the regular view hierarchy. Your goal should be to reduce the number of draw commands that occur, so I would set a simple modulus operation that only allows it to execute the move every, say, 10 onTouch events. You will have a tradeoff eventually where it looks choppy because it isn't occurring every frame if you put your modulus too high, but there should be a balance in the 5-30 range.

Above in your class, you'll want something like:

static final int REFRESH_RATE = 10; //or 20, or 5, or 30, whatever works best

int counter = 0;

And then in your touch event:

case MotionEvent.ACTION_MOVE: {
    counter += 1;
    if((REFRESH_RATE % counter) == 0){ //this is the iffstatement you are looking for
      int x_cord = (int) event2.getRawX();
      int y_cord = (int) event2.getRawY();
      float dx = x_cord - mLastTouchX;
      float dy = y_cord - mLastTouchY;
      layoutParams.leftMargin += dx;
      layoutParams.topMargin += dy;
      layoutParams.rightMargin -= dx;

      final FrameLayout.LayoutParams finalLayoutParams = layoutParams;
      inSituArt.post(new Runnable() {
        @Override
        public void run() {
           inSituArt.setLayoutParams(finalLayoutParams);
        }
      });

      mLastTouchX = x_cord;
      mLastTouchY = y_cord;
    }
    counter = 0;
}