Manage a stack of local graphic state (started about: set_dash_pattern())

[gmischler]

There's a FPDF.dashed_line() method, which is essentially just a call to FPDF.line() wrapped in two _set_dash() calls.

If you want a dashed line style for any other graphics, you'd have to violate the conventions and call the formally private _set_dash() on your own, which is not a nice situation to be in.

Wouldn't it make more sense to turn _set_dash() into a formal part of the public API as set_dash(), so it can be used with all apropriate geometry?

I am guessing that _set_dash() also shouldn't be used within a rotation context, like the other styling operations. This would mean that dashed_line() is only of limited practical use, and should really be marked as obsolete.

[Lucas-C]

I'm OK with your analysis and your suggestion to make set_dash() public and deprecate dashed_line(). Maybe set_dashed() would be a better name, what do you think?

I'd recommend to:

• add documentation on set_dash-) + line() / rect() / etc. combined usage
• add a PendingDeprecationWarning in dashed_line(), hinting set_dash() as a replacement. We'll let it live for a few versions before actually removing it.

Do you want to submit a PR for this?

[gmischler]

Looking at the specs, it seems like set_dash_pattern(on, off=0, phase=0) might be the most appropriate name and signature. Apparently only simple on-off patterns are directly supported, so if you need eg. a dash-dot pattern, you'll have to either draw it manually, or place two differently patterned lines on top of each other.

So far I've avoided fiddling with the PDF generating side of things. I'll see if I can find some time, if nobody beats me to it...

[gmischler]

Looking at the internals a bit more (been decades since I checked out PostScript syntax...), I'm starting to wonder why the ability to create a local graphics state on the stack is not exposed more conveniently.

I'm thinking of the ability to do something like this

    with pdf.local_state():
        pdf.set_draw_color(c1)
        pdf.set_fill_color(c2)
        pdf.rect(x,y,w,h,state="FD")

without polluting the already existing surrounding graphics state. If I'm not mistaken, this could be implemented in about four or five lines of code, and using it might simplify some of the existing internal code as well. In fact, I'd be tempted to use this inside templates as a matter of principle.

@Lucas-C (and others): Any thoughts? If used naively, a local graphics context will very slightly increase the file size. But if the code after it is aware that it doesn't need to reestablish the previous state, it can reduce the file size significantly. Besides that, do local graphics contexts have any noticeable effect on the performance of PDF readers?

[gmischler]

Ok, so it turns out to be more than five lines, but most of it is just straightforward boilerplate. I hadn't previously realized how much local state FPDF() is maintaining itself. The solution to that is simply to maintain our own stack of graphical state items (have I missed any relevant ones?). I experimented with a subclass do demonstrate the concept:

from contextlib import contextmanager

from fpdf import FPDF, check_page

class MyFpdf(FPDF):

    def __init__(self, *args, **kwargs):
        # All relevant local state variables become lists for stacking
        self._draw_color = ["0 G"]
        self._fill_color = ["0 g"]
        self._text_color = ["0 g"]
        self._underline = [0]
        self._font_style = [""]
        self._font_stretching = [100]
        self._font_family = [""]
        self._font_size_pt = [12]
        self._font_size = ['dummy'] # factor k not known yet in subclass
        super().__init__(*args, **kwargs)

    ### boilerplate properties management
    def _get_draw_color(self):
        return self._draw_color[-1]
    def _set_draw_color(self, v):
        self._draw_color[-1] = v
    draw_color = property(_get_draw_color, _set_draw_color)

    def _get_fill_color(self):
        return self._fill_color[-1]
    def _set_fill_color(self, v):
        self._fill_color[-1] = v
    fill_color = property(_get_fill_color, _set_fill_color)

    def _get_text_color(self):
        return self._text_color[-1]
    def _set_text_color(self, v):
        self._text_color[-1] = v
    text_color = property(_get_text_color, _set_text_color)

    def _get_underline(self):
        return self._underline[-1]
    def _set_underline(self, v):
        self._underline[-1] = v
    underline = property(_get_underline, _set_underline)

    def _get_font_size_pt(self):
        return self._font_size_pt[-1]
    def _set_font_size_pt(self, v):
        self._font_size_pt[-1] = v
    font_size_pt = property(_get_font_size_pt, _set_font_size_pt)

    def _get_font_size(self):
        return self._font_size[-1]
    def _set_font_size(self, v):
        self._font_size[-1] = v
    font_size = property(_get_font_size, _set_font_size)

    def _get_font_family(self):
        return self._font_family[-1]
    def _set_font_family(self, v):
        self._font_family[-1] = v
    font_family = property(_get_font_family, _set_font_family)

    def _get_font_style(self):
        return self._font_style[-1]
    def _set_font_style(self, v):
        self._font_style[-1] = v
    font_style = property(_get_font_style, _set_font_style)

    def _get_font_stretching(self):
        return self._font_stretching[-1]
    def _set_font_stretching(self, v):
        self._font_stretching[-1] = v
    font_stretching = property(_get_font_stretching, _set_font_stretching)

    # hack to circumvent check in subclass
    @property
    def _rotating(self):
        return False
    @_rotating.setter
    def _rotating(self, x):
        pass

    # add a stack level
    def _push_local_stack(self):
        self._draw_color.append(self._draw_color[-1])
        self._fill_color.append(self._fill_color[-1])
        self._text_color.append(self._text_color[-1])
        self._underline.append(self._underline[-1])
        self._font_size_pt.append(self._font_size_pt[-1])
        self._font_size.append(self._font_size[-1])
        self._font_family.append(self._font_family[-1])
        self._font_style.append(self._font_style[-1])
        self._font_stretching.append(self._font_stretching[-1])

    # remove a stack level
    def _pop_local_stack(self):     
        del self._draw_color[-1]
        del self._fill_color[-1]
        del self._text_color[-1]
        del self._underline[-1]
        del self._font_size_pt[-1]
        del self._font_size[-1]
        del self._font_family[-1]
        del self._font_style[-1]
        del self._font_stretching[-1]

    # the original motivation for all of this...
    @check_page
    @contextmanager
    def local_context(self):
        self._push_local_stack()
        self._out('\nq ')
        yield
        self._out(' Q\n')
        self._pop_local_stack()

    # a tiny modification in the name of flexibility
    @contextmanager
    def rotation(self, *args, **kwargs):
        with super().rotation(*args, **kwargs):
            self._push_local_stack()
            yield
            self._pop_local_stack()

