1

I have a problem with a simple android application. It has a SurfaceView with simple drawing, but activity orientation sometimes seems not to be changed after a screen rotation.

This is how an activity looks in the portrait mode:

landscape mode: enter image description here

but sometimes, when I rotate the screen, an activity looks like this in the portrait mode: enter image description here

MainActivity.java:

package com.example.myapplication;

import androidx.appcompat.app.AppCompatActivity;

import android.os.Bundle;
import android.view.SurfaceHolder;
import android.view.SurfaceView;

public class MainActivity extends AppCompatActivity implements SurfaceHolder.Callback
{
    private Thread mGameThread;
    private GameApp mGameApp;

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        SurfaceView mainView = (SurfaceView)findViewById(R.id.mainView);
        SurfaceHolder holder = mainView.getHolder();
        holder.addCallback(this);

        mGameApp = new GameApp(getResources(), holder);
    }

    @Override
    public void surfaceCreated(SurfaceHolder holder)
    {
        mGameThread = new Thread(new Runnable()
        {
            @Override
            public void run()
            {
                mGameApp.run();
            }
        });
        mGameThread.start();
    }

    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height)
    {
        mGameApp.setSurfaceSize(width, height);
    }

    @Override
    public void surfaceDestroyed(SurfaceHolder holder)
    {
        boolean retry = true;
        mGameApp.setRunning(false);
        while (retry)
        {
            try
            {
                mGameThread.join();
                retry = false;
            }
            catch (InterruptedException e)
            {
            }
        }
    }
}

GameApp.java:

package com.example.myapplication;

import android.content.res.Resources;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.drawable.Drawable;
import android.view.SurfaceHolder;

public class GameApp
{
    private Resources mResources;
    private SurfaceHolder mSurfaceHolder;
    private int mCanvasHeight = 1;
    private int mCanvasWidth = 1;
    private volatile boolean mRun = false;

    public GameApp(Resources resources, SurfaceHolder surfaceHolder)
    {
        mResources = resources;
        mSurfaceHolder = surfaceHolder;
    }

    public void setSurfaceSize(int width, int height)
    {
        synchronized (mSurfaceHolder)
        {
            mCanvasWidth = width;
            mCanvasHeight = height;
        }
    }

    public void run()
    {
        setRunning(true);

        while (mRun)
        {
            Canvas canvas = null;
            try
            {
                canvas = mSurfaceHolder.lockCanvas(null);

                synchronized (mSurfaceHolder)
                {
                    if (mRun && canvas != null)
                    {
                        draw(canvas);
                    }
                }
            }
            finally
            {
                if (canvas != null)
                {
                    mSurfaceHolder.unlockCanvasAndPost(canvas);
                }
            }
        }
    }

    public void setRunning(boolean b)
    {
        mRun = b;
    }

    private void draw(Canvas canvas)
    {
        canvas.drawColor(Color.GREEN);

        Drawable cellImage = mResources.getDrawable(R.drawable.cell);

        final float cellWidth = mCanvasWidth / 6;
        final float cellHeight = mCanvasHeight / 6;
        for (int i = 0; i < 6; i++)
        {
            for (int j = 0; j < 6; j++)
            {
                float x = i * cellWidth;
                float y = j * cellHeight;

                cellImage.setBounds(Math.round(x), Math.round(y), Math.round(x + cellWidth), Math.round(y + cellHeight));
                cellImage.draw(canvas);
            }
        }
    }
}
Denis
  • 2,786
  • 1
  • 14
  • 29

1 Answers1

2

Eventual answer:

The problem is that you are doing a while loop in your GameApp's thread that locks the canvas and then unlocks without any long blocking or sleep in between. The SurfaceHolder#lockCanvas documentation states:

If null is not returned, this function internally holds a lock until the corresponding unlockCanvasAndPost(Canvas) call, preventing SurfaceView from creating, destroying, or modifying the surface while it is being drawn.

So this means that the destroy code which is run from main thread needs to run between the unlockCanvasAndPost and the next lockCanvas. But since you have no sleep or even other code in between (except for the while condintion check), the chance of this happening is very small, and - depending on the device - could practically take forever.

To fix this, put a sleep in your game app to achieve the wanted framerate, in it's most simple from this could look like this.

