Skip to content
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

b.bounds() issues #166

Open
trych opened this issue Feb 7, 2017 · 11 comments
Open

b.bounds() issues #166

trych opened this issue Feb 7, 2017 · 11 comments
Assignees

Comments

@trych
Copy link
Contributor

trych commented Feb 7, 2017

Hi there,

I found a few problems with b.bounds() and have put together some improvements that could be made to the function.

Here we go:

1) Incorrect bounds on rotated frames

At the moment the results of b.bounds() on rotated text frames are incorrect, as values like endBaseline are not taken into consideration. Fortunately properties like left or right are all relative positions (as opposed to x and y), so I would suggest to calculate the bounds in a way that rotates along the rotated word. That way we can always use a line like

var rect = b.rect(bounds.left, bounds.top, bounds.width, bounds.height);

to draw a rect around a text (if the matrix is still rotated), no matter if that text is rotated or upside down. I have already implemented this in a branch I am working on, and can report that it works. Keep in mind that this does not change the behavior for non-rotated frames at all, it only makes it work for rotated frames, too!

2) Text collections throw an error

Text collections like [Object Words] until yesterday's commit threw the error

"b.bounds(), invalide type of parameter! Can't get bounds for this object."

as they were not recognized as a text object by basil. Now they are and b.bounds() throws an error about the fact that baseline is not a property. Therefore we need to address those collections and pick their baseline, endBaseline from the objects contained in the collections.

3) Text object throws yet another error

Text objects (as in [Object Text]) that are created by using the itemByRange() method throw a different error. They do have a baseline property, but for some reason this property is an array whose only value contains the actual baseline value. Why on earth that is the case, I have no idea. But we need to address this type of object seperately yet again.

4) Calculate actual bounds of multiline text

At the moment something like

var tf = b.text(b.LOREM, 0, 0, 200, 200);
var bounds = b.bounds(tf.paragraphs.firstItem());
b.rect(bounds.left, bounds.top, bounds.width, bounds.height);

draws really incorrect bounds (see image). For multiline text the actual last line is not taken into consideration. Also the right edge of the paragraph is calculated by the last line when it should be calculated by the longest line. I suggest to treat every type of text that the actual bounds are calculated (in this for example by figuring out the longest line of the paragraph).

bildschirmfoto 2017-02-07 um 12 25 10

5) Smarter handling of disconnected text parts

This one is tricky. At the moment on a line break basil throws the warning

warning("b.bounds(), not possible to get correct bounds, possible line break within textObj");

That's alright, but I would suggest to also introduce an boolean property like lineBreak to the returned object, so people can start to handle those line breaks in any way they desire, as I find it impossible to predict what a user wants to do with a "line broken" word. Additionally I think we should check if there is a frame break, a page break and a spread break in the text, as these might all require different plans of action. So, b.bounds() would return these flags as well.
I just had a student who was trying to connect words with lines between rotated frames cross page (sometimes) and it turned out to be really tricky. Those flags would have made his project a whole lot easier.

6) x-Height

Lastly for now, I want to address this comment in the source code

// TODO: not sure if this 100% correct, check
// http://en.wikipedia.org/wiki/File:Typography_Line_Terms.svg
var xHeight = y1 + descent;

Unfortunately this is not correct, as this effectively only subtracts the descent from the ascent. While this might result in something not far off from the actual x-height, the descent has nothing to do with the x-height and therefore could produce really wrong results. As the results are not reliable, I suggest, we take x-height out, as the user might be better off guessing the x-height or measuring it with creating outlines of an x.

I started implementing some of this stuff, but I would like to hear your thoughts, before I get to far into this.

@trych trych self-assigned this Feb 7, 2017
@b-g
Copy link
Member

b-g commented Feb 7, 2017

@trych
Many thanks for putting this list of issues together! Yes I totally agree is not the best function at the moment. I think your suggestions would be a good cure for the listed issues! I just would vote to not remove the "6) x-Height" feature. Yes it is off from the actual x-height, but it is often close and still better than nothing. IMO we should keep it.

@trych
Copy link
Contributor Author

trych commented Feb 7, 2017

Ok, but I think we should at least mention in the documentation then, that it might not be exactly on the x-height.

@ff6347
Copy link
Member

ff6347 commented Feb 7, 2017

I'm currently not deep into this function. Just a suggestion. To calculate the height of some text we could convert it to outlines. Then we have the properties of geometricBounds read them and remove the outline again. I used this on some projects to get precise locations of footnotes.

@ff6347
Copy link
Member

ff6347 commented Feb 7, 2017

/**
 * [get_height description]
 * @param p {Text, Paragraph, Line, Word, Character} 
 * @return {Array}  geometricBounds
 */
var get_height = function(p) {
  var gb = null;
  var res = p.createOutlines(false);
  if (DEBUG) {
    $.writeln("created outline");
    $.writeln(res[0].constructor.name);
  }
  gb = res[0].geometricBounds;
  for (var i = res.length - 1; i >= 0; i--) {
    res[i].remove();

  }
  return gb;
};

@trych
Copy link
Contributor Author

trych commented Feb 7, 2017

Yes, I used something like this in the past as well. The problem will be performance. If you want to retrieve the bounds of lot of words or characters, the outline thing will make it really slow. And the only thing that it would help to retrieve is the x-height. So, I think we should just do it without outlines for the sake of performance.

@ff6347
Copy link
Member

ff6347 commented Feb 8, 2017

okidoki

@b-g
Copy link
Member

b-g commented Feb 8, 2017

@fabianmoronzirfas Thanks for the thoughts and code snippet! But yes, back then we decided because of performance reasons not to go down the createOutlines route.

@trych
Copy link
Contributor Author

trych commented Feb 8, 2017

Ok, but so generally I could move forward in the ways I outlined?
I'm not entirely sure what the best solution will be for the "disconnected" text parts, but I will try to come up with something good as I go along and will ask again for your feedback then.

@b-g
Copy link
Member

b-g commented Feb 8, 2017

@trych Please!

@trych
Copy link
Contributor Author

trych commented Feb 12, 2017

Holy cow, this turns out complicated…

Anyway, one more thing I noticed (and that's something that I actually came across a few times in my class). We also need to check, if the entire text that is being investigated is not within the overflowing text area.

This could easily happen with code like this:

var tf = b.text(b.LOREM, 0, 0, 100, 100);
var words = b.words(tf.parentStory);

for(var i = 0; i < words.length; i++) {
  b.bounds(words[i]); // throws an error as soon as it reaches words in the overflow area
} 

How should we handle cases like this? Two options:

1.) Throw an error and therefore interrupt the script.
2.) Give a basil warning and let b.bounds() return null (or something similar). This would allow for more custom handling of overflow text, as you could do something like:

var tf = b.text(b.LOREM, 0, 0, 100, 100);
var words = b.words(tf.parentStory);

for(var i = 0; i < words.length; i++) {
  var b = b.bounds(words[i]);
  if(b === null) {
    // do some stuff to handle the overflow text, and keep the script running
  }
} 

I prefer the second option. How about you guys?

@b-g
Copy link
Member

b-g commented Feb 13, 2017

@trych oh no, hope it is not too painful! Option 2), to return null for objects w/o a defined bounding box, sounds best for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants