Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pic angle in a pic doesn't respect pic name/name prefix. #1363

Open
Qrrbrbirlbel opened this issue Oct 6, 2024 · 4 comments · May be fixed by #1369
Open

Pic angle in a pic doesn't respect pic name/name prefix. #1363

Qrrbrbirlbel opened this issue Oct 6, 2024 · 4 comments · May be fixed by #1369
Labels

Comments

@Qrrbrbirlbel
Copy link
Contributor

Qrrbrbirlbel commented Oct 6, 2024

Brief outline of the bug

When the pic angle (or right angle) is used inside another pic, coordinates named inside the pic with non-empty name prefix or name suffix can't be used for coordinates of the angle pic.

This is, because \tikz@lib@angle@parse uses its argument directly with \pgfpointanchor and doesn't check whether a node named \tikz@pp@name{#?} exists:

\def\tikz@lib@angle@parse#1--#2--#3\pgf@stop{%
% Compute radius:
\pgfmathsetmacro\tikz@lib@angle@rad{\pgfkeysvalueof{/tikz/angle radius}}
\ifdim\tikz@lib@angle@rad pt>0pt\else\def\tikz@lib@angle@rad{12}\fi%
% Compute first coordinate:
\pgf@process{\pgfpointanchor{#2}{center}}%
\pgf@xa=\pgf@x%
\pgf@ya=\pgf@y%
\pgf@process{\pgfpointanchor{#1}{center}}%
\pgf@xb=\pgf@x%
\pgf@yb=\pgf@y%
\pgf@process{\pgfpointanchor{#3}{center}}%

My suggestion is to do just that:

\documentclass{article}
\usepackage{tikz}
\usetikzlibrary{angles}
\newcommand*\tikzpicname{\pgfkeysvalueof{/tikz/name prefix}}

\makeatletter
\def\tikz@lib@angle@parse#1--#2--#3\pgf@stop{%
  % Compute radius:
  \pgfmathsetmacro\tikz@lib@angle@rad{\pgfkeysvalueof{/tikz/angle radius}}
  \ifdim\tikz@lib@angle@rad pt>0pt\else\def\tikz@lib@angle@rad{12}\fi%
  % Compute first coordinate:
  \pgf@process{\pgfpointanchor{\pgfutil@IfUndefined{pgf@sh@ns@\tikz@pp@name{#2}}{#2}{\tikz@pp@name{#2}}}{center}}%
  \pgf@xa=\pgf@x%
  \pgf@ya=\pgf@y%
  \pgf@process{\pgfpointanchor{\pgfutil@IfUndefined{pgf@sh@ns@\tikz@pp@name{#1}}{#1}{\tikz@pp@name{#1}}}{center}}%
  \pgf@xb=\pgf@x%
  \pgf@yb=\pgf@y%
  \pgf@process{\pgfpointanchor{\pgfutil@IfUndefined{pgf@sh@ns@\tikz@pp@name{#3}}{#3}{\tikz@pp@name{#3}}}{center}}%
  \pgf@xc=\pgf@x%
  \pgf@yc=\pgf@y%
  \advance\pgf@xb by-\pgf@xa%
  \advance\pgf@yb by-\pgf@ya%
  \advance\pgf@xc by-\pgf@xa%
  \advance\pgf@yc by-\pgf@ya%
  \pgfmathsetmacro{\tikz@start@angle@temp}{atan2(\the\pgf@yb,\the\pgf@xb)}
  \pgfmathsetmacro{\tikz@end@angle@temp}{atan2(\the\pgf@yc,\the\pgf@xc)}
  \ifdim\tikz@end@angle@temp pt<\tikz@start@angle@temp pt%
    \pgfmathsetmacro{\tikz@start@angle@temp}{\tikz@start@angle@temp-360}%
  \fi%
}%
\makeatother
\begin{document}
\tikz\pic (name-) {
  code={
    \coordinate (x) at (right:1) % actually named name-x
     coordinate (y) at (up:1)    % actually named name-y
     coordinate (O) at (0,0);    % actually named name-O
    \pic[draw, ultra thick]  {angle=x--O--y}; % works
    \pic[draw=yellow, thick] {angle=name-x--name-O--name-y}; % works as well
    \pic[draw=red]           {angle=\tikzpicname x--\tikzpicname O--\tikzpicname y}; % workaround still works
  }
};
\end{document}

Of course, just using \tikz@scan@one@point would work, too, while allowing arbitrary coordinates as well, but apparently that wasn't wanted for the angle pic since the manual points this out explicitly:

The three points \meta{A}, \meta{B}, and \meta{C} \emph{must} be the names
of nodes or coordinates; you cannot use direct coordinates like ``|(1,1)|''
here.

Minimal working example (MWE)

\documentclass{article}
\usepackage{tikz}
\usetikzlibrary{angles}
\newcommand*\tikzpicname{\pgfkeysvalueof{/tikz/name prefix}}
\begin{document}
\tikz\pic (name-) {
  code={
    \coordinate (x) at (right:1) % actually named name-x
     coordinate (y) at (up:1)    % actually named name-y
     coordinate (O) at (0,0);    % actually named name-O
    \pic[draw, ultra thick]  {angle=x--O--y};                                        % fails
    \pic[draw=yellow, thick] {angle=name-x--name-O--name-y};                         % works
    \pic[draw=red]           {angle=\tikzpicname x--\tikzpicname O--\tikzpicname y}; % workaround
  }
};
\end{document}
@hmenke
Copy link
Member

hmenke commented Oct 6, 2024

Even if it wasn't designed that way, I like the idea of using \tikz@scan@one@point. However, if it's a node, then we need to use the center anchor. This would probably require quite a lot of code. Maybe it's worth it.

@hmenke hmenke added the pic label Oct 6, 2024
@Qrrbrbirlbel
Copy link
Contributor Author

Qrrbrbirlbel commented Oct 7, 2024

Not really, since that returns the center anchor as a first suggestion and flags \iftikz@shapeborder for later:

\tikz@calc@anchor#2.center\tikz@stop% to be on the save side, in

However, the other macros use #2.center so these also need to be changed. I don't see a reason why that is needed though.

Here's just the rewrite for the calculation.
I've chosen to use a few quicker PGFMath macros. One could also think about using \pgfmathanglebetweenpoints but that does basically the same.

\documentclass{article}
\usepackage{tikz}
\usetikzlibrary{angles}
\newcommand*\tikzpicname{\pgfkeysvalueof{/tikz/name prefix}}

\makeatletter
\def\tikz@lib@angle@parse#1--#2--#3\pgf@stop{%
  % Compute radius:
  \pgfmathsetmacro\tikz@lib@angle@rad{\pgfkeysvalueof{/tikz/angle radius}}%
  \ifdim\tikz@lib@angle@rad pt>0pt\else\def\tikz@lib@angle@rad{12}\fi
  % Compute first coordinate:
  \tikz@scan@@absolute\pgf@process(#2)%
  \pgf@xa=\pgf@x
  \pgf@ya=\pgf@y
  \tikz@scan@@absolute\pgf@process(#1)%
  \pgf@xb=\pgf@x
  \pgf@yb=\pgf@y
  \tikz@scan@@absolute\pgf@process(#3)%
  \pgf@xc=\pgf@x
  \pgf@yc=\pgf@y
  \advance\pgf@xb by-\pgf@xa
  \advance\pgf@yb by-\pgf@ya
  \advance\pgf@xc by-\pgf@xa
  \advance\pgf@yc by-\pgf@ya
  \pgfmathatantwo@{\pgf@sys@tonumber\pgf@yb}{\pgf@sys@tonumber\pgf@xb}%
  \let\tikz@start@angle@temp\pgfmathresult
  \pgfmathatantwo@{\pgf@sys@tonumber\pgf@yc}{\pgf@sys@tonumber\pgf@xc}%
  \let\tikz@end@angle@temp\pgfmathresult
  \ifdim\tikz@end@angle@temp pt<\tikz@start@angle@temp pt
    \pgfmathsubtract@{\tikz@start@angle@temp}{360}%
    \let\tikz@start@angle@temp\pgfmathresult
  \fi
}
\makeatother
\begin{document}
\tikz\pic (name-) {
  code={
    \coordinate (x) at (right:1) % actually named name-x
     coordinate (y) at (up:1)    % actually named name-y
     coordinate (O) at (0,0);    % actually named name-O
    \pic[draw, ultra thick]  {angle=x--O--y};                                        % works
    \pic[draw=yellow, thick] {angle=name-x--name-O--name-y};                         % works as well
    \pic[draw=red]           {angle=\tikzpicname x--\tikzpicname O--\tikzpicname y}; % workaround still works
  }
};
\end{document}

@muzimuzhi
Copy link
Member

I didn't follow previous discussions, but the \pic[draw=yellow, thick] {angle=name-x--{0,0}--name-y}; used in the last example (#1363 (comment)) raises four identical errors

! Package PGF Math Error: Unknown operator `c' or `ce' (in '0.center').

It seems to support uses of non-node coordinates in angle specification, uses of #<n>.center in \tikz@lib@(right)angle@(back|fore)ground need to be generalized.

@Qrrbrbirlbel
Copy link
Contributor Author

It seems to support uses of non-node coordinates in angle specification, uses of #<n>.center in \tikz@lib@(right)angle@(back|fore)ground need to be generalized.

Yes, I mentioned that in my post but didn't take out the offending code from the example.

However, the other macros use #2.center so these also need to be changed. I don't see a reason why that is needed though.

I've replaced it now so that the MWE works … and would also work for arbitrary coordinates but the #2 coordinate still needs to be a node name or the other macros need changing as well.

@Qrrbrbirlbel Qrrbrbirlbel linked a pull request Oct 28, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants