Scratch-vm: Investigate performance issues in "Floating Blocks" project

[thisandagain]

Expected Behavior

• Project should run at 30fps on testing Chromebook

Actual Behavior

• Project runs slowly

Steps to Reproduce

• https://scratch.mit.edu/projects/130041250/

[towerofnix]

This project also runs very slowly in Scratch 2.0 for me - possibly the heavy use of color effects.

[towerofnix]

In Scratch 3.0, this project runs at 60fps (30fps in compatibility mode) for me, once the ghost-effect transition of the text finishes. While the text is ghost-ing, the project runs very slowly (5-10fps).

[griffpatch]

I don't think it's the ghosting.
If I remove the ghosting script then it still runs just as slow... For me
it's all in the cloning code. As more squares are cloned, it gets slower
and slower until finally they all 50 clones are created and then it bursts
into life.
If you remove the scripts that allow the squares to move around then the
cloning happens much much faster too, so perhaps something to do with
cloning and the 'when I start as a clone' hat?

On 1 November 2017 at 01:01, Florrie [e-mail] wrote:

In Scratch 3.0, this project runs at 60fps (30fps in compatibility mode)
for me, once the ghost-effect transition of the text finishes. While the
text is ghost-ing, the project runs very slowly (5-10fps).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/LLK/scratch-vm/issues/738#issuecomment-340949220, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGbNvl8hrR92KaWGzrt7NwcjuQdiQzzNks5sx8LbgaJpZM4QM_Tb
.

[griffpatch]

Ok got it... It's the if on edge bounce block. Don't know why, but if you
remove that from the movement loop then the whole things runs great.

On 1 November 2017 at 08:17, Andrew Griffin [e-mail] wrote:

I don't think it's the ghosting.
If I remove the ghosting script then it still runs just as slow... For me
it's all in the cloning code. As more squares are cloned, it gets slower
and slower until finally they all 50 clones are created and then it bursts
into life.
If you remove the scripts that allow the squares to move around then the
cloning happens much much faster too, so perhaps something to do with
cloning and the 'when I start as a clone' hat?

On 1 November 2017 at 01:01, Florrie [e-mail] wrote:

In Scratch 3.0, this project runs at 60fps (30fps in compatibility mode)
for me, once the ghost-effect transition of the text finishes. While the
text is ghost-ing, the project runs very slowly (5-10fps).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/LLK/scratch-vm/issues/738#issuecomment-340949220,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGbNvl8hrR92KaWGzrt7NwcjuQdiQzzNks5sx8LbgaJpZM4QM_Tb
.

[griffpatch]

So digging deeper:

When a sprite is 'cloned', the rendered-target.makeClone() function is
called.
This 'invalidated' the costumes convex hull bounds
(scratch-rendered.setConvexHullDirty)
However, what is a real killer is that it appear to do this for all the
clones rather than just itself? so the more clones there are, the more
times this is called (which is why it starts to grind to a half after a few
clones appear).

Here is the important stack trace. When a clone is created, it appears that
the rotation center is assumed to need setting, but I assume this must be
global across all clones because this invalidates all the clones for that
sprite by emitting _skinWasAltered in a loop for all the clones. And of
course, every time the convex hull is invalidated, the bounce on edge
causes a recalculation of the convex hull each frame and now for every
clone (not just the new ones).

setConvexHullDirty (scratch-render.js:12285)
_skinWasAltered (scratch-render.js:12382)
2.EventEmitter.emit (events.js?7c71:96) <<<<<<
setRotationCenter (scratch-render.js:9102)
setRotationCenter (scratch-render.js:12817) <<<<<
updateDrawableProperties (scratch-render.js:11156)
updateAllDrawableProperties (rendered-target.js:565)
makeClone (rendered-target.js:784)
createClone (scratch3_control.js:122)
execute (execute.js:189)
stepThread (sequencer.js:115)
stepThreads (sequencer.js:70)
_step (runtime.js:1053)
(anonymous) (runtime.js:1398)

And in conclusion I think the actual issue may come down to this:

value: function setRotationCenter(x, y) {
  var emptySkin = this.size[0] === 0 && this.size[1] === 0;
  var changed =* x !== this._rotationCenter[0] || y !==

this._rotationCenter[1]*;
if (!emptySkin && changed) {
this._rotationCenter[0] = x;
this._rotationCenter[1] = y;
this.emit(Skin.Events.WasAltered);
}
}

No margin of error has been allowed for in the setRotationCenter function.
This is being called but there is an error of 0.0001 between x and
this._rotationCenter[0]... the same for y.
Perhaps allowing for this error will solve the problem?

On 1 November 2017 at 08:20, Andrew Griffin [e-mail] wrote:

Ok got it... It's the if on edge bounce block. Don't know why, but if you
remove that from the movement loop then the whole things runs great.

On 1 November 2017 at 08:17, Andrew Griffin [e-mail] wrote:

I don't think it's the ghosting.
If I remove the ghosting script then it still runs just as slow... For me
it's all in the cloning code. As more squares are cloned, it gets slower
and slower until finally they all 50 clones are created and then it bursts
into life.
If you remove the scripts that allow the squares to move around then the
cloning happens much much faster too, so perhaps something to do with
cloning and the 'when I start as a clone' hat?

On 1 November 2017 at 01:01, Florrie [e-mail] wrote:

In Scratch 3.0, this project runs at 60fps (30fps in compatibility mode)
for me, once the ghost-effect transition of the text finishes. While the
text is ghost-ing, the project runs very slowly (5-10fps).

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/LLK/scratch-vm/issues/738#issuecomment-340949220,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGbNvl8hrR92KaWGzrt7NwcjuQdiQzzNks5sx8LbgaJpZM4QM_Tb
.

