Repository navigation
Conversation
|
@revarbat - BelfrySCAD generates the doc for text.scad without any problem, but I cannot figure out why it keeps failing here. The message in the error report doesn't clarify the situation. |
f88b538 to
c1f2f1a
Compare
|
Looking at the docs for write() I think that the letterspacing names should perhaps be letter_spacing, letter_space_em and letter_space_ref and they should all work the same way, namely that 0 adds no space, positive adds the designated amount of space and negative removes space. Having 1 be the "adds no space" option seems more confusing and also inconsistent since they don't all work that way. In fact, I feel a little uncertain that I understand exactly what they do...like is it multiplying the space...I think it's confusing. It basically implies that the operation is the incorrect thing that OpenSCAD currently does. It should be laid out as "There are three ways to letter space, either in CAD units, in units of em, or in units of "0" width (or $refchar)" and then they should all be clearly additive, with 0 meaning add no space. Is it right that we have max_width and max_height as the 2nd and 3rd positional parameters instead of box? I was imagining it the other way. The way things are now I wonder if The size section of the docs: I assume that you don't get a warning if you omit size but give em, say. I think you should delete the thing about what happens if it's omitted. Also include in the description of size the .72 relation to the em. Another concern is that you don't clearly indicate that these various options SET the font size. So they need to say something like "Set font size by specifying the height of the capital letter "H" in the font (or $refchar_cap if it is set) " I notice you use "full_height" here but in vfit you use "max" instead of "full". Why would you turn wrap_optimize off? I fed the API to claude and asked for feedback. It didn't say much of use. It did note that indent says it's sometimes ignored (when centering is on or no font selected) and it hopes that you get at least a warning message in this case. I hate it when parameters are silently ignored as I have many times spent tens of minutes trying to debug why a parameter mysteriously has no effect and it turns out it's because some other parameter I didn't notice disabled the parameter I was trying to set. I think that ideally parameters should never be ignored. Claude didn't complain, but I see a bunch of other things that are documented as "ignored if...." which I think should all be asserts. If you ask for something dumb/impossible it should not be quietly ignored. That leads to user frustration and confusion because, as noted, you don't understand WTF is going on and why you change this parameter and nothing seems to happen. The other thing claude complained about was hfit and its interaction with justify. And I agree that the docs as written are puzzling. I don't understand what/how hfit works with justify. I think both of these are kind of confusing as written. The pervasive passive voice doesn't help with clarity here. So maybe something like: hfit = Determines the with box used for anchoring the text: "tight" sets the width to the rendered horizontal width of longest line in the text, "width" sets the width to the What happens if max_width is INF? relation to justification is mysterious so I didn't propose anything there. vfit = Determines the height of the box used for anchoring the text: "tight" sets the height to the actual vertical size of the rendered text, "nominal" sets the box height using the nominal ascender and descender height for the font, and "max" sets the box height usingthe maximum ascender and descender height for the entire font. From reading this it's not clear what "nominal" actually does. The original text said "font set". Is that the max over all fonts (that's what a "set of fonts" seems to be)---but that makes no sense so presumably not. A more detailed discussion of this stuff should probably appear in the description. The text on bounding boxes says "...the wordwrapped text invariably doesn't span the entire dimensions..." If you give 2 dimensions the text won't be tight regardless of word wrapping. I would actually use the word "unlikely" instead of "invariably". I would define the tight bounding box more directly, not just tell the reader to think about it. e.g. "The tight bounding box is the box that exactly contains the rendered text. The user-defined bounds are specified using the As I ponder the above I wonder if making boxes the central thing is the wrong way to explain it. In particular, it seems awkward when it comes to the vertical box extent. And actually does vfit affect how the text fits into the user bounds? Hmm...yes it does. So changing vfit actually changes the text size, not just the box size. It doesn't seem like hfit can have the same effect---it really is just about boxes. (But maybe something to do with justification complicates matters?) The horizontal extent of a block of text is the actual width of the longest line of the rendered text, typically including a small margin defined in the font itself around the glpyh. You can choose between three different ways for defining the vertical extent of a text block. In can be "tight", the actual height of the rendered text. It can be "nominal", where the height of the text takes into account the typical space required for ascenders and descenders in the font, regardless of whether they appear in the specific text. This gives a more uniform and predictable height that doesn't vary from one text block to another. Finally the vertical text extent can be "maximal", where the height is based on the maximum possible vertical space needed for any glyph in the entire font. This may leave a large amount of extra space around most font glyphs depending on the font design. In the examples I checked the difference between nominal and max seemed very small. Is it sometimes big? I originally wrote the above with vfit options called out but then I realized that things are more of a mess because you use this in vfit but you also use it to specify font size, e.g. nom_height= and full_height=, so the concepts need to be defined clearly up front, not only for vfit. Also you need to standardize on "maximal" or "full" for how you want to describe the largest possible vertical size extent. Once you have the above in place it now makes sense to talk about defining the size with reference to the noimal and maximal (or full) text extent. But we now have a problem that hfit and vfit are doing different things even though they have parallel names. That seems like it makes it hard to write a clear doc text. Actually hfit has bugs and also unexpected behavior. If I do The bug is that in this case, the anchoring is wrong. It appears that it ignores hfit, actually, so when hfit="width" I'm supposed to anchor on the user given width but that doesn't happen, it anchors on the text itself. And when max_width is given it anchors on fictional centered text that doesn't exist. Getting back to documenting what happens....I had to do testing to understand what hfit did with "justify" because I couldn't guess it from the docs. It seems like you've overloaded hfit with two unrelated functions. What if I want to justify a text block and have if tight but anchor on the user box? That seems to be impossible. I think you should have a separate boolean, justify_tight=true/false that separately controls this. Docs for justify_last should say: allowed options are "left", "right", "center". And it should be an error if you give something else. (Currently it just does left if you make a typo.) And actually, maybe we have a similar double-use problem with vfit. Or maybe not...I'm uncertain. You're using vfit to determine the anchoring box but also the way we measure text height, which determines what text fits in a space. What if I want to fit the text into the box using "nominal" but then anchor on the text itself. You said on the chat that you already propagate some |
|
Maybe vfit should remain as it is and hfit goes away and we add vanchor and hanchor or something to specify anchoring? Anchoring could also be done as a pair, like anch_box="tight" gives you the tight box, and anch_box="user" gives you the user specified box. And anch_box=["tight","nominal"] specifies the vertical and horizontal in a mixed fashion. Not sure if that is better than a pair of params. |
|
Thank you for the detailed review!
Now that documentation is in place, I'll move onto the bug and the rest of your review later today. |
|
The reason I got to wondering about box= is that right now you would do |
|
Another observation, if I give max_height but not max_width I only get one line, even when max_height is generous and should allow word wrapping. Shouldn't this case wrap to use the allowed vertical space so it fits the text in the minimal width? |
|
Similarly if max_width and max_height are given but no font size I always get one line, but shouldn't it pick the largest font size that fits the text with wrapping into the box? |
No, that's an indeterminate problem. There could be multiple solutions of wrapped lines and font size. If you give a complete box with no font size, the font size is adjusted to fit within whichever limits are hit first. If you want wrapping in such a case, it's best you do it yourself by inserting An oversight on my part is omitting the feature where if you specify only max_height with a font size, then it should try to wrap the text as much as needed to fit into that height. At the extreme end you get one word per line. I have not written that yet. I made a couple of attempts this past week but it's involving some refactoring. |
|
The problem of given a box and no font size, find the largest font size that fits with wrapping can't possibly be indeterminate. Either you can or you can't do it with font size X. There must be a largest size because otherwise you're claiming I can make the font arbitrarily large and it still fits. Now it's a difficult problem that probably requires repeatedly trying to fit the font into the box at different sizes and iteratively locates the largest one that fits. |
|
I see. You're proposing an iteration, in which the font size converges to a value where the wrapped lines don't exceed the bounds in either dimension. It may not be too hard, and may be reasonably fast if an epsilon of 0.1 is allowed. What write() does now is find an exact solution for the font size that would fit the given text constrained by the bounds, and the text can be multi-line text that you have composed using newlines. The question is, what's more useful? Two situations could both be supported by write():
|
|
I think the non-wrap version (already implemented?) is probably more useful to users. |
The first example you identified as a bug was actually working but the box registration was off, causing incorrect positioning of the text and the attachments. I fixed that, but I cannot figure out what's wrong with this short example you posed (I added I think there's still something fundamental that I am not understanding about attachments. Stripped of the box display stuff, here's what I have:
What am I missing? Latest commit with the changes above have been pushed. |
|
The variable err9 is assigned twice. Note that you can save on dummy variables by chaining assertions like I think to fix anchoring you need the argument BTW,
I think you missed the point about error with regards to box_align. The point is that if a user asks for box_align=BACK then something should happen. It should never be a synonym for box_align=CENTER. Or if you insist that bogus stuff work, there should be a warning message. So there needs to be either an assert or a warning if: max_height=INF and box_align.y !=0 or box_align.z !=0. And same thing if max_width=INF and box_align.x !=0. This is one of those "and we ignore what you said" cases and it needs to be either a warning or error. Some suggested doc changes: Delete the sentence after that about indent because indent is described later. The section about inline font styles add an intro sentence:
Then after the list of styles change first sentence:
Delete the last sentence that says "If you want a new paragraph to have a different style...". That's obvious. Then the line about nonbreaking space should be changed to
Delete the heading labeled "bounding boxes" or give it a different name. There is only one "bounding box" so that's confusing. I guess if you want every single paragraph to have a heading you could rename it "The Bounding Box" as the heading. I found the section describing the bounding box to be unnecessarily wordy. I suggest There's some stuff after this about the bounding box having margins and alignment. That should be deleted because alignment is described in the layout section. Do you need to comment on ligatures in the letterspacing paragraph? The last bit of that should say:
I passed the API to ChatGPT and asked for critique. Here are its main points to either consider or ignore:
I also asked chatgpt about errors and warnings and it supports your position more than mine.
It made the interesting point that generic code is easier to write if options that aren't relevant are ignored. I also asked for advice about propagating $ variables. It does seem like propagating a $write object is the best strategy. I assume that works. I know we want bounds information, but AI suggested some things I hadn't thought of:
For defining the anchor box it suggests a pair of strings, a horizontal string and vertical string, with the horizontal options being "bounds" or "max". The vertical options:
|
|
The checkboxes you put in your comments are not clickable for me. Putting
Something does happen. It's aligned to the back, just as you asked. The alignment isn't being changed silently to CENTER, it's kept as BACK. The documentation saying it's "ignored" is wrong, and I have changed it. If you don't set either of the limits, they take on the size of the bounding box. If the inside box happens to be the same size as the outside box (which is the case when you don't set I changed the docs per your other suggestions, and added some words about ligatures. Ligatures don't appear because each character of the input text is uniquely rendered according to its position and font style. If you want a ligature you need to put the unicode for it in your text, but it may look weird for anything other than the default letterspacing.
The AI has interesting suggestions, several of which I implemented.
I just pushed a new commit with these changes. If you installed BelfrySCAD, it has a really nice feature "View > Show Docs" in the pulldown menu. You can see approximately what the docs would look like, including all the rendered examples. Of the examples I made up, there may need to be more, or they may need re-ordering. Be sure to get the latest release -- I submit bug reports every day (it's getting harder to find things to report) and Revar has been good about turning them around the same day or the next day and making a new release. |
|
This is just a response to what you wrote above, not a full review of the latest. I actually do NOT think it makes sense to just propagate your internal structure with 100 things in it. I think we should propagate something more limited and thought out that just has a few key pieces of info we think users may want. The simple fact that you say it's going to be terrifying to document this means it's going to be terrifying for users to read the docs. I'd rather have a propagation that misses things and we can add a few more later than something overwhelming. Also, the name Weird that you can't click the text boxes. I have been able to click textboxes in issues others have posted. I wonder if it matters if it's in the issue vs in a comment. I agree that we don't need to follow the same font as text() does but it does seem potentially confusing not to do so, and there need to be instructions on how to get the default font if you want it. I think we should poll others to see what people think is the right choice for default font. I don't think you understand the chaining issue. Suppose I have a list of 2N words where N is not known until run time and I want to set the words in alternating size, so word 0 is at size 15 and word 1 is at size 10 and word 2 at size 15 and so on. How do you do that? You can't use chaining because chaining requires hard coding the number of text blocks and here that's not known until run time. Now....is this an important issue? It doesn't seem like it to me. But it is a limitation of the architecture. I guess you can state in words that tight and rtl are named parameters and should not be used positionally. So regarding {{r}} etc, if you really want to typeset {{r}} you're out of luck? Is that the way it works? You put a zero width space in there if you want to? Seems like a bit of documentation about this is warranted. I thought about the \n handling and I do not think it's worth changing. But as currently documented it's confusing, because it seems like \n as things stand does double duty as a line break and a paragraph separator. The docs lean on the paragraph interpretation, but it seems like most use cases are going to be with what the user sees as several lines that are still (to the user) one paragraph. I asked AI for help. It suggests renaming "paragraph" to something else. And I like this idea. It suggests "text block" or "input block" as its first proposal instead of paragraph, and then "rendered line" for an actual output line after wrapping or presumably just "line". The rule is "Each string in the input and each \n starts a new text block. A text block may render as one or more lines if wrapping is enabled." Then you can explain indent as "indent offsets the first rendered line of each text block relative to any wrapped continuation lines." And "para_spacing" becomes "block_spacing". Docs could say things like this:
AI pushed back on the need for "\n{ }\n". Why can't "\n\n" be two blocks? It's pretty weird that ["acbum","","bcdef"] only produces two lines. Is there some compelling reason here? Now I like that idea a lot, but it has a problem which is that we just made |
|
Latest changes:
We're getting close.... probably next step is to change the default font to regular style. While I find it annoying that I must always specify boldface in OpenSCAD, I am also finding it annoying that the default in write() isn't regular style, just because it isn't a normal expectation. |
|
I'm now getting a duplicate assignment on err10. The whole point of introducing the "text block" concept was to ELIMINATE reference to paragraphs outside of a word-wrapping context, not to have two terms for the same concept. The problem with "paragraph" is that it means something to users that is different than "lines". You have combined the concept of lines and paragraphs into one concept, and for most users, most of the time, the "line" interpretation is most important and should dominate, and the paragraph interpretation is not important and should be secondary. This issue is real: I was seriously confused when you presented write() the first time because I thought it was impossible to create lines. The fix is to completely eliminate the word paragraph from any centrality in the discussion. It should only appear as a secondary term like "a text block can look like a paragraph". So introducing them as text blocks or "paragraphs" completely defeats the purpose because you have introduced two terms for one thing, one unfamiliar term (immediately forgotten) and one familiar term (paragraph), which now dominates. The term text block is new and unfamiliar so you need to use that term clearly and systematically and not suggest that text blocks are anything other than text blocks. You can say that text blocks can render as lines or as paragraphs, for example, but not that they ARE paragraphs. If the strings are "text blocks" then "block_spacing" should be the parameter that controls the spacing between blocks, not "para_spacing". This spacing parameter does, after all, change the spacing between blocks that are functioning as lines, not just blocks that are functioning as paragraphs. Another thing is that suppose I create a text with several short lines and I want to spread them out a bit. If I set line_spacing, nothing happens because line_spacing only changes the space between continuation lines in wrapping. So the so-called "para_spacing" parameter is actually really the line spacing parameter too! I think this means Typo counters -> encounters Change: to Is that bad behavior for future proofing? e.g. if you do want to add {{smiley}}? Note that the chaining issue is super minor. I'm not sure it really warrants an example...though if you've got one I wouldn't toss it. Some of these questions are not about saying "The API is deficient and needs to be fixed" and maybe more like "The API has a limit at this weird edge case...so maybe we should document the limit?" For some reason I had in my head that the recursive solution to chaining wouldn't work somehow...not sure why. Regarding default font and your frustration with always wanting bold, would adding a "style" parameter be helpful and worth doing for getting into bold without having to give a full fontspec? It would presumably override the style of the font. I see you've propagated I would like to see the three vbound heights propagated in $write. So something like $write.height.nominal, $write.height.full, $write.height.tight. The final missing piece I see is that there's no way to specify the vertical and horizontal extent of the anchor box. We need to work out a plan there. It appears that something is wrong with the alignment. The bounding box is taking up the whole limits even though it's supposed to be tight on the sides and nominal in the vertical direction, which means my frame_align doesn't do anything. There's no parameter to change the horizontal behavior. And vbound seems to be ignored. It also seems that users may be surprised by the position of the green dot, so that may require an explanation. Right now the docs have no description of anchoring, which clearly needs fixing, so the problem of the green dot can go there once that's written. Basically the normal behavior is that children appear with their centers aligned with the parent center. That's not happening here, I assume because we used On the matter of ligatures, are they eliminated even when no letterspacing is requested? Or only when letterspacing is active? So main issues
|
|
All right:
In your example, add the parameter You can see this by adding Try it again, and use
How is that a missing piece? The anchor box isn't specified, it's derived. It's a computed box. It inherits its dimensions from the user specified bounds or from the bounding box if a user bound isn't given. If you don't provide a limit, the corresponding anchor box dimension is the same as the bounding box.
The behavior with ligatures is documented in the version you reviewed; let me know if anything needs clarification. A ligature is used only if you specify the unicode for it. I was thinking that it's possible for a user to define a list of custom inline codes, setting them to |
|
Ligatures are supported, by requiring that any desired ligature must be included in the input text as a unicode character. That is the only way, other than treating whole words as units as you suggest. (And by the way, I have never seen OpenSCAD render a ligature automatically if it could have done so, so I really don't see the relevance). I cannot think of any other way to support ligatures through the features available in OpenSCAD. We couldn't support Korean either (in fact I doubt native OpenSCAD could work with Korean) because every word is basically a ligature of phoneme characters. The AI advice is irrelevant because there is no interface into Harfbuzz other than via Kerning is being preserved by comparing the advance property of every character pair against the sum of advance properties of the individual characters in the pair. To the extent that some languages have overlapping glyphs, the kerning should handle it. As far as I know, OpenSCAD doesn't support contextual character substitution. There isn't any compelling reason to be doing something down in the weeds with font glyphs that OpenSCAD doesn't do in the first place. Yes, the named anchors have Z components. Why shouldn't they? In 2D, the z component is always zero. In 3D, the Z component is determined by the direction vectors, TOP, BOT, CENTER. Where is the Z component a problem? I even include an example in write3d() of anchoring the baseline on a Z component. Your example: |
|
I've added I also added a bunch more literal substitution codes and provided the capability for the user to add more via the Still mulling what we talked about in chat, how I might support ligatures by treating whole words as single units if letterspacing=0 and there is no style change mid-word. |
|
Latest changes:
After pushing this latest commit, I fixed one other long-standing bug involving words too long to fit within max_width. I'll push that after further feedback. I need one or two good demonstrations of ligatures and contextual substitution using macOS-native fonts, but I cannot come up with those examples because I haven't had a Macbook for 3 years, it's just Windows now. I also found that |



Addresses #1642. Changes:
write()andwrite3d, and a couple of useful functions to get font sizestext()andtext3d()into text.scadStill TBD:
write_path()