bugGNU roff - Bugs: bug #62934, [gropdf] sometimes \(em becomes a...

 
 

bug #62934: [gropdf] sometimes \(em becomes a different char when rendered

Submitter:  Nikita Ivanov <nikitaivanov>
Submitted:  Sun 21 Aug 2022 08:42:49 PM UTC
   
 
Category:  Driver gropdf Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  deri
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 23 Aug 2022 07:53:02 PM UTC, comment #10: 

That one is also after a vim modeline was added to the bottom of gropdf in git. I will add it to my working copy so they are the same, maybe that will settle it down.

Deri James <deri>
Group Member
Tue 23 Aug 2022 06:27:14 PM UTC, comment #9: 

comment #7:

> Thanks for this good advice, but does not explain why it has
> started happening.


I've noticed it in previous commits of yours (but assumed it was intentional).  E.g., commit 0a4c256f changed leading tabs to spaces on several lines whose code was unaltered.

Dave <barx>
Group Member
Tue 23 Aug 2022 05:47:43 PM UTC, comment #8: 


comment #7:

> Thanks for this good advice, but does not explain why it has started happening.


If I get any idea, I'll tell you.  :(

G. Branden Robinson <gbranden>
Group administrator
Tue 23 Aug 2022 05:44:42 PM UTC, comment #7: 

Thanks for this good advice, but does not explain why it has started happening.

Deri James <deri>
Group Member
Tue 23 Aug 2022 03:44:52 PM UTC, comment #6: 

comment #5:

> I'm not sure what happened, both of my last commits were one liners where I used kompare to change the single line which was relevent from my working copy of gropdf.


I wasn't really trying to complain; I was mainly curious what the fix looked like.  :)

> Do you have any idea how I can stop this happening?


I tend to run two commands compulsively that do not appear to be in your work flow.


git diff
git diff --cached


Because I run them so often, I have set up aliases for them in my $HOME/.gitconfig.


[alias]
        dc = diff --cached
        di = diff


"git diff --cached" shows you what you have "staged" for the next commit, and "git diff" shows you the diff against what is staged if anything, and against the currently checked out commit (often HEAD) otherwise.

Try running those at various points and see if that tells you where the crazy formatting changes creep in.

G. Branden Robinson <gbranden>
Group administrator
Tue 23 Aug 2022 02:06:18 PM UTC, comment #5: 

I'm not sure what happened, both of my last commits were one liners where I used kompare to change the single line which was relevent from my working copy of gropdf. This is the workflow which I have always used:-

git stash
git pull
Run kompare to copy relavent changes to git version.
Make
Run test of current fix with test-groff.
Edit Changelog (copy text into clipboard).
git add Changelog gropdf.
git commit (copy clipboard).
git push
git stash pop

My "working copy" of gropdf is actually 1.22.4 which lives in /usr/bin.

I have always done this, without whitespace issues. One thought is a vim: line has been added to the git copy of gropdf and something is adjusting the white space to follow the vim settings when it saves the file, which would point at kompare which probably shares code with the kde editor which I think does parse modelines in files.

Do you have any idea how I can stop this happening?


Deri James <deri>
Group Member
Tue 23 Aug 2022 09:38:13 AM UTC, comment #4: 

There were a lot of formatting changes with that patch.  The heart of the fix seems to be this.


diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
index 4b6b1c009..b5fe98d87 100644
--- a/src/devices/gropdf/gropdf.pl
+++ b/src/devices/gropdf/gropdf.pl
@@ -3779,7 +3779,6 @@ sub RemapChr
        my $glyph=$fnt->{NO}->[$unused]->[1];
        delete($fontlst{$cft}->{CACHE}->{$cftsz});
        $fnt->{NAM}->{$chnm}->[ASSIGNED]=$unused;
-        $fnt->{NAM}->{$chnm}->[USED]=1;
        $fnt->{NO}->[$unused]->[1]=$chnm;
        $widtbl=CacheWid($cft);


G. Branden Robinson <gbranden>
Group administrator
Tue 23 Aug 2022 09:36:37 AM UTC, comment #3: 


