Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new updateDrawable* methods #2203

Merged
merged 2 commits into from
Nov 19, 2019

Conversation

mzgoddard
Copy link
Contributor

@mzgoddard mzgoddard commented Jun 8, 2019

Resolves

Depends on #2196.
Depends on scratchfoundation/scratch-render#469.

Make VM and Render faster.

Proposed Changes

  • Use updateDrawable* methods added in render PR

Reason for Changes

  • Use updateDrawable* methods added in render PR

As mentioned in the other PR, using an active tense API in place of a passive tense API allows changes to propagate to Drawables much faster.

Benchmark Data

Refer to tables in scratchfoundation/scratch-render#469.

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@kchadha kchadha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kchadha kchadha removed their assignment Jun 25, 2019
@fsih fsih assigned mzgoddard and unassigned cwillisf Oct 1, 2019
@fsih
Copy link
Contributor

fsih commented Oct 1, 2019

@mzgoddard Can you resolve the conflicts? Then this will be good to merge

@fsih
Copy link
Contributor

fsih commented Oct 1, 2019

When this goes in, test by running projects that use position, scale, visibility, rotation center, changing costumes, and graphic effects.

Release notes: N/A (rendering should be faster)

- replace updateDrawableProperties with updateDrawablePosition and other methods
- use all of the updateDrawable* methods in updateAllDrawableProperties
@mzgoddard mzgoddard force-pushed the render-update-drawable branch from 45771a4 to 1e9f240 Compare November 12, 2019 18:20
@fsih fsih merged commit d47ea58 into scratchfoundation:develop Nov 19, 2019
@BryceLTaylor
Copy link

Test plan:

Description: The VM will now make changes to drawable properties (i.e. position, rotation, costume #, graphic effects, etc.) on drawables (i.e. sprites, clones, pen layer, backdrop, etc.) using different methods from the Renderer.

Properties:

  • position
  • direction
  • scale
  • visibility
  • graphical effects (ghost, whirl, etc.)
  • costume/backdrop (which one is showing)
  • rotation center (from paint editor)

To test:

  • Make 10+ clones of a sprite
  • Update the properties (listed above) of the sprite and all the clones. Make sure it behaves as expected.
  • Look out to see if the changes happen slower. They should be faster, though.
  • Try adding more clones to see if it makes it worse
  • Draw with the pen
  • Load the video extension and make sure it looks the same.
  • Flip the video
  • Try out some existing graphically intensive projects. They should work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants