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

allow sloped/transform shape with non-identity transformation #1287

Closed
wants to merge 7 commits into from

Conversation

Qrrbrbirlbel
Copy link
Contributor

@Qrrbrbirlbel Qrrbrbirlbel commented Oct 21, 2023

This applies the fixes of @thinbold and @muzimuzhi also to curves and arc while not globalize xc/yc for the line function.

In my opinion, this is not a breaking change because the previous behavior was never the intended way.

Motivation for this change

Fixes #1058

Checklist

Please signoff your commits to explicitly state your agreement to the Developer Certificate of Origin. If that is not possible you may check the boxes below instead:

Qrrbrbirlbel and others added 2 commits October 21, 2023 16:56
This applies the fixes of thinbold and muzimuzhi also to curves and lines while not globalize xc/yc for the line.

Co-Authored-By: Yukai Chou <[email protected]>
Co-Authored-By: Thin Bold <[email protected]>
Co-Authored-By: Yukai Chou <[email protected]>
Co-Authored-By: Thin Bold <[email protected]>
Copy link
Member

@muzimuzhi muzimuzhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only reviewed the styling. I already forgot my own patches. :)

A new test is needed, though it could be added after #1116 is handled first, which has blocked all PRs more than just typo fixes for a while.

Comment on lines 26 to 30
- muzimuzhi
- Qrrbrbirlbel
- quark67
- thinbold
- Yukai Chou (muzimuzhi)
Copy link
Member

@muzimuzhi muzimuzhi Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was already listed on line 30. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I didn't realize that. The recent update only had your nickname so I didn't even scan past the m

Comment on lines 468 to 472
\def\pgftransformlineattime#1#2#3{%
\let\pgf@tempaa\pgf@pt@aa
\let\pgf@tempab\pgf@pt@ab
\let\pgf@tempba\pgf@pt@ba
\let\pgf@tempbb\pgf@pt@bb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three pair of additions to \pgftransformlineattime, \pgftransformarcaxesattime, and \pgftransformcurveattime are so similar that maybe two new macros abstracting them correspondingly can be provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, unfortunately \pgftransformlineattime uses \pgf@xc/\pgf@yc and \pgf@x/\pgf@y isn't save to use inbetween. I would just refactor that so that it uses x/y instead of xc/yc. (Too big of a change?)

That said, there's a lot more that they have in common.

@Qrrbrbirlbel
Copy link
Contributor Author

A new test is needed, though it could be added after #1116 is handled first, which has blocked all PRs more than just typo fixes for a while.

I do know that there's that big #1116 delaying everything else (but I got impatient because it really gets in the way oftentimes – xscale and yscale as in #1287's source is probably rare [and avoidable by scaling via x and y keys] – but just rotating, a very common transformation, is also broken this way).

I do not know much about “a new test”, though, and what is required from me.

@Qrrbrbirlbel
Copy link
Contributor Author

Qrrbrbirlbel commented Oct 21, 2023

Here's a \pgftransform@commonattime macro that combines all the common parts of the \pgftransform…attime commands:

  • #1 needs to setup \pgf@x and \pgf@y for the position, i.e. the \pgftransformshift, which is done by the corresponding \pgfpoint…attime.
  • #2 needs to setup \pgf@x and \pgf@y to be the tangent vector.

The, probably, legacy code of \pgftransformlineattime calculated the tangent again; this is now extracted from \pgfpointlineattime – just as it is with the others.

Here's a testfile with many transformations and all three types of paths (following the example of #1287).
It also contains an adjustment to the \tikz@timer@line timer that would allow distances for the pos key, which then uses a new \pgftransformlineatdistance macro whose definition is now just as easy as for the …attime macros.

