Search code examples
androidmultithreadingimageviewtouch

Android drag ImageView very laggy


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;
            }
        });
    }

Solution

  • 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.