Merge pull request #5992 from jfbu/latex_safe_graphics

LaTeX: safer image inclusion

CHANGES

@@ -62,6 +62,9 @@ Incompatible changes
 * websupport: unbundled from sphinx core. Please use sphinxcontrib-websupport
 * C++, the visibility of base classes is now always rendered as present in the
   input. That is, ``private`` is now shown, where it was ellided before.
+* LaTeX: graphics inclusion of oversized images rescales to not exceed
+  the text width and height, even if width and/or height option were used.
+  (refs: #5956)
 
 Deprecated
 ----------

sphinx/texinputs/sphinx.sty

@@ -707,53 +707,79 @@
 
 %% GRAPHICS
 %
-% \sphinxincludegraphics defined to resize images larger than the line width,
-% except if height or width option present.
+% \sphinxincludegraphics resizes images larger than the TeX \linewidth (which
+% is adjusted in indented environments), or taller than a certain maximal
+% height (usually \textheight and this is reduced in the environments which use
+% framed.sty to avoid infinite loop if image too tall).
+%
+% In case height or width options are present the rescaling is done
+% (since 2.0), in a way keeping the width:height ratio either native from
+% image or from the width and height options if both were present.
 %
-% If scale is present, rescale before fitting to line width. (since 1.5)
-\newbox\spx@image@box
-\newcommand*{\sphinxincludegraphics}[2][]{%
-  \in@{height}{#1}\ifin@\else\in@{width}{#1}\fi
-  \ifin@ % height or width present
-    \includegraphics[#1]{#2}%
-  \else % no height nor width (but #1 may be "scale=...")
-    \setbox\spx@image@box\hbox{\includegraphics[#1,draft]{#2}}%
-    \ifdim \wd\spx@image@box>\linewidth
-      \setbox\spx@image@box\box\voidb@x % clear memory
-      \includegraphics[#1,width=\linewidth]{#2}%
-    \else
-      \setbox\spx@image@box\box\voidb@x % clear memory
-      \includegraphics[#1]{#2}%
-    \fi
-  \fi
-}
-% \sphinxsafeincludegraphics resizes images larger than the line width,
-% or taller than about the text height (whether or not height/width options
-% were used). This is requested to avoid a crash with \MakeFramed as used by
-% sphinxShadowBox (topic/contents) and sphinxheavybox (admonitions), and also
-% by sphinxVerbatim (but a priori no image inclusion there).
 \newdimen\spx@image@maxheight
-% default maximal setting will get reduced by sphinxShadowBox/sphinxheavybox
 \AtBeginDocument{\spx@image@maxheight\textheight}
+
+% box scratch register
+\newdimen\spx@image@box
 \newcommand*{\sphinxsafeincludegraphics}[2][]{%
-    \gdef\spx@includegraphics@options{#1}%
+    % #1 contains possibly width=, height=, but no scale= since 1.8.4
     \setbox\spx@image@box\hbox{\includegraphics[#1,draft]{#2}}%
-    \in@false
+    \in@false % use some handy boolean flag
     \ifdim \wd\spx@image@box>\linewidth
-      \g@addto@macro\spx@includegraphics@options{,width=\linewidth}%
-      \in@true
+      \in@true % flag to remember to adjust options and set box dimensions
+      % compute height which results from rescaling width to \linewidth
+      % and keep current aspect ratio. multiply-divide in \numexpr uses
+      % temporarily doubled precision, hence no overflow. (of course we
+      % assume \ht is not a few sp's below \maxdimen...(about 16384pt).
+      \edef\spx@image@rescaledheight % with sp units
+           {\the\numexpr\ht\spx@image@box
+                        *\linewidth/\wd\spx@image@box sp}%
+      \ifdim\spx@image@rescaledheight>\spx@image@maxheight
+        % the rescaled height will be too big, so it is height which decides
+        % the rescaling factor
+        \def\spx@image@requiredheight{\spx@image@maxheight}% dimen register
+        \edef\spx@image@requiredwidth % with sp units
+         {\the\numexpr\wd\spx@image@box
+                      *\spx@image@maxheight/\ht\spx@image@box sp}%
+        % TODO: decide if this commented-out block could be needed due to
+        % rounding in numexpr operations going up
+        % \ifdim\spx@image@requiredwidth>\linewidth
+        %     \def\spx@image@requiredwidth{\linewidth}% dimen register
+        % \fi
+      \else
+        \def\spx@image@requiredwidth{\linewidth}% dimen register
+        \let\spx@image@requiredheight\spx@image@rescaledheight% sp units
       \fi
-    % no rotation, no need to worry about depth
+    \else
+      % width is ok, let's check height
       \ifdim\ht\spx@image@box>\spx@image@maxheight
-      \g@addto@macro\spx@includegraphics@options{,height=\spx@image@maxheight}%
         \in@true
+        \edef\spx@image@requiredwidth % with sp units
+            {\the\numexpr\wd\spx@image@box
+                         *\spx@image@maxheight/\ht\spx@image@box sp}%
+        \def\spx@image@requiredheight{\spx@image@maxheight}% dimen register
       \fi
+    \fi % end of check of width and height
     \ifin@
-      \g@addto@macro\spx@includegraphics@options{,keepaspectratio}%
-    \fi
+      \setbox\spx@image@box
+      \hbox{\includegraphics
+            [%#1,% contained only width and/or height and overruled anyhow
+            width=\spx@image@requiredwidth,height=\spx@image@requiredheight]%
+            {#2}}%
+      % \includegraphics does not set box dimensions to the exactly
+      % requested ones, see https://github.com/latex3/latex2e/issues/112
+      \wd\spx@image@box\spx@image@requiredwidth
+      \ht\spx@image@box\spx@image@requiredheight
+      \leavevmode\box\spx@image@box
+    \else
+      % here we do not modify the options, no need to adjust width and height
+      % on output, they will be computed exactly as with "draft" option
       \setbox\spx@image@box\box\voidb@x % clear memory
-    \expandafter\includegraphics\expandafter[\spx@includegraphics@options]{#2}%
+      \includegraphics[#1]{#2}%
+    \fi
 }%
+% Use the "safe" one by default (2.0)
+\def\sphinxincludegraphics{\sphinxsafeincludegraphics}
 
 
 %% FIGURE IN TABLE
@@ -1374,7 +1400,6 @@
            +2\sphinxshadowsep
            +\sphinxshadowsize
            +\baselineskip\relax
-   \let\sphinxincludegraphics\sphinxsafeincludegraphics
    % configure framed.sty not to add extra vertical spacing
    \ltx@ifundefined{OuterFrameSep}{}{\OuterFrameSep\z@skip}%
    % the \trivlist will add the vertical spacing on top and bottom which is
@@ -1461,7 +1486,6 @@
    -\dimexpr2\FrameRule
            +2\FrameSep
            +\baselineskip\relax % will happen again if nested, needed indeed!
-   \let\sphinxincludegraphics\sphinxsafeincludegraphics
    % configure framed.sty's parameters to obtain same vertical spacing
    % as for "light" boxes. We need for this to manually insert parskip glue and
    % revert a skip done by framed before the frame.

tests/roots/test-latex-includegraphics/conf.py

@@ -0,0 +1,49 @@
+master_doc = 'index'
+
+exclude_patterns = ['_build']
+
+latex_elements = {
+    'preamble': r'''
+\makeatletter
+\def\dividetwolengths#1#2{\the\dimexpr
+    \numexpr65536*\dimexpr#1\relax/\dimexpr#2\relax sp}%
+\newwrite\out
+\immediate\openout\out=\jobname-dimensions.txt
+\def\toout{\immediate\write\out}
+\def\getWfromoptions #1width=#2,#3\relax{\def\WidthFromOption{#2}}%
+\def\getHfromoptions #1height=#2,#3\relax{\def\HeightFromOption{#2}}%
+\def\tempincludegraphics[#1]#2{%
+    \sphinxsafeincludegraphics[#1]{#2}%
+    \edef\obtainedratio
+       {\dividetwolengths\spx@image@requiredheight\spx@image@requiredwidth}%
+    \getWfromoptions#1,width=,\relax
+    \getHfromoptions#1,height=,\relax
+    \def\ratiocheck{}%
+    \ifx\WidthFromOption\empty\else
+    \ifx\HeightFromOption\empty\else
+      \edef\askedforratio{\dividetwolengths\HeightFromOption\WidthFromOption}%
+      \edef\ratiocheck{\dividetwolengths\obtainedratio\askedforratio}%
+    \fi\fi
+    \toout{original options = #1^^J%
+           width = \the\dimexpr\spx@image@requiredwidth,
+           linewidth = \the\linewidth^^J%
+           height = \the\dimexpr\spx@image@requiredheight,
+           maxheight = \the\spx@image@maxheight^^J%
+           obtained  H/W = \obtainedratio^^J%
+    \ifx\ratiocheck\empty
+    \else
+           asked for H/W = \askedforratio^^J%
+           ratio of ratios = \ratiocheck^^J%
+    \fi
+          }%
+    \ifx\ratiocheck\empty
+    \else
+      \ifpdfabsdim\dimexpr\ratiocheck-1pt\relax > 0.01pt
+        \ASPECTRATIOERROR
+      \fi
+    \fi
+}
+\def\sphinxincludegraphics#1#{\tempincludegraphics#1}
+\makeatother
+''',
+}

tests/roots/test-latex-includegraphics/index.rst

@@ -0,0 +1,37 @@
+====================
+Test image inclusion
+====================
+
+Tests with both width and height
+--------------------------------
+
+.. an image with big dimensions, ratio H/W = 1/5
+.. image:: img.png
+   :height: 200
+   :width: 1000
+
+.. topic:: Oversized images
+
+   .. an image with big dimensions, ratio H/W = 5/1
+   .. image:: img.png
+      :height: 1000
+      :width: 200
+
+   .. height too big even if width reduced to linewidth, ratio H/W = 3/1
+   .. image:: img.png
+      :width: 1000
+      :height: 3000
+
+Tests with only width or height
+-------------------------------
+
+.. topic:: Oversized images
+
+   .. tall image which does not fit in textheight even if width rescaled
+   .. image:: tall.png
+      :width: 1000
+
+.. wide image which does not fit in linewidth even after height diminished
+.. image:: sphinx.png
+   :height: 1000
+

tests/test_build_latex.py

@@ -1387,3 +1387,13 @@ def test_default_latex_documents():
     expected = [('index', 'stasi.tex', 'STASI™ Documentation',
                  r"Wolfgang Schäuble \& G'Beckstein.\@{}", 'manual')]
     assert default_latex_documents(config) == expected
+
+
+@skip_if_requested
+@skip_if_stylefiles_notfound
+@pytest.mark.sphinx('latex', testroot='latex-includegraphics')
+def test_includegraphics_oversized(app, status, warning):
+    app.builder.build_all()
+    print(status.getvalue())
+    print(warning.getvalue())
+    compile_latex_document(app)