# testing
p = MyFpdf()
p.add_page()
p.set_font('helvetica', '', 12)
p.set_text_color(0x00, 0xFF, 0x00)
p.set_fill_color(0xFF, 0x88, 0xFF)
p.set_y(20)
p.cell(txt='outer 01', ln=1, fill=True)
with p.local_context():
    p.set_font('courier', 'BIU', 30)
    p.set_text_color(0xFF, 0x00, 0x00)
    p.set_fill_color(0xFF, 0xFF, 0x00)
    p.cell(txt='inner 01', ln=1, fill=True)
    p.set_x(70)
    with p.rotation(30, p.get_x(), p.get_y()):
        p.set_fill_color(0x00, 0xFF, 0x00)
        p.cell(txt='inner 02', ln=1, fill=True)
    p.set_stretching(150)
    p.cell(txt='inner 03', ln=1, fill=True)
p.cell(txt='outer 02', ln=1, fill=True)
p.output('contexttest.pdf')

Yes, using styling operations within a rotation context is no problem at all anymore. Which means that template.py may become less convoluted again.

There are a few other context managers that I think won't be affected (but could be easily instrumented otherwise). The internal ._apply_style() should use this stacking mechanism, though.

It might be a good idea to put the stack management code into a seperate file (eg. graphics_state.py), and make it a superclass to FPDF(), since fpdf.py is already long enough.

And this is what the result of the test sequence above looks like: contexttest.pdf

[Lucas-C]

Any thoughts?

fpdf2 is intended to be a "high level" lib to build PDFs. That being said, why not providing more "low level" control to its users, as long as it doesn't make "basic things" more complex to do.

If used naively, a local graphics context will very slightly increase the file size. But if the code after it is aware that it doesn't need to reestablish the previous state, it can reduce the file size significantly. Besides that, do local graphics contexts have any noticeable effect on the performance of PDF readers?

As far as I know, they do not have any significant impact on performances.

The solution to that is simply to maintain our own stack of graphical state items (have I missed any relevant ones?).

This looks promising! I have a few minor concerns regarding this approach:

• backward compatibility: .draw_color, .fill_color, .text_color, .underline, .font_style, etc. are currently public attributes of FPDF, and they are sometimes directly modified in users code. EDIT: by mad, I see now that you have defined @property methods to ensure backward compatibility: good job!

• fpdf2 source code readability: moving the code related to handling of the graphical state in a separate module is an excellent idea for sure! Still, the price to pay in term of maintainability seems a bit high to me, in order to build this new nice feature of .local_context() by turning attributes into lists... What do you think of this alternative approach @gmischler :

class GraphicalState(NamedTuple):
    draw_color: str = "0 G"
    fill_color: str = "0 g"
    text_color: str = "0 g"
    underline: bool = False
    font_style: str = ""
    font_stretching: int = 100
    font_family: str = ""
    font_size_pt: int = 12