commit f275b477e0f49d180224a77271c1b68e913b71b5
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Mon Aug 22 23:30:40 2022 +0100

    Bug #62934 - after glyph remapped mark it as used

    When many glyphs are remapped from code points above 255
    such as writing documents in cyrillic with the U-TR fonts,
    gropdf starts reusing code points in the range 128-255.
    If subsequently one of those code points is actually required,
    such as \(em (code 138), and it has been replaced by a
    cyrillic, then it needs to be mapped to another free code.

    To determine if a particular code point is free each glyph
    has a USED flag. The bug was caused because after remapping
    \(em to the next free glyph the USED flag was not set. So the
    next new cyrillic character to be entered was given the same
    code point as had been allocated to \(em.

    * src/devices/gropdf/gropdf.pl: Set the USED flag on remapped
    glyphs.

    Fixes <https://savannah.gnu.org/bugs/?62934>.

    Thanks to Nikita Ivanov for spotting the problem and testing
    the fix.


G. Branden Robinson <gbranden>
Group administrator
Mon 22 Aug 2022 05:06:42 PM UTC, comment #2: 

The patch you provided works, thank you!

Nikita Ivanov <nikitaivanov>
Mon 22 Aug 2022 04:02:53 PM UTC, comment #1: 

This is a remapping issue. If glyphs have a code point above 255 they have to be remapped to a lower number which is not in use, but if the code it is mapped to belongs to a glyph which is used later in the document, then that original glyph needs to be mapped to another unused code as well, this is remapping.

If you run groff with -P-d flag the pdf contains debugging information. Useful information can be gained by using the command "egrep -a 'AAA|MMM' broken.pdf". This is what is produced:-


% AAA Assign u0420 (304) to 10
% AAA Assign u0430 (362) to 11
% AAA Assign u0437 (572) to 12
% AAA Assign u043D (485) to 13
% AAA Assign u043E (581) to 14
% AAA Assign u0431 (565) to 15
% AAA Assign u0440 (402) to 16
% AAA Assign u044B (355) to 17
% AAA Assign u0438_0306 (535) to 18
% AAA Assign u0438 (433) to 19
% AAA Assign u0433 (277) to 20
% AAA Assign u0442 (404) to 21
% AAA Assign u043F (518) to 22
% AAA Assign u0432 (426) to 23
% AAA Assign u043C (444) to 24
% AAA Assign u0447 (516) to 25
% AAA Assign u043B (583) to 26
% AAA Assign u0441 (568) to 27
% AAA Assign u0435 (554) to 28
% AAA Assign u0434 (336) to 29
% AAA Assign u0444 (434) to 30
% AAA Assign u044E (493) to 31
% AAA Assign u0446 (477) to 127
% AAA Assign u0443 (539) to 145
% AAA Assign u044C (500) to 160


At this point all the /undef glyphs have been used up, now look for glyphs between 128-255 which have not been already "used" (flag in glyph structure USED).


% AAA Assign u0448 (338) to 128
% AAA Assign u0445 (555) to 129
% AAA Assign u044F (340) to 130
% AAA Assign u043A (401) to 131
% AAA Assign u0449 (290) to 132
% AAA Assign u0436 (437) to 133
% AAA Assign u0410 (390) to 134
% AAA Assign u042F (414) to 135
% AAA Assign u0411 (318) to 136
% AAA Assign u044A (509) to 137
% AAA Assign u041A (566) to 138  <-- 138 is code point for \(em
% AAA Assign u044D (264) to 146
% AAA Assign em (138) to 147     <-- we have an \(em but its position has been taken by \[u041a]
% MMM Remap 138 to 147           <-- So choose code 147 for \(em instead
% AAA Assign u0412 (260) to 147  <-- Here's the bug, 147 is \(em now, so it should be USED (but isn't!!)
% AAA Assign u0421 (496) to 148
% AAA Assign u041F (463) to 149
% AAA Assign u041C (447) to 150
% AAA Assign u0414 (502) to 151
% AAA Assign u0435_0308 (548) to 152
% AAA Assign a" (147) to 153    <-- Another \(em but it doesn't match current occupant so gets remapped again.
% MMM Remap 147 to 153


So it is clear that when the first \[em] is received and it is mapped to 147 the USED flag is not being set, since the next glyph which needs moving uses 147 as well. This patch fixes the issue (for me, please test).


--- gropdf      2022-08-22 15:07:10.218388264 +0100
+++ /usr/bin/gropdf     2022-08-22 15:02:47.293997392 +0100
@@ -3772,6 +3772,7 @@
        my $glyph=$fnt->{NO}->[$unused]->[1];
        delete($fontlst{$cft}->{CACHE}->{$cftsz});
        $fnt->{NAM}->{$chnm}->[ASSIGNED]=$unused;
+       $fnt->{NAM}->{$chnm}->[USED]=1;
        $fnt->{NO}->[$unused]->[1]=$chnm;
        $widtbl=CacheWid($cft);

 
The reason your good version works is because the first use of \[em] happens before codepoint 138 is assigned one of the cyrillic glyphs, so the USED flag has already been set, preventing it being used for mapping.


Deri James <deri>
Group Member
Sun 21 Aug 2022 08:42:49 PM UTC, original submission:  

GROFF VERSION:
1.23.0.rc1.2845-342a1-dirty

OS:
GNU/Linux

COMPILER:
clang

COMMAND LINE:
groff works.troff -k -Tpdf
groff broken.troff -k -Tpdf

DESCRIPTION OF INCORRECT BEHAVIOUR:
Sometimes \(em character turns into a different character when rendered with gropdf (grops works fine). I've only encountered the bug when writing documents with Cyrillic characters.

There are two files attached: works.troff and broken.troff. The only difference between them is that in the broken version the \(em characher appears only the 3rd paragraph while in the working version it appears in the first. The document also is rendered fine if the first \(em characher appears in the second paragraph.

It looks like that \(em characters are rendered properly only if the first em-dash appears in the beginning of a document.

It's probably gropdf problem because in the groff out Cem lines are there.

Nikita Ivanov <nikitaivanov>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53598:  broken.troff added by nikitaivanov (6KiB - application/octet-stream)
file #53599:  works.troff added by nikitaivanov (6KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by deri (Posted a comment)
  • -email is unavailable- added by nikitaivanov (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-08-23 gbranden Planned Release1.20.0 1.23.0
    2022-08-23 deri Planned Release1.23.0 1.20.0
    2022-08-23 gbranden CategoryGeneral Driver gropdf
        StatusReady for Merge Fixed
        Assigned toNone deri
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
        Summarygropdf: sometimes \(em becomes a different char when rendered with gropdf [gropdf] sometimes \(em becomes a different char when rendered
    2022-08-22 deri StatusNeed Info Ready for Merge
    2022-08-22 deri StatusNone Need Info
    2022-08-21 nikitaivanov Attached File- Added broken.troff, #53598
        Attached File- Added works.troff, #53599

    Back to the top

    Powered by Savane 3.12.
    Corresponding source code