From f8251cc64bd50bf80541c8d57edf351ef168fdfa Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@media.mit.edu>
Date: Mon, 15 Oct 2018 18:26:58 -0400
Subject: [PATCH] Cached url should be shared across watermark and
sprite-selector-item for performance, address other review feedback.
---
src/containers/sprite-selector-item.jsx | 10 +----
src/containers/watermark.jsx | 25 +++----------
src/lib/get-costume-url.js | 50 +++++++++++++++----------
3 files changed, 36 insertions(+), 49 deletions(-)
diff --git a/src/containers/sprite-selector-item.jsx b/src/containers/sprite-selector-item.jsx
index c33aa41e7..aceffc30e 100644
--- a/src/containers/sprite-selector-item.jsx
+++ b/src/containers/sprite-selector-item.jsx
@@ -46,15 +46,7 @@ class SpriteSelectorItem extends React.Component {
if (this.props.costumeURL) return this.props.costumeURL;
if (!this.props.assetId) return null;
- if (this.decodedAssetId === this.props.assetId) {
- // @todo consider caching more than one URL.
- return this.cachedUrl;
- }
-
- this.decodedAssetId = this.props.assetId;
- this.cachedUrl = getCostumeUrl(this.props.assetId, this.props.vm);
-
- return this.cachedUrl;
+ return getCostumeUrl(this.props.assetId, this.props.vm);
}
handleMouseUp () {
this.initialOffset = null;
diff --git a/src/containers/watermark.jsx b/src/containers/watermark.jsx
index 17a85dab7..c14b70ad4 100644
--- a/src/containers/watermark.jsx
+++ b/src/containers/watermark.jsx
@@ -1,4 +1,5 @@
import bindAll from 'lodash.bindall';
+import omit from 'lodash.omit';
import PropTypes from 'prop-types';
import React from 'react';
import {connect} from 'react-redux';
@@ -21,28 +22,15 @@ class Watermark extends React.Component {
getCostumeData () {
if (!this.props.assetId) return null;
- if (this.decodedAssetId === this.props.assetId) {
- return this.cachedUrl;
- }
-
- this.decodedAssetId = this.props.assetId;
- this.cachedUrl = getCostumeUrl(this.props.assetId, this.props.vm);
-
- return this.cachedUrl;
+ return getCostumeUrl(this.props.assetId, this.props.vm);
}
render () {
- const {
- /* eslint-disable no-unused-vars */
- assetId,
- vm,
- /* eslint-enable no-unused-vars */
- ...props
- } = this.props;
+ const componentProps = omit(this.props, ['assetId', 'vm']);
return (
<WatermarkComponent
costumeURL={this.getCostumeData()}
- {...props}
+ {...componentProps}
/>
);
}
@@ -73,11 +61,8 @@ const mapStateToProps = state => {
};
};
-const mapDispatchToProps = () => ({});
-
const ConnectedComponent = connect(
- mapStateToProps,
- mapDispatchToProps
+ mapStateToProps
)(Watermark);
export default ConnectedComponent;
diff --git a/src/lib/get-costume-url.js b/src/lib/get-costume-url.js
index 7d12edf80..41602b9f9 100644
--- a/src/lib/get-costume-url.js
+++ b/src/lib/get-costume-url.js
@@ -3,27 +3,37 @@ import {SVGRenderer} from 'scratch-svg-renderer';
// Contains 'font-family', but doesn't only contain 'font-family="none"'
const HAS_FONT_REGEXP = 'font-family(?!="none")';
-const getCostumeUrl = function (assetId, vm) {
-
- const storage = vm.runtime.storage;
- const asset = storage.get(assetId);
- // If the SVG refers to fonts, they must be inlined in order to display correctly in the img tag.
- // Avoid parsing the SVG when possible, since it's expensive.
- if (asset.assetType === storage.AssetType.ImageVector) {
- // If the asset ID has not changed, no need to re-parse
-
- const svgRenderer = new SVGRenderer();
-
- const svgString = vm.runtime.storage.get(assetId).decodeText();
- if (svgString.match(HAS_FONT_REGEXP)) {
- svgRenderer.loadString(svgString);
- const svgText = svgRenderer.toString(true /* shouldInjectFonts */);
- return `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+const getCostumeUrl = (function () {
+ let cachedAssetId;
+ let cachedUrl;
+
+ return function (assetId, vm) {
+
+ if (cachedAssetId === assetId) {
+ return cachedUrl;
}
- return vm.runtime.storage.get(assetId).encodeDataURI();
- }
- return vm.runtime.storage.get(assetId).encodeDataURI();
-};
+
+ cachedAssetId = assetId;
+
+ const storage = vm.runtime.storage;
+ const asset = storage.get(assetId);
+ // If the SVG refers to fonts, they must be inlined in order to display correctly in the img tag.
+ // Avoid parsing the SVG when possible, since it's expensive.
+ if (asset.assetType === storage.AssetType.ImageVector) {
+ const svgString = vm.runtime.storage.get(assetId).decodeText();
+ if (svgString.match(HAS_FONT_REGEXP)) {
+ const svgRenderer = new SVGRenderer();
+ svgRenderer.loadString(svgString);
+ const svgText = svgRenderer.toString(true /* shouldInjectFonts */);
+ cachedUrl = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`;
+ }
+ cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI();
+ }
+ cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI();
+
+ return cachedUrl;
+ };
+}());
export {
getCostumeUrl as default,
--
GitLab