[3D] slow (refresh after?) sprite touching tests

[TheLogFather]

For more details, see demo project: https://scratch.mit.edu/projects/158676183/ The odd things seem to be that it's much worse when the clones have different looks (i.e. size/direction/effects) and that the slowdown occurs in the refresh after the actual touch tests (maybe? -see next comment below concerning non-refresh custom block). See also the extra notes (in Notes & Credits) giving various alternative tests to try out.

This became apparent after playing around with @rpglurker's breakout project which has coloured brick clones (using colour effect) that continually test if touching the ball sprite. It shows frequent 'pauses' while playing, which go away when toggling off Stage3D (on my MacBook, at least). A good way to see just how much difference it makes is to run with turbo (compare Stage3D vs normal).

[TheLogFather]

OK, I'm really confused now...

I put the simple touching test script ("if touching then / set var to YES!") into a custom block and that makes no difference (as you'd expect).

However, set it non-refresh and it means (most of) the measured time moves from the refresh timer to the touching timer. Yet that simple script has no loops, so I wouldn't expect disabling refresh to make any difference.

:/

[cwillisf]

Very strange... I know things like the touch-test can be expensive in a 3D context but it shouldn't be that bad...

[TheLogFather]

OK, had a quick run through some older versions, and narrowed down the problem to this:

https://github.com/LLK/scratch-flash/commit/9855a667e222ec082b8cacee9038b5f66190cf72

Looking at this code in public function bitmap (ScratchSprite.as): [code] if(Scratch.app.isIn3D) { var oldGhost:Number = filterPack.getFilterSetting('ghost'); filterPack.setFilter('ghost', 0); updateEffects(); var bm:BitmapData = Scratch.app.render3D.getRenderedChild(this, b.widthscaleX, b.heightscaleY); filterPack.setFilter('ghost', oldGhost); updateEffects(); ... [/code]

Does that mean it always rebuilds the bitmap without ghost, even when it doesn't need to ('cos there was no ghost applied)...?

[TheLogFather]

(Huh, git commenting is acting up... claims to fail when I comment, but comment arrives anyway, and then won't let me edit, so I can't fix formatting above... weird...)

[cwillisf]

I'm curious: have you tried removing that code to see if setting the ghost effect to 0 is the problem? I would expect that to be relatively cheap, but it's possible that it's causing the shader to recompile, which would certainly be more expensive than intended. There's a caching system in place which is supposed to prevent compiling each shader more than once, but it could be that this is somehow defeating the cache lookup.

The getRenderedChild call is what I expect to be the "expensive" part of doing a touch-test in 3D: it asks the video card to draw the sprite as it currently is, effects shaders and all. That's relatively cheap, but then it tells the video card to transfer those pixels back to main memory, which is relatively slow and may also pause all other rendering activity. I expect this to be "expensive" in that I wouldn't want to do it many times in one frame, but I don't expect it to take tens of milliseconds to do it once.

[TheLogFather]

Yes, it's getRenderedChild that's slow.

If I comment out everything else around it (all the ghost and updateEffects code), the slowdown remains.

If I change it to set bm to cachedBitmap, and only use getRenderedChild if that turns out null, then it all becomes fast again (but that causes various other things to go wrong, as you might expect).

[TheLogFather]

I kinda hacked together the following to replace public function bitmap in ScratchSprite.as:

    private var touchW:Number = 0.0;
    private var touchH:Number = 0.0;
    private var touchRot:Number = 0.0;
    private var touchGhost:Number = 0.0;
    private var touchBitmap:BitmapData = null;
    public function bitmap(forTest:Boolean = false):BitmapData {
        if (cachedBitmap != null && (!forTest || !Scratch.app.isIn3D))
            return cachedBitmap;

        // compute cachedBitmap
        // Note: cachedBitmap must be drawn with alpha=1 to allow the sprite/color touching tests to work
        var m:Matrix = new Matrix();
        m.rotate((Math.PI * rotation) / 180);
        m.scale(scaleX, scaleY);
        var b:Rectangle = (!Scratch.app.render3D || currentCostume().bitmap) ? img.getChildAt(0).getBounds(this) : getVisibleBounds(this);
        var r:Rectangle = transformedBounds(b, m);

        // returns true if caller should immediately return cachedBitmap
        var self:ScratchSprite = this;
        function bitmap2d():Boolean {
            if ((r.width == 0) || (r.height == 0)) { // empty costume: use an invisible 1x1 bitmap
                cachedBitmap = new BitmapData(1, 1, true, 0);
                cachedBounds = cachedBitmap.rect;
                return true;
            }

            var oldTrans:ColorTransform = img.transform.colorTransform;
            img.transform.colorTransform = new ColorTransform(1, 1, 1, 1, oldTrans.redOffset, oldTrans.greenOffset, oldTrans.blueOffset, 0);
            cachedBitmap = new BitmapData(Math.max(int(r.width), 1), Math.max(int(r.height), 1), true, 0);
            m.translate(-r.left, -r.top);
            cachedBitmap.draw(self, m);
            img.transform.colorTransform = oldTrans;
            return false;
        }

        if (SCRATCH::allow3d) {
            if (Scratch.app.isIn3D) {
                var oldGhost:Number = filterPack.getFilterSetting('ghost');
                if (touchBitmap && rotation==touchRot && touchGhost==oldGhost) {
                    if (b.width*scaleX==touchW && b.height*scaleY==touchH) {
                        return touchBitmap;
                    }
                }
                touchRot = rotation;
                touchW = b.width*scaleX;
                touchH = b.height*scaleY;
                touchGhost = oldGhost;
                if (oldGhost!=0) {
                    filterPack.setFilter('ghost', 0);
                    updateEffectsFor3D();
                    var bm:BitmapData = Scratch.app.render3D.getRenderedChild(this, b.width * scaleX, b.height * scaleY);
                    filterPack.setFilter('ghost', oldGhost);
                    updateEffectsFor3D();
                } else {
                    bm = cachedBitmap;
                    if (bm) {
                        touchBitmap = cachedBitmap;
                        return cachedBitmap;
                    }
                    bm = Scratch.app.render3D.getRenderedChild(this, b.width * scaleX, b.height * scaleY);
                }
                if (rotation != 0) {
                    m = new Matrix();
                    m.rotate((Math.PI * rotation) / 180);
                    b = transformedBounds(bm.rect, m);
                    touchBitmap = new BitmapData(Math.max(int(b.width), 1), Math.max(int(b.height), 1), true, 0);
                    m.translate(-b.left, -b.top);
                    touchBitmap.draw(bm, m);
                }
                else {
                    touchBitmap = bm;
                }
                var cropR:Rectangle = touchBitmap.getColorBoundsRect(0xFF000000, 0, false);
                if ((cropR.width > 0) && (cropR.height > 0)) {
                    var cropped:BitmapData = new BitmapData(Math.max(int(cropR.width), 1), Math.max(int(cropR.height), 1), true, 0);
                    cropped.copyPixels(touchBitmap, cropR, new Point(0, 0));
                    touchBitmap = cropped;
                }
                if (oldGhost==0) {
                    cachedBitmap = touchBitmap;
                    cachedBounds = cropR;
                    cachedBounds.offset(r.x,r.y);
                }
                return touchBitmap;
            } else {
                if (bitmap2d()) return cachedBitmap;

                cachedBounds = cachedBitmap.rect;

                // crop cachedBitmap and record cachedBounds
                // Note: handles the case where cropR is empty
                cropR = cachedBitmap.getColorBoundsRect(0xFF000000, 0, false);
                if ((cropR.width > 0) && (cropR.height > 0)) {
                    cropped = new BitmapData(Math.max(int(cropR.width), 1), Math.max(int(cropR.height), 1), true, 0);
                    cropped.copyPixels(cachedBitmap, cropR, new Point(0, 0));
                    cachedBitmap = cropped;
                    cachedBounds = cropR;
                }

                cachedBounds.offset(r.x, r.y);
                return cachedBitmap;
            }
        } else {
            if (bitmap2d()) return cachedBitmap;

            cachedBounds = cachedBitmap.rect;

            // crop cachedBitmap and record cachedBounds
            // Note: handles the case where cropR is empty
            var cropR:Rectangle = cachedBitmap.getColorBoundsRect(0xFF000000, 0, false);
            if ((cropR.width > 0) && (cropR.height > 0)) {
                var cropped:BitmapData = new BitmapData(Math.max(int(cropR.width), 1), Math.max(int(cropR.height), 1), true, 0);
                cropped.copyPixels(cachedBitmap, cropR, new Point(0, 0));
                cachedBitmap = cropped;
                cachedBounds = cropR;
            }

            cachedBounds.offset(r.x, r.y);
            return cachedBitmap;
        }
    }

It's kinda messy at the mo with the repeated chunks of code, and I don't know if it really needs to check whether the width/height has changed (i.e. updating touchW/H).

(Apart from above chunk of code, it also needs touchBitmap setting to null in clearCachedBitmap (ScratchSprite.as) to prevent exceptions when toggling Stage3D with a speech bubble showing.)

But the idea behind it is that touchBitmap is cached if various conditions have not changed since last touch test, and it is identical to cachedBitmap if ghost is zero.

But if ghost is not zero then it can be different. (Though I'm not really clear why the cachedBitmap does need to take ghost into account...? If it doesn't need to do that, then it seems it could always just use cachedBitmap instead.)

[TheLogFather]

OTOH, I wrote the above under the assumption that cachedBitmap was actually used with some other purpose elsewhere, and so touchBitmap could potentially be different if ghost is non-zero.

But I just searched through the source code and it's not obvious to me where else that might be...

In which case, doesn't that mean it can always just return cachedBitmap if it's non-null, as long as a few conditions haven't changed since last time? (I.e. if rotation is the same, and maybe height/width too) So where does ghost come in? –Perhaps it should be finding the cropped dimensions from the version of the bitmap with ghost applied, or something like that? –though that's not what it's doing at the moment, so maybe not...?

(I s'pose I'm still kinda guessing rather a lot, unfortunately, about exactly what the cachedBitmap is meant to be doing, how it all fits together with colour/sprite touch-testing, and maybe even stamps?)