Move all image lookups to XML tree and support directly specifying image file (#886)

* handle image lookup in initialize for zip files

* remove unused params

* get image paths from node separately

* do image lookups in initialize

* fix lint errors

* handle case insensitivity and small tweaks

* handle nodes without name before we call toLowerCase

* fix isString

* move image lookups to xml tree

* update function comment

* spell out lowercase

* add additional comment about why we return node itself

* remove extra variable

* rename imagePaths to imageAttributes

* early returns

* move all resolved images to js_assets attribute instead of overwriting attr

* missed on previous renaming path->attribute

* fix lint errors

* need node back after rebase

* dont need children param anymore

modern/src/App.js

@@ -190,8 +190,9 @@ function Container(props) {
 }
 
 function Layout({
-  node,
   id,
+  node,
+  js_assets,
   background,
   // desktopalpha,
   drawBackground,
@@ -211,7 +212,7 @@ function Layout({
   }
 
   if (drawBackground) {
-    const image = node.js_imageLookup(background);
+    const image = js_assets.background;
     if (image == null) {
       console.warn(
         "Unable to find image to render. Rendering null",
@@ -279,12 +280,12 @@ function Layout({
   );
 }
 
-function Layer({ node, id, image, x, y }) {
+function Layer({ id, node, js_assets, image, x, y }) {
   if (image == null) {
     console.warn("Got an Layer without an image. Rendering null", id);
     return null;
   }
-  const img = node.js_imageLookup(image.toLowerCase());
+  const img = js_assets.image;
   if (img == null) {
     console.warn("Unable to find image to render. Rendering null", image);
     return null;
@@ -407,7 +408,8 @@ function AnimatedLayer({
 
 function Button({
   id,
-  image,
+  js_assets,
+  // image,
   // action,
   x,
   y,
@@ -417,15 +419,10 @@ function Button({
   node,
 }) {
   const [down, setDown] = React.useState(false);
-  const imgId = down && downImage ? downImage : image;
-  if (imgId == null) {
-    console.warn("Got a Button without a imgId. Rendering null", id);
-    return null;
-  }
   // TODO: These seem to be switching too fast
-  const img = node.js_imageLookup(imgId);
+  const img = down && downImage ? js_assets.downimage : js_assets.image;
   if (img == null) {
-    console.warn("Unable to find image to render. Rendering null", image);
+    console.warn("Got a Button without a img. Rendering null", id);
     return null;
   }
 

modern/src/initialize.js

@@ -19,6 +19,47 @@ import EqVis from "./runtime/EqVis";
 import AnimatedLayer from "./runtime/AnimatedLayer";
 import Component from "./runtime/Component";
 
+async function prepareMakiImage(node, zip, file) {
+  let { h, w } = node.attributes;
+  // TODO: Escape file for regex
+  const img = Utils.getCaseInsensitveFile(zip, file);
+  if (img === undefined) {
+    return {};
+  }
+  const imgBlob = await img.async("blob");
+  const imgUrl = await Utils.getUrlFromBlob(imgBlob);
+  if (w === undefined || h === undefined) {
+    const { width, height } = await Utils.getSizeFromUrl(imgUrl);
+    w = width;
+    h = height;
+  }
+
+  return {
+    h,
+    w,
+    imgUrl,
+  };
+}
+
+function imageAttributesFromNode(node) {
+  if (!node.name) return [];
+  switch (node.name.toLowerCase()) {
+    case "layer": {
+      return ["image"];
+    }
+    case "layout": {
+      return ["background"];
+    }
+    case "button":
+    case "togglebutton": {
+      return ["image", "downImage"];
+    }
+    default: {
+      return [];
+    }
+  }
+}
+
 const noop = (node, parent, zip, store) =>
   new GuiObject(node, parent, undefined, store);
 
@@ -54,40 +95,7 @@ const parsers = {
   sendparams: noop,
   elements: (node, parent, zip, store) =>
     new JsElements(node, parent, undefined, store),
-  bitmap: async (node, parent, zip, store) => {
-    let { h, w, x, y } = node.attributes;
-    const { file, gammagroup, id } = node.attributes;
-    // TODO: Escape file for regex
-    const img = Utils.getCaseInsensitveFile(zip, file);
-    if (img === undefined) {
-      return new MakiObject(node, parent);
-    }
-    const imgBlob = await img.async("blob");
-    const imgUrl = await Utils.getUrlFromBlob(imgBlob);
-    if (w === undefined || h === undefined) {
-      const { width, height } = await Utils.getSizeFromUrl(imgUrl);
-      w = width;
-      h = height;
-      x = x !== undefined ? x : 0;
-      y = y !== undefined ? y : 0;
-    }
-
-    return new MakiObject(
-      node,
-      parent,
-      {
-        id,
-        file,
-        gammagroup,
-        h,
-        w,
-        x,
-        y,
-        imgUrl,
-      },
-      store
-    );
-  },
+  bitmap: noop,
   eqvis: (node, parent, zip, store) =>
     new EqVis(node, parent, undefined, store),
   slider: (node, parent, zip, store) =>
@@ -170,6 +178,51 @@ async function parseChildren(node, children, zip, store) {
   node.js_addChildren(filteredChildren);
 }
 
+async function nodeImageLookup(node, root, zip) {
+  const imageAttributes = imageAttributesFromNode(node);
+  if (!imageAttributes || imageAttributes.length === 0) {
+    return;
+  }
+  if (!node.attributes.js_assets) {
+    node.attributes.js_assets = {};
+  }
+  await Promise.all(
+    imageAttributes.map(async attribute => {
+      const image = node.attributes[attribute];
+      if (!image || !Utils.isString(image)) {
+        return;
+      }
+      let img;
+      if (image.endsWith(".png")) {
+        img = await prepareMakiImage(node, zip, image);
+      } else {
+        const elementNode = Utils.findXmlElementById(node, image, root);
+        if (elementNode) {
+          img = await prepareMakiImage(
+            elementNode,
+            zip,
+            elementNode.attributes.file
+          );
+
+          const { x, y } = elementNode.attributes;
+          img.x = x !== undefined ? x : 0;
+          img.y = y !== undefined ? y : 0;
+        } else {
+          console.warn("Unable to find image:", image);
+        }
+      }
+      node.attributes.js_assets[attribute.toLowerCase()] = img;
+    })
+  );
+}
+
+async function applyImageLookups(root, zip) {
+  await Utils.asyncTreeFlatMap(root, async node => {
+    await nodeImageLookup(node, root, zip);
+    return node;
+  });
+}
+
 async function applyGroupDefs(root) {
   await Utils.asyncTreeFlatMap(root, async node => {
     switch (node.name) {
@@ -204,12 +257,8 @@ async function applyGroupDefs(root) {
 
 async function initialize(zip, skinXml, store) {
   const xmlRoot = skinXml.children[0];
-  const root = new JsWinampAbstractionLayer(
-    skinXml.children[0],
-    null,
-    undefined,
-    store
-  );
+  await applyImageLookups(xmlRoot, zip);
+  const root = new JsWinampAbstractionLayer(xmlRoot, null, undefined, store);
   await parseChildren(root, xmlRoot.children, zip, store);
   await applyGroupDefs(root);
   return root;

modern/src/runtime/MakiObject.ts

@@ -68,15 +68,6 @@ class MakiObject {
     this._emitter.dispose();
   }
 
-  js_imageLookup(id: string) {
-    const element = Utils.findElementById(this, id);
-    if (element) {
-      return element.js_annotations;
-    }
-
-    return null;
-  }
-
   js_groupdefLookup(id: string) {
     const groupdef = Utils.findGroupDefById(this, id);
     if (groupdef) {

modern/src/utils.ts

@@ -17,6 +17,14 @@ export function isPromise(obj) {
   return obj && typeof obj.then === "function";
 }
 
+export function isString(obj) {
+  return typeof obj === "string";
+}
+
+export function isObject(obj) {
+  return obj === Object(obj);
+}
+
 // Convert windows filename slashes to forward slashes
 function fixFilenameSlashes(filename) {
   return filename.replace(/\\/g, "/");
@@ -232,7 +240,10 @@ export function findDescendantByTypeAndId(node, type, id) {
 }
 
 function findDirectDescendantById(node, id) {
-  return node.children.find(item => item.attributes.id === id);
+  const lowerCaseId = id.toLowerCase();
+  return node.children.find(
+    item => item.attributes && item.attributes.id.toLowerCase() === lowerCaseId
+  );
 }
 
 function* iterateLexicalScope(node) {
@@ -286,6 +297,39 @@ export function findGroupDefById(node, id) {
   });
 }
 
+// Search down the tree for <Elements> nodes that are in node's lexical scope.
+// return the first child of an <Elements> that matches id unless we find
+// node first, in that case we didn't find the element
+// TODO: this might be overly generous, including some definitions that
+// shouldn't be accessible. But it's working for now :-X
+export function findXmlElementById(node, id, root) {
+  if (root.uid === node.uid) {
+    // Search ends if we find the node that initiated the search, since it means we weren't able to
+    // find the match in its scope
+    // Return the node itself as a kind of sentinel value to look for, since finding the node is an
+    // ending condition for the search
+    return node;
+  } else if (root.name === "elements") {
+    const element = findDirectDescendantById(root, id);
+    if (element) {
+      return element;
+    }
+  } else {
+    const children = root.children || [];
+    for (const child of children) {
+      const element = findXmlElementById(node, id, child);
+      if (element) {
+        if (element.uid === node.uid) {
+          // This happens when we find the node before we find the declaration, which means it
+          // either doesn't exist or it wouldn't be in scope
+          return null;
+        }
+        return element;
+      }
+    }
+  }
+}
+
 // This is intentionally async since we may want to sub it out for an async
 // function in a node environment
 export async function getUrlFromBlob(blob: Blob): Promise<string> {