latex3 / pdfresources

LaTeX PDF resource management
LaTeX Project Public License v1.3c
22 stars 8 forks source link

problem with `shipout` + `\pdfmanagement_add:nnn` #81

Open lemzwerg opened 1 week ago

lemzwerg commented 1 week ago

[I'm not sure whether this is a problem with \pdfmanagement_add:nnn, a problem with \AddToHook{shipout}, or lacking documentation. ]

[LaTeX2e <2024-11-01>] [L3 programming layer <2024-11-02>]

Consider the following example to rotate all odd pages.

\RequirePackage{pdfmanagement-testphase}
\DocumentMetadata{}
\documentclass[landscape]{article}

\ExplSyntaxOn
\AddToHook{shipout}{%
  \ifodd\thepage
  \else
    \pdfmanagement_add:nnn{ThisPage}{Rotate}{180}%
  \fi}
\ExplSyntaxOff

\begin{document}
foo\newpage
bar\newpage
baz\newpage
urgh
\end{document}

I would expect that the above code rotates the first page, too – at the moment of the shipout, \thepage already has been set to the next page number, or so I think. However, it doesn't work, and I don't see why. My guess is that it is not suitable to use \thepage for the test. What should be used instead?

u-fischer commented 1 week ago

There is no "moment of the shipout". Shipout is a rather complex operation with lots of things happening there. The code that handles the ThisPage setting is executed there too -- in a kernel command just after the shipout/background command has been handled. That means that you are too late if you put your settings into the shipout hook. You need the shipout/background hook. You do not to load the testphase package, that is now done by \DocumentMetadata. And to test the page use \c@page or \the\value{page}.

\DocumentMetadata{}
\documentclass[landscape]{article}

\ExplSyntaxOn
\AddToHook{shipout/background}{%
  \ifodd\the\value{page}%
  \else
    \pdfmanagement_add:nnn{ThisPage}{Rotate}{180}%    
  \fi}
\ExplSyntaxOff

\begin{document}
foo\newpage
bar\newpage
baz\newpage
urgh
\end{document}
FrankMittelbach commented 1 week ago

Not sure @u-fischer I agree. As documented, shipout/background has a very specific purpose, ie

It should therefore only receive \put commands or other commands suitable in a picture environment and the vertical coordinate values would normally be negative.

The right place to manipulate the box to be shipped out should be the shipout hook, at least that's the intention, but perhaps that has to play with \ShipoutBox then

lemzwerg commented 1 week ago

@u-fischer, thanks! Note, however, that your code should actually be

\DocumentMetadata{}
\documentclass[landscape]{article}

\ExplSyntaxOn
\AddToHook{shipout/background}{%
  \ifodd\the\value{page}%
    \pdfmanagement_add:nnn{ThisPage}{Rotate}{180}%    
  \fi}
\ExplSyntaxOff

\begin{document}
foo\newpage
bar\newpage
baz\newpage
urgh
\end{document}

so that odd pages (including the first one) get rotated.

@FrankMittelbach, the advantage of Ulrike's code is that it actually works :slightly_smiling_face: Using the shipout hook fails for the first page. I agree, however, that shipout is the natural choice for such operations – maybe the kernel code can be adjusted to enable this?

u-fischer commented 1 week ago

@FrankMittelbach But the box is not manipulated. The code sets an attribute for the current Page object.

11 0 obj
<<
/Type /Page
/Contents 12 0 R
/Resources 10 0 R
/MediaBox [0 0 792 612]
/Rotate 180  <-----------------------------------
/Parent 9 0 R
>>
endobj

The code for it is in the shipout/background box (with other code related to PDF ressources that needs to be in a box) in the \@kernel@after@shipout@background command and it is fine if more is added. A picture environment does not require that everything in it is in a \put command but if you want you can use \put(0,0){\pdfmanagement_add:nnn{ThisPage}{Rotate}{180}}.

u-fischer commented 1 week ago

@lemzwerg

so that odd pages (including the first one) get rotated.

Sorry I didn't read the text, only used your code.

that shipout is the natural choice for such operations – maybe the kernel code can be adjusted to enable this?

PDF resources needs to be for some engines inside a box, that's why there are all handled inside the shipout/background box.

lemzwerg commented 1 week ago

PDF resources needs to be for some engines inside a box, that's why there are all handled inside the shipout/background box.

OK. Then I suggest to improve the documentation of \pdfmanagement_add:nnn{ThisPage}, mentioning that PDF attribute manipulation (like rotation) that doesn't change the shipout box should be done with the ship/background hook.

u-fischer commented 1 week ago

I added a sentence.

FrankMittelbach commented 1 week ago

The code for it is in the shipout/background box (with other code related to PDF ressources that needs to be in a box) in the \@kernel@after@shipout@background command and it is fine if more is added. A picture environment does not require that everything in it is in a \put command but if you want you can use \put(0,0){\pdfmanagement_add:nnn{ThisPage}{Rotate}{180}}.

I'm aware that this is "technically" not necessary to use put commands, I'm concerned about the user perspective. Conceptually, I don't think that should happen in that hook. If that code is supposed but executed within the box it is meant for it would be more understandable (imo) if it is done as \setbox\ShipoutBox\vbox{\unvbox\ShipoutBox ...} in the shipouthook, but, of course, that is unnecessarily inefficient.

So I would suggest to make an explicit statement in the documentation of shipout/background that it can also be used for pdfmanagement operations, or document that as part of of \pdfmanagement_add:nnn, or perhaps even add a dedicated hook for that purpose that is documented as meant for this purpose.

car222222 commented 1 week ago

Yes: an extra hook, with a suitable name, sounds good in principal for such non-background actions.