class GameApp

    ...
    while (mRun)
        {
            Canvas canvas = null;
            try
            {
                canvas = mSurfaceHolder.lockCanvas(null);

                synchronized (mSurfaceHolder)
                {
                    if (mRun && canvas != null)
                    {
                        draw(canvas);
                    }
                }
            }
            finally
            {
                if (canvas != null)
                {
                    mSurfaceHolder.unlockCanvasAndPost(canvas);
                }
            }
            // Add some sleep time depending on how fast you want to refresh
            try {
                Thread.sleep(1000/60); //~60 fps
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }

Original answer 1: What could have helped in case of handling orientation changes

Seems like the surface is not always re-drawn on a config change.

Try overriding the onConfigurationChanged method of the activity and then triggering a re-layout of the surface

in MainActivity.java:

...
@Override
public void onConfigurationChanged(Configuration newConfig) {
   // You should save (SurfaceView)findViewById(R.id.mainView) in a field for better performance, but I'm putting it here for shorter code.
   SurfaceView mainView = (SurfaceView)findViewById(R.id.mainView);
   mainView.invalidate();
   mainView.requestLayout();
}
...

More info on these methods nicely explained here: Usage of forceLayout(), requestLayout() and invalidate()

Original answer 2: A way to check if your threads are blocked

Another possibility is that you have a thread lock issue on your main thread.

For checking that you could change your activity like this:

public class MainActivity extends AppCompatActivity implements SurfaceHolder.Callback {
    private Thread mGameThread;
    private GameApp mGameApp;
    private static View currMainView;
    private static Thread logPosterThread = new Thread(new Runnable() {
        volatile boolean mainLogDone = true;
        @Override
        public void run() {
            Runnable mainThreadLogger = new Runnable() {
                @Override
                public void run() {
                    Log.d("THREAD_CHECK", "Main Thread is ok");
                    mainLogDone = true;
                }
            };
            while (true) {
                try {
                    int sleepTime = 1000;
                    Thread.sleep(sleepTime);
                   
                    if (currMainView != null) {
                        if (mainLogDone) {
                            mainLogDone = false;
                            Log.d("THREAD_CHECK", "Main Thread doing post");

                            currMainView.post(mainThreadLogger);
                        } else {
                            Log.w("THREAD_CHECK", "Main Thread did not run the runnable within " + sleepTime + "ms");
                            mainLogDone = true;
                        }
                    }
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    });
    static {
        logPosterThread.start();
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        final SurfaceView mainView = (SurfaceView) findViewById(R.id.mainView);
        currMainView = mainView;
        SurfaceHolder holder = mainView.getHolder();
        holder.addCallback(this);
        mGameApp = new GameApp(getResources(), holder);
    }
    @Override
    public void onPause() {
...

And then see if those logs still get printed with 1000ms in between when the issue happens.

When you try this, you will see that actually the main thread is hanging when this happens.

francis duvivier
  • 2,056
  • 11
  • 17
  • I have tried this, but onConfigurationChanged is never called because I didn't change manifest for treatment of orientation changes as config changes. However, I realized, in case when the layout is not updated during screen rotation, methods onStop of Activity and onSurfaceDestroyed of SurfaceHolder.Callback are never called, and I don't know how right is such behavior – Denis Sep 03 '20 at 07:43
  • Is the oncreate of the activity still called in that case? – francis duvivier Sep 03 '20 at 08:52
  • No, just like onDestroy isn't called for a previous activity – Denis Sep 03 '20 at 22:48
  • 1
    In that case, I suspect your main thread is blocked sometimes on a rotation change. Could be a deadlock, but I cannot spot it immediately in you code. Check if your `mGameThread.join()` is 100% safe and whether you have some other `synchronized` blocks or `join()` calls which could cause your main thread to be blocked indefinitely. Or to test that hypothesis, you can post a runnable to the main thread that logs something eg. every 100ms, and then see if that keeps working or not. – francis duvivier Sep 04 '20 at 00:23
  • `mGameThread.join()` is called in `surfaceDestroyed`, but `surfaceDestroyed` itself is not called in that case, so I don't think the problem is in a thread. I have uploaded the whole project (it's small) demonstrating the problem https://easyupload.io/tazvu3, maybe it might help – Denis Sep 04 '20 at 13:49
  • 1
    Did you try the logs on main thread? And I tried to reproduce it, but it happens for a very short moment with me, is that the same with you or does it stay stuck when that happens? – francis duvivier Sep 04 '20 at 17:05
  • 1
    I checked it out as I described and found that the threads indeed arrive in some dead lock in some way and it's because you are endlessly looping and posting to the surface without any stop while this logic should happen once per frame. I updated my answer. – francis duvivier Sep 04 '20 at 18:08
  • Yes, it stay stuck forever. I have already debugged it, and the loop indeed works while the layout is freezed. Tomorrow I will try to add a sleep to the loop, but I still don't understand how it may block, because even without a sleep it releases the `synchronized` block in every iteration – Denis Sep 04 '20 at 23:12
  • 1
    The `synchronized` block or `thread.join` in your code is indeed not to blame. But what you should know is that the `lockCanvas` method also blocks the `surfaceDestroyed` from being called until the `unlockCanvasAndPost` is called plus that main thread needs to wait for this surface to update. In your while look, you immediately lock after unlocking, so the chance that the unlock code is being run in between those two statements is pretty small and depending on the device and threading implementation, could practically take forever. – francis duvivier Sep 05 '20 at 22:54
  • I have tried to add a sleep in the loop and it seems there's no such problem anymore. But now in rare cases the whole device is freezed when I pause the app by switching between applications :( – Denis Sep 06 '20 at 11:41
  • Hmm, where did you put the sleep and how much? And any error logs? – francis duvivier Sep 06 '20 at 12:51