Call into V2SVGAdapter instead of SVGRenderer #2800
Conversation
| @@ -8,8 +8,7 @@ const loadVector_ = function (costume, runtime, rotationCenter, optVersion) { | |||
| if (optVersion && optVersion === 2 && !runtime.v2SvgAdapter) { | |||
| log.error('No V2 SVG adapter present; SVGs may not render correctly.'); | |||
| } else if (optVersion && optVersion === 2 && runtime.v2SvgAdapter) { | |||
| runtime.v2SvgAdapter.loadString(svgString, true /* fromVersion2 */); | |||
| svgString = runtime.v2SvgAdapter.toString(); | |||
| svgString = runtime.v2SvgAdapter(svgString); | |||
There was a problem hiding this comment.
I'm planning to change this to always run, regardless of whether optVersion === 2 or not, since we want to run fixup on all SVGs. After that, we shouldn't need to export V2SVGAdapter from ScratchSVGRenderer, and we shouldn't need to run fixup in paint (since SVGs will pass through loadCostume before being loaded in paint)
There was a problem hiding this comment.
Let me know once that change goes in--I can rebase this after it does, to instead call:
serializeSvgToString(loadSvgString(svgString, optVersion === 2 /* fromVersion2 */));One issue is that once we start exporting multiple individual functions, passing them all in from the GUI (whereas before, we only had to pass in a single SVGRenderer object) could start to get cumbersome. IIRC this change was made to make updating the SVG renderer dependency less painful--is there some way to encode "this package works with any version of scratch-svg-renderer, but use the parent package's version"?
There was a problem hiding this comment.
I think peer dependencies? I would have to research them more.
|
Closing in favor of #2954 |
Depends on scratchfoundation/scratch-svg-renderer#213 and scratchfoundation/scratch-gui#6547
Resolves
Resolves scratchfoundation/scratch-svg-renderer#211
Proposed Changes
This PR changes the costume loading API to use the new
V2SVGAdapterAPI (a function) instead of theSvgRendererAPI (a class).Reason for Changes
By depending only on the parts of the SVG renderer API that are actually used here, this will allow the monolithic
SvgRendererclass to be removed.Test Coverage
Tested manually