SurfaceView onDraw, at times doesn't refresh itself before draw... why?

Question

I have a simple onDraw method as below (in 'SurfaceView'), where the 'startCount' will go from 1 to 360, the filled circle get drawn.

@Override
protected void onDraw(Canvas canvas) {
    super.onDraw(canvas);
    //canvas.drawColor(0xFFEEEEEE);
    if (startCount == 360) startCount= 0;
    canvas.drawArc(mShadowBounds,
            0, startCount, true, mPiePaint);
}

And after complete a cycle. It will continue refresh and redrawn itself again from the beginning like radar detector moving.

The odd things is, at times, after a complete cycle, it doesn't refresh, so the big black circle is shown instead.

Illustration in https://youtu.be/sc56FYUqV7M, the first 2 cycles is what I expect. However when it completes the second cycle, it remains all black moving forward, which is strange it didn't remove previous drawing. I'm expecting it to repeat itself like how it did on the second cycle.

This doesn't always happens after the second cycle. At times one should wait a while after many cycles before it happen, without any interference (i.e. not touching the device). Hard to predict when it will happen.

What's the cause of it? How to even debug on this issue?

(FYI. I could use the same algorithm on the 'onDraw' in 'View' class. This issue doesn't happen at all.)

Attached below the complete code for Custom Surface View and it's Thread code.

public class TimerSurfaceView extends SurfaceView implements SurfaceHolder.Callback {

    private Paint mPiePaint;
    private RectF mShadowBounds;
    private float diameter;

    int startCount = 0;
    private PanelThread thread;

    public TimerSurfaceView(Context context) {
        super(context);
        init();
    }

    public TimerSurfaceView(Context context, AttributeSet attrs) {
        super(context, attrs);
        init();
    }

    public TimerSurfaceView(Context context, AttributeSet attrs, int defStyleAttr) {
        super(context, attrs, defStyleAttr);
        init();
    }

    @TargetApi(VERSION_CODES.LOLLIPOP)
    public TimerSurfaceView(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) {
        super(context, attrs, defStyleAttr, defStyleRes);
        init();
    }

    private void init() {
        getHolder().addCallback(this);
        mPiePaint = new Paint(Paint.ANTI_ALIAS_FLAG);
        mPiePaint.setStyle(Paint.Style.FILL);
        mPiePaint.setColor(0xff000000);

        setZOrderOnTop(true);
        getHolder().setFormat(PixelFormat.TRANSLUCENT);

    }

    @Override
    protected void onSizeChanged(int w, int h, int oldw, int oldh) {
        super.onSizeChanged(w, h, oldw, oldh);

        // Account for padding
        float xpad = (float)(getPaddingLeft() + getPaddingRight());
        float ypad = (float)(getPaddingTop() + getPaddingBottom());

        float ww = (float)w - xpad;
        float hh = (float)h - ypad;

        // Figure out how big we can make the pie.
        diameter = Math.min(ww, hh);
        mShadowBounds = new RectF(0, 0, diameter, diameter);

    }

    @Override
    protected void onDraw(Canvas canvas) {
        super.onDraw(canvas);
        //canvas.drawColor(0xFFEEEEEE);
        if (startCount == 360) startCount= 0;
        canvas.drawArc(mShadowBounds,
                0, startCount, true, mPiePaint);
    }

    public void incrementCount() {
        startCount++;
    }

    @Override
    protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
        int minw = getPaddingLeft() + getPaddingRight() + getSuggestedMinimumWidth();
        int w = resolveSizeAndState(minw, widthMeasureSpec, 1);
        int h = resolveSizeAndState(MeasureSpec.getSize(w), heightMeasureSpec, 0);
        setMeasuredDimension(w, h);
    }

    @Override
    public void surfaceCreated(SurfaceHolder holder) {
        setWillNotDraw(false); //Allows us to use invalidate() to call onDraw()
        thread = new PanelThread(getHolder(), this); //Start the thread that
        thread.setRunning(true);                     //will make calls to
        thread.start();                              //onDraw()
    }

    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {

    }

    @Override
    public void surfaceDestroyed(SurfaceHolder holder) {
        // tell the thread to shut down and wait for it to finish
        // this is a clean shutdown
        if (thread != null) {
            boolean retry = true;
            while (retry) {
                try {
                    thread.setRunning(false);                //Tells thread to stop
                    thread.join();                           //Removes thread from mem.
                    retry = false;
                } catch (InterruptedException e) {
                    // try again shutting down the thread
                }
            }
            thread = null;
        }
    }
}

And the Thread class

class PanelThread extends Thread {
    private SurfaceHolder surfaceHolder;
    private TimerSurfaceView panel;
    private boolean startRunning = false;


    public PanelThread(SurfaceHolder surfaceHolder, TimerSurfaceView panel) {
        this.surfaceHolder = surfaceHolder;
        this.panel = panel;
    }


    public void setRunning(boolean run) { //Allow us to stop the thread
        startRunning = run;
    }


    @Override
    public void run() {
        Canvas c;
        while (startRunning) {     //When setRunning(false) occurs, startRunning is
            c = null;      //set to false and loop ends, stopping thread
            try {
                c = surfaceHolder.lockCanvas(null);
                synchronized (surfaceHolder) {
                    //Insert methods to modify positions of items in onDraw()
                    panel.incrementCount();
                    panel.postInvalidate();
                }
            } finally {
                if (c != null) {
                    surfaceHolder.unlockCanvasAndPost(c);
                }
            }

        }
    }
}

Answer 1

You're mixing up two different approaches to drawing.

The SurfaceView has two parts, the Surface and the View. The View part is normally just a transparent rectangle that lets you "see through" the View UI layer. The Surface is an independent layer that, by default, sits behind the View UI layer.

By subclassing SurfaceView and defining an onDraw() method, you're treating it like a custom View. You're drawing on the View part, but you don't appear to be erasing it first, so it should still be mostly transparent. So you'll see whatever you draw on the View on top of whatever appears on the Surface.

It doesn't look like you're actually drawing on the Surface, so that should just be black.

You're creating a separate thread and calling postInvalidate() as fast as you can. The fact that you're locking and unlocking the Surface doesn't change much except to cause you to submit an un-drawn buffer for composition on the Surface layer, which is a lot of work considering all it really gets you is a 60fps pace. The invalidate and draw is being performed on the main UI thread, and it's the View system that calls your onDraw() method.

You should go one way or the other. Either do your drawing from the PanelThread with an explicit draw call (in which case I'd recommend you don't subclass SurfaceView at all), or ditch SurfaceView and just use a custom View instead. Once you get that sorted out things should start to behave in ways that make more sense.

Update: I suspect the reason your updating appears to stop is because you have a race condition. One thread does this:

startCount++;

and another thread that does this:

if (startCount == 360) startCount= 0;

This only behaves correctly if the main UI thread (executing the second line of code) sees every increment. If the main UI thread gets stalled and misses an update, it might see startCount at 359, and then at 361. At that point startCount will just keep incrementing without being reset, and your draw function will fill the entire circle because:

If the sweep angle is >= 360, then the oval is drawn completely.

Changing the test to restart when startCount >= 360 will probably fix things, but it would be best to avoid distributing your logic across threads.