microsoft / maker.js

📐⚙ 2D vector line drawing and shape modeling for CNC and laser cutters.
http://maker.js.org
Apache License 2.0
1.78k stars 273 forks source link

Captions are not drawn relative to model origin #469

Open hjfreyer opened 4 years ago

hjfreyer commented 4 years ago

Maybe this is working as intended, but I found it surprising.

Steps to reproduce

  1. Write a script with a top-level model that includes sub-models with captions. For example:
var m = require("makerjs");

function box1() {
  return {
    paths: {},
    origin: [0, 0],
    models: {
      inner: new m.models.Rectangle(10, 10)
    },
    caption: {
      text: "B1",
      anchor: new m.paths.Line([5, 5], [5, 5])
    }
  };
}

function box2() {
  return {
    paths: {},
    origin: [11, 0],
    models: {
      inner: new m.models.Rectangle(10, 10)
    },
    caption: {
      text: "B2",
      anchor: new m.paths.Line([5, 5], [5, 5])
    }
  };
}

function main() {
  return {
    paths: {},
    models: {
      box1: box1(),
      box2: box2()
    }
  };
}

module.exports = main();
  1. Render the script in the playground or with SVG.

Expected result

Each caption is in the middle of its box.

Actual result

Captions are displayed on top of each other.

Workaround

Calling originate() on the overall model puts the captions into their proper places:

function main() {
  return originate({
    paths: {},
    models: {
      box1: box1(),
      box2: box2()
    }
  });
}

Commentary

I think this is very confusing, considering everything else is drawn relative to the model's origin.

If I put the caption in its own sub-model, then it works as I expected:

function box2() {
  return {
    paths: {},
    origin: [11, 0],
    models: {
      inner: new m.models.Rectangle(10, 10),
      cap: {
        caption: {
          text: "B2",
          anchor: new m.paths.Line([5, 5], [5, 5])
        }
      }
    }
  };
}

Is this the intended use case? If so addCaption is a little weird for not doing it that way.

danmarshall commented 4 years ago

Thanks for pointing this out @hjfreyer - I believe your expectation is correct, I'll take a look.