(And, as I now realize, this also shows that the name \pgftransform@commonattime isn't really correct, either.)

I've also adjusted the examples in the source since they either used deprecated macros (\pgfxy) or were simply wrong.

\documentclass[tikz]{standalone}
\begin{document}
\tikzset{
  every picture/.append style={
    xscale=2,    yscale=3,
    xshift=40pt, yshift=50pt,
    xslant=.2,   yslant=-.3,
    rotate=-35,
    nodes=above,
    execute at begin picture={
      \draw[help lines, step=1] (0,0) grid (7,3);
      \draw[thick, <->] (0,1) |- (1,0);}}}
%\makeatletter
%\let\iftikz@time@isdistance\iffalse
%\tikzset{
%  pos/.code=\edef\tikz@time{#1}\pgfutil@ifxempty\tikz@time{}{%
%              \pgfmathsetmacro\tikz@time{\tikz@time}%
%              \let\iftikz@time@isdistance\ifpgfmathunitsdeclared}}
%\def\tikz@timer@line{%
%  \iftikz@time@isdistance
%    \pgftransformlineatdistance{\tikz@time pt}{\tikz@timer@start}{\tikz@timer@end}%
%  \else
%    \pgftransformlineattime{\tikz@time}{\tikz@timer@start}{\tikz@timer@end}%
%  \fi}%
%\def\pgftransformlineatdistance#1#2#3{%
%  \pgftransform@commonattime{\pgfpointlineatdistance{#1}{#2}{#3}}{\pgf@x=\pgf@xa\pgf@y=\pgf@ya}}
%\makeatother
\begin{tikzpicture}
\draw (0, 0) -- node[sloped                 ] {\texttt{sloped} only}                         +(2, 2);
\draw (2, 0) -- node[        transform shape] {\texttt{transform shape} only}                +(2, 2);
\draw (4, 0) -- node[sloped, transform shape] {\texttt{sloped} and \texttt{transform shape}} +(2, 2);
\end{tikzpicture}
%\begin{tikzpicture}[pos=2cm]
%\draw (0, 0) -- node[sloped                 ] {\texttt{sloped} only}                         +(2, 2);
%\draw (2, 0) -- node[        transform shape] {\texttt{transform shape} only}                +(2, 2);
%\draw (4, 0) -- node[sloped, transform shape] {\texttt{sloped} and \texttt{transform shape}} +(2, 2);
%\end{tikzpicture}
\begin{tikzpicture}
\draw (0, 0) to[bend left] node[sloped                 ] {\texttt{sloped} only}                         +(2, 2);
\draw (2, 0) to[bend left] node[        transform shape] {\texttt{transform shape} only}                +(2, 2);
\draw (4, 0) to[bend left] node[sloped, transform shape] {\texttt{sloped} and \texttt{transform shape}} +(2, 2);
\end{tikzpicture}
\begin{tikzpicture}[start angle=180, delta angle=-120, x radius=1.5, y radius=.75, nodes=midway]
\draw (0, 0) arc[] node[sloped                 ] {\texttt{sloped} only}                        ;
\draw (2, 0) arc[] node[        transform shape] {\texttt{transform shape} only}               ;
\draw (4, 0) arc[] node[sloped, transform shape] {\texttt{sloped} and \texttt{transform shape}};
\end{tikzpicture}
\end{document}

Qrrbrbirlbel and others added 4 commits October 30, 2023 22:45
This applies the fixes of thinbold and muzimuzhi also to curves and lines while not globalize xc/yc for the line.

Co-Authored-By: Yukai Chou <[email protected]>
Co-Authored-By: Thin Bold <[email protected]>
Signed-off-by: Qrrbrbirlbel <[email protected]>
Co-Authored-By: Yukai Chou <[email protected]>
Co-Authored-By: Thin Bold <[email protected]>
Signed-off-by: Qrrbrbirlbel <[email protected]>
Signed-off-by: Qrrbrbirlbel <[email protected]>

Signoff

Signed-off-by: Qrrbrbirlbel <[email protected]>
@Qrrbrbirlbel Qrrbrbirlbel deleted the branch pgf-tikz:master November 8, 2023 15:27
@Qrrbrbirlbel Qrrbrbirlbel deleted the master branch November 8, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

sloped should consider the current transformation
2 participants