bugGNU roff - Bugs: bug #57836, groff's adjustment algorithm is...

 
 

bug #57836: groff's adjustment algorithm is not environment-aware (and want `adjparity` register)

Submitter:  Dave <barx>
Submitted:  Mon 17 Feb 2020 07:43:04 AM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Confirmed
Privacy:  Public Assigned to:  None
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 01 Dec 2023 08:54:55 PM UTC, comment #7: 

We have an explicit external use case for user control of adjustment parity.

https://github.com/NetHack/NetHack/issues/1153#issuecomment-1830869312

G. Branden Robinson <gbranden>
Group administrator
Sun 09 Oct 2022 03:45:14 AM UTC, comment #6: 

Bug #62691 also seeks to make certain global state information environment-specific.

Dave <barx>
Group Member
Wed 02 Jun 2021 05:43:42 AM UTC, comment #5: 

Brilliant!

Since diversions and environments don't necessarily go hand in hand, your solution may not improve the output of the specific example in the original submission,  But it does give the user a tool to solve the problem himself--which, for edge cases like this, ought to be good enough.

comment #4:

> Consider: a diversion gobbles up the current partially collected line
> if there is one.  I'm thinking that it is therefore hard to judge
> whether the adjustment parity should be reset upon simply entering a
> diversion or not, since there might not be a partially collected line,
> if a break happened accidentally or otherwise just before the diversion
> was created.


It seems like this diversion behavior is something you want only if you're writing for an Obfuscated Code contest, which I presume is why (a) .box was invented, and (b) the new-environment-for-new-diversion advice is given.

Dave <barx>
Group Member
Wed 02 Jun 2021 03:10:03 AM UTC, comment #4: 

Thanks, Dave.

Solution-wise, here's a sketch of what I'm thinking.

1. Get rid of this static `reverse` flag in `env.cpp:distribute_space()`.

2. Instead, we want a new concept called something like "(current) adjustment parity", which is a property of the `environment` class.

3. The parity only needs to take two values, so it could remain an int, become a bool (which I'd prefer to an int), or become a two-valued enum.  One value would mean "distribute space starting from the left," and the other, "from the right".  In terms of the current code, from the left means you don't reverse the node list before and after distributing space, and from the right means you do.

4. An environment seems like the right data structure in which to house this datum because (A) it preserves continuity with the existing code and (B) it doesn't feel quite like the right fit for a diversion given our current documentation.  I'm further updating the summary of this bug to reflect that, though you may want to change it again if I'm not persuasive.  Consider: a diversion gobbles up the current partially collected line if there is one.  I'm thinking that it is therefore hard to judge whether the adjustment parity should be reset upon simply entering a diversion or not, since there might not be a partially collected line, if a break happened accidentally or otherwise just before the diversion was created.

5. A common idiom in AT&T troff, according to our Texinfo manual, was to switch environments immediately upon starting a diversion.  ("Switching to another (empty) environment (with the ev request) avoids the inclusion of the current partially collected line" node "Diversions".)

6. With all that bookkeeping in place, we can turn to the question of whether it makes sense to expose this property of an environment via the language in any way.  Adding it to the output of .pev seems hard to object to.

7. The more interesting question is whether to make the property user-modifiable.  I cannot imagine what would break if we did (maybe someone else can).  Rather than adding a request, I'd be inclined to expose a writable register for it.  Even though I regard the current practice (inherited from AT&T) of having troff-internal writable registers reside in the (global) name space without any sort of prefix with chagrin, changing that would be groff 2.0-level disruptive and is orthogonal to the current issue.  So I'd call the new register, say, "adjparity", and map assigned values the way we do *roff Booleans: Non-positive -> zero ("left"); positive -> one ("right").

8. As something of a frivolous bonus, since the adjustment parity has to be initialized as part of every environment, including the default one created automatically at startup, we could assign it a default value such that we initially adjust lines from the same direction Heirloom Doctools troff, and presumably all other AT&T-descended troffs.

Thoughts?


G. Branden Robinson <gbranden>
Group administrator
Tue 01 Jun 2021 08:33:13 PM UTC, comment #3: 

As bug #60707 indicates, the problem described below is not grotty-specific; PostScript output exhibits the same behavior, just on a scale invisible to the naked eye.  Updating the Category and Summary accordingly.

Dave <barx>
Group Member
Wed 26 May 2021 03:54:48 AM UTC, comment #2: 

comment #1:

> But if that turns out to be wrong, I can open a new bug report
> for this manifestation.


I believe it has in fact turned out to be wrong, based on the code snippet Branden posted in bug #60665; accordingly, I've opened the new bug #60673 to track the quasi-related comment #1 problem.

Dave <barx>
Group Member
Wed 15 Jul 2020 03:59:51 PM UTC, comment #1: 

This could be considered a different bug from the one reported in comment #0.  But they are closely related, and fixing both at the same time is probably easier than fixing them both in isolation.  But if that turns out to be wrong, I can open a new bug report for this manifestation.

$ nroff tty | uniq
While  the  example  in  this  bug's previous comment is somewhat
contrived and a bit of an edge case in real life, there turns out
to  be  a  more  innate  bug  in grotty's balancing algorithm. As
mentioned before (and easily observable), when grotty adds spaces
to  a  line  in  the  process  of justifying it, the algorithm it
utilizes adds spaces from opposite ends of each line. But when it
adds  this  space,  it  does  not take into account lines with no
adjustment at all required. Therefore if space only need be added
to  every  other  line  of  the text, all the space ends up being
added to the same end of the line, degrading the uniform grayness
of  the  output,  as  can  be  seen in this example. There is one
fairly simple way to address this: grotty shouldn't "count" lines
that  don't  need  to  be  adjusted; instead, it should apply the
alternation pattern only to those lines that do need adjustment.

$ cat tty
.nh
.ss 12 0
While the example in this bug's previous comment is somewhat
contrived and a bit of an edge case in real life, there turns out
to be a more innate bug in grotty's balancing algorithm. As
mentioned before (and easily observable), when grotty adds spaces
to a line in the process of justifying it, the algorithm it
utilizes adds spaces from opposite ends of each line. But when it
adds this space, it does not take into account lines with no
adjustment at all required. Therefore if space only need be added
to every other line of the text, all the space ends up being
added to the same end of the line, degrading the uniform grayness
of the output, as can be seen in this example. There is one
fairly simple way to address this: grotty shouldn't "count" lines
that don't need to be adjusted; instead, it should apply the
alternation pattern only to those lines that do need adjustment.
$

Here is the same paragraph adjusted by hand using the suggested change to the algorithm.  The overall grayness does not change from one side of the column to the other as it did before; it's much more pleasing to the eye, while being no harder (or easier) to read.  (If you're reading this in an email client that uses a proportional typeface, neither example will display properly.  Look at them in the bug tracker.)