[mzgoddard]

@griffpatch One link we can add to that is makeClone calls updateAllDrawableProperties, which includes rotationCenter, which calls skin.setRotationCenter in RenderWebGL.

Adding your mentioned error margin to setRotationCenter will reduce the setConvexHullDirty calls done to zero a frame while the project is making clones. Each new clone will still go through the costly step of building its convex hull the first time.

---

I can add on what that convex hull building time is doing. Building the convex hull spends the majority of its time in a few deeper calls: WebGLRenderingContext.readPixels then _getPoint and then hull. Using the Chrome performance inspector tab I've seen individual calls while building the convex hull take from 2ms to 16ms. readPixels stalls the rendering pipeline and busy waits for the gpu to be able to respond. The best way to reduce time spent on readPixels by that nature would be to group uses of it into one call, like build convex hull of multiple Drawables at one time, or to build the hull without the gpu and readPixels.

---

One other costly but less noticeable code block slows down this project once all the clones spawn: touching mouse-pointer?. touching mouse-pointer? also uses readPixels and the gpu to find if the Drawable is under the mouse. The RenderWebGL call that runs this pick is less costly in this case where it doesn't need to also to _getPoint and hull work. As well it reads in this case one pixel back instead of an area the Drawable is rendered into.

Each of the still existing clones (50) uses touching mouse-pointer? every frame. Powerful machines can power through these operations and this project can hit 30fps during this part.

[griffpatch]

I do wonder why the convex hull is not stored against the costume rather
than within the sprite / clone? If it was stored on the costume and
invalidated there too then it would be super fast to retrieve it once it
was first created. I guess it depends whether an effect has been applied
that causes the shape to change? Then each clone might have a different
convex hull? Is that it? Size and rotation are reset before calculating so
I'm guessing at that. Perhaps there still may be scope to store the vanilla
hull as that would be the expected one... Oh wait, what about sprites that
flip horizontally...

On 1 Nov 2017 20:15, "mzgoddard" [e-mail] wrote:

@griffpatch https://github.com/griffpatch One link we can add to that
is makeClone calls updateAllDrawableProperties, which includes
rotationCenter, which calls skin.setRotationCenter in RenderWebGL.

Adding your mentioned error margin to setRotationCenter will reduce the
setConvexHullDirty calls done to zero a frame while the project is making
clones. Each new clone will still go through the costly step of building

its convex hull the first time.

I can add on what that convex hull building time is doing. Building the
convex hull spends the majority of its time in a few deeper calls:
WebGLRenderingContext.readPixels then _getPoint and then hull. Using the
Chrome performance inspector tab I've seen individual calls while building
the convex hull take from 2ms to 16ms. readPixels stalls the rendering
pipeline and busy waits for the gpu to be able to respond. The best way to
reduce time spent on readPixels by that nature would be to group uses of
it into one call, like build convex hull of multiple Drawables at one time,

or to build the hull without the gpu and readPixels.

One other costly but less noticeable code block slows down this project
once all the clones spawn: touching mouse-pointer?. touching
mouse-pointer? also uses readPixels and the gpu to find if the Drawable
is under the mouse. The RenderWebGL call that runs this pick is less
costly in this case where it doesn't need to also to _getPoint and hull
work. As well it reads in this case one pixel back instead of an area the
Drawable is rendered into.

Each of the still existing clones (50) uses touching mouse-pointer? every
frame. Powerful machines can power through these operations and this
project can hit 30fps during this part.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLK/scratch-vm/issues/738#issuecomment-341227990, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGbNvqSI78Kunfpta-iTbbYkKWR9YZTZks5syNFNgaJpZM4QM_Tb
.

[mzgoddard]

@griffpatch Yeah its shape changes due to effects that need a convex hull per target. Changes to effects like fisheye, spiral, and pixelate set the convex hull to dirty.

I don't think size and rotation or flipping horizontally result in needing a new convex hull. The same points are used in those cases as they get multiplied against the Drawable's transformation matrix. And those resulting points are used to initialize a bounding box for the Drawable.

One possibility may be to memoify the resulting convex hulls against an id formed from its inputs. At least that would remove further recalculations from drawable property changes?

[griffpatch]

That is what I wondered, and going by the fact that most sprites don't have
an effect like whirl enabled, then maybe we could cache a vanilla convex
hull for each costume (calculated just in time) and not stored on scratch
server or anything. Cache against internal costume id and invalidated on
costume edit. Then use this of sprite has no effects applied, otherwise
calculate as we currently do (expect to fix current issue with rotation
center error)

On 2 Nov 2017 02:48, "mzgoddard" [e-mail] wrote:

@griffpatch https://github.com/griffpatch Yeah its shape changes due to
effects that need a convex hull per target. Changes to effects like
fisheye, spiral, and pixelate set the convex hull to dirty.

I don't think size and rotation or flipping horizontally result in needing
a new convex hull. The same points are used in those cases as they get
multiplied against the Drawable's transformation matrix. And those
resulting points are used to initialize a bounding box for the Drawable.

One possibility may be to memoify the resulting convex hulls against an id
formed from its inputs. At least that would remove further recalculations
from drawable property changes?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLK/scratch-vm/issues/738#issuecomment-341302452, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGbNvu8oGRz860EzmIxhDhCk75FsDOjnks5syS1igaJpZM4QM_Tb
.

[thisandagain]

This project is now running at > 30 fps in the VM Playground on our testing hardware.