-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
createOutlines() + pathToPoints() #352
Conversation
- added `createOutlines()` function, handles textFrames, textPaths, multi-line text, multi-box text. - added `pathToPoints()` function, handles polygons from `createOutlines()` or most other pageItems (oval, rect, polygon, etc), also accepts groups and arrays. - added tests for both functions - updated changelog - added nice error message for `textSize()`, common beginner mistake giving size less than zero.
PS - anyone using Sublime + docblockr (for jsdocs) know of a trick for adding the * before a pasted in block of code for an example?? ie. when adding an example of code to an
(where only the first line where pasted into is commented... all the following need to be manually given the
What's the trick for pasting in and getting those *'s.. or post-paste, getting them without re-formatting the whole jsdoc block (which doesn't add proper *'s to code block either).... Maybe there's a better Sublime plugin? Would like to eventually go through and add lots of tiny examples to functions – but need to figure out this workflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ffd8, great stuff!
Here is some observations and suggestions:
pathsToPoint()
-
Docs:
-
I think the description is really confusing. I did not understand at all what to expect until I started examining the examples in detail with help of the inspect function.
Suggestion:
Returns an object containing an array of all points, an array of all beziers (points + their anchor points) and an array of all paths of a given path item in InDesign. Together withcreateOutlines()
this can be used on text items to retrieve points, beziers and paths of text items. Accepts both single paths or a collection/group of paths.
When using this on a multi path object (e.g. text with separate paths), thepaths
property can be used to loop over every path separately, whereas the propertiespoints
andbeziers
contain arrays for all paths combined.
An optional second parameter allows to add and return additional points between existing points, which is helpful for subdividing existing paths. -
Skip the "Ignore if using beziers."
-
Return value I would describe like this:
Returns object with the following arrayspoints
,beziers
,paths
-
-
Examples
The examples you give are more suited for our examples repo as they go into detail quite a bit.
I think the examples that should be given in the inline docs should mostly show the syntax and the different options how to use the function with its different options.Generally the examples are good, however, I think they could use some improvements in some areas:
-
Example 1
I think I would rather use some sort of s-shaped path here instead of a circle. With the circle it's not quite obvious at first glance that the five extra points between the original points are necessarily created via thepointsToPath()
function (alternativly they might be placed there via matrix rotation). I think an actual path in an s-shape makes that more clear. -
Examples 2
I would set the debug bezier paths to a different color (or more transparent) to make them distinct from the actual letter paths. Also, you should use a different character than "H", because if the users have set Arial or so as their default font they won't see any beziers. Maybe an "f" or "G" -
Example 3
I would assign a random color to each separate path to make it more obvious that the script results in separated paths. -
Example 1 & Example 2
It seems you created the script with points as default unit. When I run it withMM
, the bezier points are huge and don't really look like path points anymore. We should just addunits(PT)
at the beginning of the script to make the result look consistent.
-
-
Function:
If I pass the second parameter (add extra points) on open curves, it adds points as if the shape were closed (see image). I would expect it to only return points of actual existing paths though.
createOutlines
-
Docs
I would skip "Intended for use withpathToPoints()
..." as the function has lots of other useful applications. -
Examples
Same as above, I find the examples to extensive for the inline examples. I would move them to the examples repo and instead only provide short examples here that show the syntax and how to use the different options-
Example 1
Instead of setting fill color to none, I would set it to some distinct color.
Either uncomment the inspect line or take it out. -
Example 2
Instead of setting fill color to none, I would set it to some distinct color.
I find the mapping of the point size more confusing than helpful. Maybe that's also a unit problem? -
Example 3
Love it! :)
Example 1, Example 2
Same comment about the units as thepathToPoints()
examples above, set to PT -
-
Function
-
Return value: Would it maybe be better to always return an array, even if there is just one polygon? Otherwise the user would always have to test first, if they have a polygon or an array? Not sure. If you feel otherwise, ignore this.
-
if converting a story or a word with overset text, InDesign will throw an error. What should we do about this?
-
if converting linked text frames this behaves really unpredictably. It deletes all text that comes after the give text frame. It converts all text frames to groups of polygons, if there are text frames before the given text frames. So the return value becomes even more unpredictable. I think we should try to return predictable results. For example if I use this command on a text frame that is linked with other text frames, I think all these text frames should be converted to outlines and then all the polygons should be returned in an array.
-
From the docs:
Warning, InDesign requires that text have a fill color in order to createOutlines.
I think it would be nice if the function would take care of not letting that happen (i.e. give text temporarily a fill color so it can be converted to outlines)
-
-
All in all the function seems quite slow. I wonder if it can be optimized in any way or if we just hit the limits of InDesign here?
Other:
You changed the checkNull
option to
var checkNull = function (obj) {
if(obj === null || typeof obj === undefined) error("Received null " + obj);
};
What's the purpose of that? This prints "Received null null" when an actual null is passed to this function.
Thanks for the thorough review- good A couple replies to some of the feedback:
Good suggestion above (haha probably wrote too late/fast in first draft).
Ahhh damn.. found a weird case I hadn't tested.. guess I'll have to check if line or closed shape – since in all closed shapes, need to finish interpolation back to starting point.
No prob to remove
Good question and idea.. yeah, it can be random/surprising when InDesign says it's a single polygon or an array = probably safest to just always suggest iterating over an array.
Hmm ok gotta test this (and see what natively happens in ID using GUI option to createOutlines – some of this weird behavior would happen here too).
Strange.. only tested with two linked textFrames (on same page.. no prob, on split pages, naturally only the one on active page converted) - but will test more.
This is tricky territor, as within InDesign GUI of createOutlines - you get the same warning if trying to createOutlines on non-filled text = who are we to change the color of that type? and to what color? Of course it helps if one had said
True – it definitely is slower than just using the GUI menu.. but maybe we hit some slog in how many things we have to check that they don't?
That was while trying to finally debug (and quiely exit) a super common bug when doing something like |
PS @trych - while I redo/simplify the examples.. do you know the trick in Sublime + Docblockr for pasting in code after |
- returned core.js back to normal - modified shape description
@ffd8 Ok, first this, before I address the other points. If I understand you right you just need to place several |
About the other points:
For me it would be enough to have this in an example script, but if you want to briefly mention it in the docs, fine with me.
👍
Wonder if this is maybe a CS6 issue? I had linked text frames on the same text frames. Maybe let's set up an example and compare on our systems, so we can see if this behaves differently in CS6 and latest CC.
Yeah, that's what I meant. Does it also not work on text with no fill color but with a stroke color? Because that would be strange if that failed to convert it to outlines if the user can even see the type.
Will have a closer look at the function itself later, maybe there is some space for optimization, I just checked the functionality itself, had not really a closer look at the code yet.
Yes, I think that would be good. I'm not a big fan of the checknull function to be honest, it is a pretty lazy checking and gives a pretty poor feedback to users without letting them know which function causes the error. So, yes, let's address this separately. |
Info is stored in the path object: anyone know how to tell if a polygon has an open path or not
if(pageItem.paths[0].pathType === PathType.OPEN_PATH) {
// open path
}
if(pageItem.paths[0].pathType === PathType.CLOSED_PATH) {
// closed path
} See: https://www.indesignjs.de/extendscriptAPI/indesign8/#Path.html |
- updated descriptions - reduced examples to minimum #### pathToPoints() - fixed open path items w/ `addPts` #### createOutlines() - fixed no fill/stroke text with - fixed issue with linked textFrames - always returns array of polygons
Just pushed new code reacting to most things mentioned above. textPaths (on pageItems, oval/rect/polygon/line) – trying to remove that item after converting to polygons.. in GUI, it goes poof.. by code, it sticks around. Any ideas? See code here I just noticed a super slight shift of the type to the left when doing createOutlines by code vs GUI.... not sure if it's an issue with this function.. but could be annoying for really precise type treatment. Maybe it's just my setup.. or maybe InDesign is doing quite some fancy things when doing |
.. left on while testing..
I also get a shift when I use the GUI. I think it's usually because it turns of kerning. Or do you mean an extra shift on top when using
Could you post a code sample here so I could give this some testing? Will review the stuff tomorrow if I find time. |
Actually to test this, could you maybe share your test scripts and (if required) test docs in .idml format? This is a quite complex feature, it would help me, if I would not have to build everything from scratch as I hae quite limited time right now. |
And of course, while preparing the same document for sharing/testing – the lag went poof and the slight offset too??? wtf.. haha now it's the same speed ~100ms and no offset. 🤔 Anyways here's the basic code + IDML.
|
clean up of wording/formatting for docs + made simpler minimal examples. For `pathToPoints()` I think it's still necessary to give small `for` loop examples for a beginner to know just how to process the object.
Just pushed a latest update. I think it qualifies all review points now. The only detail that bugs me is this textPath object not being removed. But that's a tiny detail that will likely bother only me. Can re-approach this later, but would be nice to merge and start testing with students asap. |
added small update to `textSize()` to changelog
Been playing with this all week and it's been working really well – no speed issues anymore - outlines instantly. Drawing the bez/points as shapes ( @trych any issues, or can we merge it (and eventually improve if there's any found issues). |
@ffd8 Sorry, I have really limited time due to this lockdown situation and having my kids at home 24/7. Will try to review this tonight. If there is an urgent need to have your students use this, I would suggest they could download the branch's |
@trych noo problem and totally understand. Great when you have a chance, but no rush. Indeed – was interested to introduce this to my students next week, and if it's not merged by next Thursday, I'll simply deliver a custom dev basil.js w/ it, since function names/obj etc will likely stay the same. |
Ok, here we go. (sorry, quite a few points again 🙈 ) pathToPoints()Regarding the examples: I think it's fine to have some loops, as it helps to demonstrate the functions. But I would suggest these improvements:
In example 2, 3 & 4, if you want to keep the println statements, please insert a proper comment before "# of {something}", else there will be a syntax error when copy pasting the examples. So something like /*
* @example <caption>Points</caption>
* var pts = pathToPoints(obj);
* println(pts.points.length); // # of points
*
* for (var i = 0; i < pts.points.length; i++) {
* var pt = pts.points[i];
* point(pt.x, pt.y);
* }
*/ i would change to /*
* @example <caption>Draw all points of a vector path</caption>
* var myCircle = ellipse(50, 50, 20, 20);
* var pts = pathToPoints(myCircle);
*
* for (var i = 0; i < pts.points.length; i++) {
* var pt = pts.points[i];
* ellipse(pt.x, pt.y, 3, 3);
* }
*/ (I replaced the point function with the ellipse function here as it would be easier to see the resulting circles) Same notes go for the other examples. Example 3: Example 4: And one tiny extra note (that's probably something about you being an native English speaker and me not), I am not very familiar with the "w/" as a short form for "with", so for out international audience I would simply stay with "with". :) Sorry, this all might seem pretty nit-picky, but if even I am struggling to get your examples to work easily, then a new user will surely have a difficult time. Function:
var tf = text(LOREM, 20, 20, 50, 50);
pathToPoints(); // -> Typfehler: undefined ist kein Objekt
pathToPoints("Test"); // -> Typfehler: undefined ist kein Objekt
pathToPoints([2, 3]); // -> Typfehler: undefined ist kein Objekt
pathToPoints(tf.words.firstItem()); // -> Fehler: Objekt unterstützt Eigenschaft oder Methode paths nicht
createOutlines()Docs:
Example 1:
Example 2:
Function:
error("createOutlines(), incorrect first parameter. Use: Story | TextFrame ...");
|
- ran into strange issue with current dev branch
- forgot some inline comment slashes - untested nested loop was using same var
- cleaned up examples (captions, code, self-contained) - linted code - more typechecking and clearer error messages - removed debuging code
@trych thanks for the thorough review. While it pushed the PR to the backburner, all are valid points that I've incorportated and built upon. Just merged, since it's working (and tested with 25 students over the past month). There are minor things to try and improve in the future ( |
createOutlines()
, handles textFrames, textPaths, multi-line text, multi-box text.pathToPoints()
, handles polygons fromcreateOutlines()
or most other pageItems (oval, rect, polygon, etc), also accepts groups and arrays.textSize()
, common beginner mistake giving size less than zero.(totally new approaching, replacing basiljs gets points! #268 )