While  the  example  in  this  bug's previous comment is somewhat
contrived and a bit of an edge case in real life, there turns out
to be a more innate  bug  in  grotty's  balancing  algorithm.  As
mentioned before (and easily observable), when grotty adds spaces
to  a  line  in  the  process  of justifying it, the algorithm it
utilizes adds spaces from opposite ends of each line. But when it
adds this space, it does not take  into  account  lines  with  no
adjustment at all required. Therefore if space only need be added
to  every  other  line  of  the text, all the space ends up being
added to the same end of the line, degrading the uniform grayness
of the output, as can be seen  in  this  example.  There  is  one
fairly simple way to address this: grotty shouldn't "count" lines
that  don't  need  to  be  adjusted; instead, it should apply the
alternation pattern only to those lines that do need adjustment.


Dave <barx>
Group Member
Mon 17 Feb 2020 07:43:04 AM UTC, original submission:  


$ groff -Tascii tty_example | head -14
For  TTY  output  with  full justification, grotty pads by adding
extra spaces alternating every line left-to-right  and  right-to-
left,  as  can be seen in this paragraph.  This helps to preserve
overall uniform grayness  throughout  the  text,  as  well  as  a
monospaced typeface allows.

But  diversions  requiring  adjustment apparently are included in
this  sequence  even  if they are not output right away (or ever,
for  that  matter).   This  means that a number of closely spaced
diversion  definitions  can  break  grotty's balancing algorithm,
putting  all  the  space  on  the same end of every line, as this
paragraph demonstrates.


$ cat tty_example
.nh
For TTY output with full justification, grotty pads by
adding extra spaces alternating every line left-to-right and
right-to-left, as can be seen in this paragraph.  This helps
to preserve overall uniform grayness throughout the text,
as well as a monospaced typeface allows.
.
.sp
.
But diversions requiring adjustment apparently are included in
this sequence even if they are not output right away (or ever,
.box dummy
This is some dummy text, enough to kick in the adjustment algorithm.
.box
for that matter).  This means that a number of closely spaced
.box dummy
This is some dummy text, enough to kick in the adjustment algorithm.
.box
diversion definitions can break grotty's balancing algorithm,
.box dummy
This is some dummy text, enough to kick in the adjustment algorithm.
.box
putting all the space on the same end of every line, as this
.box dummy
This is some dummy text, enough to kick in the adjustment algorithm.
.box
paragraph demonstrates.
$ groff -Tascii --version
GNU groff version 1.22.4.74-b400-dirty
Copyright (C) 2018 Free Software Foundation, Inc.
GNU groff comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of groff and its subprograms
under the terms of the GNU General Public License.
For more information about these matters, see the file
named COPYING.

called subprograms:

GNU grotty (groff) version 1.22.4.74-b400-dirty
GNU troff (groff) version 1.22.4.74-b400-dirty


Dave <barx>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by barx (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-12-01 gbranden Summarygroff's adjustment algorithm is not environment-aware groff's adjustment algorithm is not environment-aware (and want `adjparity` register)
    2021-06-02 gbranden StatusNone Confirmed
        Summarygroff's adjustment algorithm is not diversion-aware groff's adjustment algorithm is not environment-aware
    2021-06-01 barx CategoryDriver - others/general Core
        Summarygrotty's adjustment algorithm is not diversion-aware groff's adjustment algorithm is not diversion-aware

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code