Set SVG skin size synchronously #497
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/cc @kchadha |
adroitwhiz
commented
Sep 4, 2019
| @@ -100,7 +114,19 @@ class SVGSkin extends Skin { | |||
| * @fires Skin.event:WasAltered | |||
| */ | |||
| setSVG (svgData, rotationCenter) { | |||
| this._svgRenderer.fromString(svgData, 1, () => { | |||
| this._svgRenderer.loadString(svgData); | |||
There was a problem hiding this comment.
my additional changes start here
cwillisf
changed the base branch from
develop
to
revert-493-revert-470-skin-alter-push
…-size-synchronously-2
cwillisf
merged commit 30ef2b1
into
scratchfoundation:revert-493-revert-470-skin-alter-push
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves
Solves
SVGSkins' size staying small; subsequently supersedes #496, #486This PR fixes SVG costumes' size showing as 0x0 in the editor, which was causing the GUI's SVG costume integration tests to fail as they were specifically searching for the string
100 x 100.Proposed Changes
This PR re-merges "Skin alter push", but changes
SVGSkin.setSVG()to set the costumesizeoutside of theSVGRenderercallback rather than inside it.It also includes a minor change (previously #486), which swaps the order of setting the skin
sizeand callingcalculateRotationCenter. This fixes calculated rotation centers.Reason for Changes
The VM expects the skin size to be properly set immediately after calling
updateSVGSkin, as it sets the costume size to the skin size after calling it. If the skin size is only set after the SVG renderer callback fires, the SVG costume size will appear as 0x0 in the editor.Test Coverage
This appears to be, in a way, tested for in the GUI. A more explicit test over there might be a good idea.