pgf-tikz / pgf

A Portable Graphic Format for TeX
https://pgf-tikz.github.io/
1.08k stars 104 forks source link

\tikz@transform is compared to \relax but sometimes set to \pgfutil@empty #963

Open muzimuzhi opened 3 years ago

muzimuzhi commented 3 years ago

Brief outline of the bug

\tikz@transform is compared to \relax, but sometimes set to \pgfutil@empty, sometimes set to \relax. https://github.com/pgf-tikz/pgf/blob/88f5a0efce527de4af0c28f829dcb33411e98563/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex#L53-L62

In \tikz@normal@fig: https://github.com/pgf-tikz/pgf/blob/88f5a0efce527de4af0c28f829dcb33411e98563/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex#L3860 In \tikz@children@collected: https://github.com/pgf-tikz/pgf/blob/88f5a0efce527de4af0c28f829dcb33411e98563/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex#L4646-L4649 and more

This may wrongly delay the effect of \tikz@addtransform, for example in https://github.com/pgf-tikz/pgf/issues/889#issuecomment-748494912.

Minimal working example (MWE)

DO NOT USE the following code in your document unless you know what you are doing.

\documentclass{article}
\usepackage{tikz}

\begin{document}
% this is a simplified form of option "name intersections"
% DO NOT USE this in your document
\tikzset{my key/.code={%
  \path[reset cm] coordinate (a);
}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key] (a);
  \draw (a) circle (2pt);
\end{tikzpicture}
\end{document}

Expected: (a) is at (0, 0). Actually: (a) is at (1.5, 0), hence the transformation matrix is still applied.

muzimuzhi commented 3 years ago

\let\tikz@transform=\pgfutil@empty, 10 matches

tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:3111:              \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:3475:              \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:3622:    \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:3860:      \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4503:    \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4646:    \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4709:            \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:5270:    \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/frontendlayer/tikz/libraries/datavisualization/tikzlibrarydatavisualization.code.tex:2183:  \let\tikz@transform=\pgfutil@empty%
tex/generic/pgf/graphdrawing/tex/tikzlibrarygraphdrawing.code.tex:97:  \let\tikz@transform=\pgfutil@empty%

\let\tikz@transform=\relax, 14 matches

tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:362:        \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:586:\def\tikz@do@fade@transform{\let\tikz@transform=\relax\expandafter\tikzset\expandafter{\tikz@fade@transform}}%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:1726:  \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:3122:              \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4065:                \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4649:    \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4713:            \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/tikz.code.tex:4851:  \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibraryanimations.code.tex:241:    \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibraryanimations.code.tex:320:          \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibraryspy.code.tex:85:        \let\tikz@transform=\relax
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibraryspy.code.tex:116:        \let\tikz@transform=\relax%
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibrarypatterns.meta.code.tex:45:      \let\tikz@transform=\relax\pgfkeys{/tikz/.cd,#1}}%
tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibrarypatterns.meta.code.tex:66:        \let\tikz@transform=\relax%

grep'ed by (on macOS, hence a BSD grep)

grep -rni -e '\\let\\tikz@transform[^a-zA-Z@]=\?\\pgfutil@empty' tex
grep -rni -e '\\let\\tikz@transform[^a-zA-Z@]=\?\\relax' tex
hmenke commented 3 years ago

Great work! Thank you for reporting.

marmotghost commented 3 years ago

I agree with all your observations, yet want to add that defining a code that inserts a path as in

\tikzset{my key/.code={%
  \path[reset cm] coordinate (a);
}}

and then use it in another path

\path coordinate[my key] (a);

is very dangerous and can lead to nonsense regardless of the issues you mention. It is basically the same as adding a path in a \pgfextra command, which the pgfmanual explicitly warns users from doing, and which really has many bad side effects.

muzimuzhi commented 3 years ago

@marmotghost Yes, I agree with you and I admit a poor example was given in https://github.com/pgf-tikz/pgf/issues/963#issue-771412930, mainly because I couldn't make a better or more natural one. Then perhaps the implementation of name intersections need some adjustments. Considering it can be used in nearly everywhere and its final step is to construct coordinates, some tricky codes like directly assigning internal representations of a coordinate may be needed. (a bit off-topic) https://github.com/pgf-tikz/pgf/blob/88f5a0efce527de4af0c28f829dcb33411e98563/tex/generic/pgf/frontendlayer/tikz/libraries/tikzlibraryintersections.code.tex#L79-L117

marmotghost commented 3 years ago

I agree with you. It is the case that many different libraries have been written by different authors. These libraries are great and overall work fine, and have been tested over the years by many users. But some of them contain some practice which is perhaps not as clean as it possibly could be. (Even tikz.code.tex contains utils exec in

\def\tikz@edge@from@parent@macro#1#2{
  [style=edge from parent, #1, /utils/exec=\tikz@node@is@a@labeltrue] \tikz@edge@to@parent@path #2}%

but as Henri pointed out using /utils/exec is not really optimal.) Cleaning everything up seems to be a next-to-impossible task because it is very complicated and will almost inevitably break working examples. (I played a bit with decorations but any change there would have to be a drastic one if it is aimed at removing the dimension too large errors once and for all.) So all I want to say is that one may want to warn the readers of the posts that the examples here are not something that they should "try out at home".

muzimuzhi commented 3 years ago

Added two warnings to my poor example.

marmotghost commented 3 years ago

Thanks! Interestingly, if you use the "recommended" way to do the same the problem disappears.

\documentclass{article}
\usepackage{tikz}

\begin{document}
\tikzset{my key/.style={insert path={%
 [reset cm] coordinate (a)
}}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key] (a);
  \draw (a) circle (2pt);
\end{tikzpicture}
\end{document}
muzimuzhi commented 3 years ago

You used (a) twice. Replacing (a) used in \path coordinate[my key] (a); with another name, then the problem comes back.

marmotghost commented 3 years ago

Yes, but you also set a twice. If you call the coordinates differently, you get all sorts of effects (but I agree that this should not be the case).

\documentclass{article}
\usepackage{tikz}
\begin{document}

\tikzset{my key/.code={%
  \path[reset cm] coordinate (a);
}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key] (b);
  \path (a) node[circle,draw]{a} (b) node[circle,draw]{b};
\end{tikzpicture}

\tikzset{my key'/.style={insert path={%
 [reset cm] coordinate (a')
}}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key'] (b');
  \path (a') node[circle,draw]{a} (b') node[circle,draw]{b};
\end{tikzpicture}

\tikzset{my key''/.style={insert path={%
  coordinate[reset cm] (a'')
}}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key''] (b'');
  \path (a'') node[circle,draw]{a} (b'') node[circle,draw]{b};
\end{tikzpicture}

\tikzset{my key'''/.style={insert path={%
  [reset cm] coordinate[reset cm] (a''')
}}}

\begin{tikzpicture}
  \draw (0,0) grid (2,2);

  \tikzset{scale=1.5, xshift=1cm}
  \path coordinate[my key'''] (b''');
  \path (a''') node[circle,draw]{a} (b''') node[circle,draw]{b};
\end{tikzpicture}

\end{document}
muzimuzhi commented 3 years ago

Yes, but you also set a twice.

Sorry. Apparently I forgot how and why I prepared that poor example. How about continuing the discussion in #964 #965?