[bug #64624] [pic] doesn't restore fill color after writing output

[G. Branden Robinson]

Update of bug #64624 (project groff):

                  Status:               Confirmed => Need Info              
             Assigned to:                    None => gbranden               

    _______________________________________________________

Follow-up Comment #11:

[comment #10 comment #10:]
> [comment #9 comment #9:]
> > Apart from the semantics of "fill color" being different for _nroff_
> > devices, I don't think this report has much to do with _grotty_.
> 
> I suppose the underlying question is whether nroff's "fill color" semantics
make sense.  My (limited) understanding is that fill color is intended to
affect drawing commands, so nroff's decision to interpret it as changing text
background color is questionable.

Okay.  It strains orthogonality, I'll grant, but given that:

1.  No known _nroff_ device in any implementation supports filled geometric
objects, and
2.  No known _troff_ device in any implementation supports the "character
cell" concept, let alone assignment of background color thereto,

...it seems to me fairly harmless.

I'll grant, if we add ReGIS support to _grotty_(1), our lives will become more
interesting.

(I guess the Tektronix 4014 came closest to straddling this picket fence, but
it was a monochrome device.)

> > _pic_ moved something and didn't put it back when it was done.
> 
> Are changes to fill color expected to be persistent across pic calls?

Why would they?  If you use different typefaces for text inside _tbl_, _eqn_,
or _pic_ regions, general expectations seem to be that preprocessor regions
don't change the face of following material.

It won't do to claim (as one might) that the fill color applies only to _pic_
regions in the first place.  It doesn't: _pic_ is a convenience for producing
`\D` escape sequences less tediously, as _eqn_ is for setting math.

>  If the user runs a pic region whose code explicitly changes the fill color,
should they expect that change to still be in effect in a subsequent pic
region in the document?  Or should they expect every pic invocation to restore
settings upon exit to what they were before that invocation?

The latter, I believe.

> The problem of this ticket seems a little more focused than that: upon
issuing only an "arrow" command, a user probably shouldn't expect pic to alter
the fill color at all.

As I understand it, the problem is the arrowhead, which demands filling (by
default).

>  When pic itself chooses to set a fill color, without the user asking for
it, that seems a clearer case of pic needing to put things back where they
came from.

I agree with this.

Here is a patch to implement what I believe to be user expectations.  It also
solves the problem you reported.  No new warnings are produced, and scrolling
through the 39 pages of "pic.ps" reveals no obvious problems to me.


diff --git a/src/preproc/pic/troff.cpp b/src/preproc/pic/troff.cpp
index c66e43e2f..dcb19f42f 100644
--- a/src/preproc/pic/troff.cpp
+++ b/src/preproc/pic/troff.cpp
@@ -271,6 +271,9 @@ inline position troff_output::transform(const position
&pos)
 // If this register is defined, geqn won't produce '\x's.
 #define EQN_NO_EXTRA_SPACE_REG "0x"
 
+#define SAVED_STROKE_COLOR_STR "gpic*saved-stroke-color"
+#define SAVED_FILL_COLOR_STR "gpic*saved-fill-color"
+
 void troff_output::start_picture(double sc,
                                 const position &ll, const position &ur)
 {
@@ -291,6 +294,10 @@ void troff_output::start_picture(double sc,
   // This guarantees that if the picture is used in a diversion it will
   // have the right width.
   printf("\\h'%.3fi'\n.sp -1\n", width);
+  (void) puts(".ds " SAVED_STROKE_COLOR_STR " default ");
+  (void) puts(".ds " SAVED_FILL_COLOR_STR " default ");
+  (void) puts(".if !'\\n[.m]'' .ds " SAVED_STROKE_COLOR_STR " \\n[.m]");
+  (void) puts(".if !'\\n[.M]'' .ds " SAVED_FILL_COLOR_STR " \\n[.M]");
 }
 
 void troff_output::finish_picture()
@@ -303,6 +310,8 @@ void troff_output::finish_picture()
   printf(".if \\n(" FILL_REG " .fi\n");
   printf(".br\n");
   printf(".nr " EQN_NO_EXTRA_SPACE_REG " 0\n");
+  (void) puts(".gcolor \\*[" SAVED_STROKE_COLOR_STR "]");
+  (void) puts(".fcolor \\*[" SAVED_FILL_COLOR_STR "]");
   // this is a little gross
   set_location(current_filename, current_lineno);
   if (want_flyback)


Thoughts?


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?64624>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/