#511 shadings with parameters cannot be redefined

[Eric Domenjoud]

When redefining a shading with parameters, all instances already created keep their old definition and may not be redefined. If one declares a vertical shading myshading with one color parameter mycolor, use it with say mycolor=red and then redefine myshading as a radial shading, any subsequent invocation of myshading with mycolor=red will incorrectly use the vertical shading.

\documentclass{article}

\usepackage{xcolor,pgf}

\begin{document}
\pgfdeclareverticalshading[mycolor]{myshading}{1cm}{%
    color(0cm)=(mycolor);color(1cm)=(mycolor!20)}

\colorlet{mycolor}{red}
\pgfuseshading{myshading}% vertical shading 'myshading[red]'

\pgfdeclareradialshading[mycolor]{myshading}{\pgfpointorigin}{%
    color(0cm)=(mycolor!20);color(.5cm)=(mycolor)}

\colorlet{mycolor}{blue}
\pgfuseshading{myshading}% correct: radial shading 'myshading[blue]'

\colorlet{mycolor}{red}
\pgfuseshading{myshading}% incorrect: vertical shading 'myshading[red]'
\end{document}

[Henri Menke]

As far as I can see this is actually a feature. Shadings are expensive to compute, so when requesting a myshading with the color red, it is cached to avoid reevaluation when using it again later. It seems like redeclaring myshading fails to clear the cache.
The issue is kind of exotic though. Not too many users will reuse the same name for different shadings.

[Eric Domenjoud]

I understand that caching the instances of a parameterized shading may save some computation time. However, it is not the point here. The problem is that when the definition of a shading is changed, the result is (almost) unpredictible because some instances will use the new definition while other will use the old one.

Even if changing the definition of a shading might not seem a good idea, it is not forbidden and one could expect that the new definition is taken into account. Furthermore, it is currently impossible to check it safely because parameterized shadings are declared locally while parameterless ones (including instances of parameterized shadings) are created globally.

I see only two solutions to this problem:

1. define all shadings globally and forbid to redefine them.
2. implement a mechanism which ensures that when the defnition of a shading is changed, the new definition is taken into account. This may be done for instance by incrementing a counter each time a parameterized shading is declared. This counter may be integrated in the internal name of the shading so that pgf will not confuse instances of the old definition and the new one.

In any case, there should be some coherence between local and global definitions.

I personnaly favor the second solution with local definitions for all kinds of shadings. I implemented this solution and it works flawlessly. I compiled the whole pgf manual without any problem. It requires only small changes to the following definitions:

• \pgf@declare(horizontal|vertical|radial|functional)shading, \pgfshade@functionaldo, \pgfuseshading and \pgfaliasshading in pgfcoreshade.code.tex.
• \pgfsys@(hori|vert|radial|functional)shading in each pgfsys-<driver>.def.

The definitions of \pgfsys@(hori|vert|radial|functional)shading in pgfsys.code.def and \pgfsys@functionalshading in pgfsys-common-svg.def do not need to be changed although they define shadings globally. These are only fallback macros and since the corresponding shadings are anyway not available, it is harmless to define them globally.

[Eric Domenjoud]

[Henri Menke]

Fixed in https://sourceforge.net/p/pgf/git/ci/350971220aa5f364db825f38060beaaf65a30854/

[Eric Domenjoud]

I'm very pleased to see that you incorporated my patch. Unfortunately, there is little glitch in the macro \pgfshade@functionaldo in the file pgfcoreshade.code.tex. The braces in

\def\pgfshade@functionaldo#1#2#3#4#5{%
  {%
    #4%
    \pgfsys@functionalshading{#1}{#2}{#3}{#5}%
    \expandafter\pgfmath@smuggleone\csname @pgfshading#1!\endcsname
  }%
}

should be changed to \begingroup, \endgroup because \pgfmath@smuggleone must be followed by \endgroup. The correct code is

\def\pgfshade@functionaldo#1#2#3#4#5{%
  \begingroup
    #4%
    \pgfsys@functionalshading{#1}{#2}{#3}{#5}%
    \expandafter\pgfmath@smuggleone\csname @pgfshading#1!\endcsname
  \endgroup
}

[Eric Domenjoud]

Another potential problem with the way you changed my code. I introduced on purpose the character % in the code for \pgf@num@pgfshading<shading>!. Otherwise, a confusion might occur if one declares on one hand a parametrized shading myshading[mycolor] and use it with say mycolor=red which actually declares myshading<some number>,1,0,0 and on the other hand a parameterless shading myshading<the same number>,1,0,0.

[Henri Menke]

Your patch also broke dvisvgm https://travis-ci.com/pgf-tikz/pgf/jobs/175012402. Can you please fix it?

[Eric Domenjoud]

Sorry for this. I didn't check each driver since the changes where essentially the same. I'll check this and provide a correct patch.

[Henri Menke]

Nevermind, it was my mistake. Thank you very much for your efforts!

[Eric Domenjoud]

I'm not sure to understand. Do you mean that it actually worked or should I nevertheless update the patch ?

[Henri Menke]

It was another fuckup of mine. The patch should work now.

[Eric Domenjoud]

Very well then. I'm very pleased to be able to contribute to this great package.