class FPDF:
    def __init__(self, *args, **kwargs):
        self._graphical_state = [GraphicalState()]
        ...

    ### User-friendly methods to manage the current graphical state:
    @property
    def draw_color(self):
        return self._graphical_state[-1].draw_color

    @draw_color.setter
    def draw_color(self, draw_color):
        self._graphical_state[-1].draw_color = draw_color

Rewriting fpdf2 code in order to introduce graphical stacks everywhere looks colossal to me! If you feel like tackling this challenge, I'd be very happy to merge a PR implementing your suggestion 😉

[Lucas-C]

@allcontributors please add @gmischler for ideas

[allcontributors[bot]]

@Lucas-C

I've put up a pull request to add @gmischler! :tada:

[Lucas-C]

Now that I think about it: I wonder if your idea @gmischler hasn't already been implemented by @torque in https://github.com/PyFPDF/fpdf2/pull/196 as FPDF.drawing_context / drawing.GraphicsStateDictRegistry

The goal is a bit different as the focus there is on SVG paths, but it may be interesting to assemble both ideas. Hence I'd be inclined to wait until @torque contribution is merged before starting to implement a local_context() method.

[gmischler]

* **backward compatibility**: `.draw_color`, `.fill_color`, `.text_color`, `.underline`, `.font_style`, etc.
  are currently public attributes of `FPDF`, and they are sometimes directly modified in users code.
  **EDIT**: by mad, I see now that you have defined `@property` methods to ensure backward compatibility: good job!

I went with the properties approach to avoid having to modify all the methods using those values. That they can remain publicly visible happens to be an additional benefit...

class GraphicalState(NamedTuple):
   ...

class FPDF:
    def __init__(self, *args, **kwargs):
        self._graphical_state = [GraphicalState()]
        ...

Yes, I am now leaning towards this type of solution as well. I started with a smaller number of variables, but as I discovered more that needed to be covered (dash_pattern will be another one), packing them all together became more and more desirable.

Now that I think about it: I wonder if your idea @gmischler hasn't already been implemented by @torque in #196 as FPDF.drawing_context / drawing.GraphicsStateDictRegistry

That is one massive PR, which I'm not going to even try to understand... Might be best if @torque can comment and explain any possible overlaps and/or differences.

[Lucas-C]

The initial topic of this issue has been fixed by https://github.com/PyFPDF/fpdf2/pull/260

@gmischler: do you want to keep that issue open to discuss your idea of a stack of graphical state items? I should probably rename it then...

[gmischler]

Yeah, I've just renamed it. It would be really helpful if @torque wants to comment. I increasingly suspect that his drawing.GraphicsStateDictRegistry serves a similar purpose, but is used purely within his graphics rendering pipeline, shielding the rest of the library from the effects of its operations.

His commit from Sep 5, 2021 includes this comment:

This ignores some of the existing APIs that fpdf provides for e.g. setting the stroke/fill color (though it will default to the colors set by those API calls) in order to provide more descriptive objects to accomplish the same purpose.

So it seems once my context stack is added, his code will simply use the values from the current stack level. We'll see if my interpretation is correct...

[torque]

summary of my thoughts: this is conceptually similar to the drawing.GraphicsStyle class I came up with but I think trying to unify the two of them may require a fairly involved plumbing change in fpdf itself and I don't know if it would be worth it. As far as I understood from reading this, this proposal is primarily an API change and wouldn't make much of a difference to how the resulting PDF is generated, so I wouldn't expect it to interfere with the drawing module.

I skimmed through this thread fairly quickly so apologies if I overlooked something, but the proposed GraphicalState class above is similar in concept to the fpdf.drawing.GraphicsStyle class that I am introducing as part of the drawing support. However, the drawing GraphicsStyle class doesn't include any of the text styling information, so maybe there isn't that much overlap in terms of the actual implementation.

There are two main goals I have tried to design the GraphicsStyle for in the drawing pull request: the first is to support nested graphics states using PDF's builtin functionality (its q and Q operators) to avoid having to track every nested state change on the python side (i.e. let the PDF viewer handle appying and removing graphics styles like colors, line thickness, and so on). The other goal was to support some graphical parameters that cannot be emitted by inline operators (e.g. transparency can only be specified via a reference to a different object, a "graphics state parameter dictionary"). The drawing.GraphicsStateDictRegistry tracks the created graphics state parameter dictionaries for later use because they are written to the PDF at a different stage than the drawing that references them.

So it seems once my context stack is added, his code will simply use the values from the current stack level.

I believe this statement is correct.

[gmischler]

I believe this statement is correct.

Thanks for confirming that, @torque. So I'll just go ahead, and we'll see how it all plays together in the end.